Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Librespot: thread '' panicked at 'attempted to zero-initialize type vorbisfile_sys::ov_callbacks, which is invalid' #681

Closed
heitbaum opened this issue Mar 26, 2021 · 5 comments · Fixed by #750

Comments

@heitbaum
Copy link

With LE10 - librespot was failing due to an error in vorbis-rs

LibreELEC/LibreELEC.tv#5038

the fix was identified in tomaka/vorbis-rs#19 and RustAudio/lewton#89

we are adding a patch to LE10 to have librespot working via PR LibreELEC/LibreELEC.tv#5166 and specifically the following patch to the librespot 0.1.6 source LibreELEC/LibreELEC.tv@04c7591

would be great if you could have this or similar included in the mainline librespot.

@heitbaum heitbaum changed the title Librespot: thread '' panicked at 'attempted to zero-initialize type vorbisfile_sys::ov_callbacks, which is invalid', /rustc/cb75ad5db02783e8b0222fee363c5f63f7e2cf5b/library/core/src/mem/mod.rs:623:9 Librespot: thread '' panicked at 'attempted to zero-initialize type vorbisfile_sys::ov_callbacks, which is invalid' Mar 26, 2021
@Johannesd3
Copy link
Contributor

Seems as if vorbis-rs isn't maintained anymore by @tomaka. Should librespot create its own fork? Maybe we can share some code with librespot-tremor, as it is essentially the same.

@roderickvd
Copy link
Member

While forking seems a fine idea if we want to keep libvorbis support around, is that indeed what we want to do? What does it add over lewton, that's fast enough even on a RPi Zero? Even for tremor I'm not sure which hardware without hard float support is actually a target for librespot.

@roderickvd
Copy link
Member

@heitbaum removing support for libvorbis and tremor is under serious consideration. As far as I know, LibreELEC is the only thing downstream that compiles with --features with-vorbis. Any arguments against moving to lewton 100%?

I just checked and compiling with-vorbis only saves 167 kB. Likewise the slight boost in performance will be neglible. At the same time, the bindings are unmaintained and only pass integer samples instead of the floats they were originally in, causing quantization error and lower sound quality. Finally, there is no Rust-unsafe code in lewton. So there's a lot to say about moving to lewton and nothing of significance for keeping libvorbis as I can see.

Thanks in advance for your feedback, I'll be submitting a PR to remove libvorbis and tremor soon.

@heitbaum
Copy link
Author

I have just raised the PR for LIbreELEC for 0.3.1 - just asked our forum for someone to test. All updated with your vorbis cleanups 👍 LibreELEC/LibreELEC.tv#5891 and LibreELEC/LibreELEC.tv#5892

@roderickvd
Copy link
Member

Great stuff @heitbaum!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants