search

RustAudio / lewton

Rust vorbis decoder
Other
261 stars 26 forks source link

The dev/cmp bench command panics #89

Open nico-abram opened 3 years ago

nico-abram commented 3 years ago

After patching the dead bitbucket link for thingy-floor0.ogg to http://achurch.org/hg/libnogg/raw-file/tip/tests/data/thingy-floor0.ogg in /dev/cmp/src/lib.rs, it panics within vorbis-rs when running cargo run --release -- bench:

thread 'main' panicked at 'attempted to zero-initialize type `vorbisfile_sys::ov_callbacks`, which is invalid', C:\Users\Nick\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib/rustlib/src/rust\library\core\src\mem\mod.rs:623:9

I opened https://github.com/tomaka/vorbis-rs/issues/19, and opened a PR into vorbisfile-sys. If that gets merged, then a simple change in vorbis-rs should fix this specific panic, however the master branch for vorbis-rs also has other issues (I was also unable to compile it on windows, ran into a missing pthread.h C compiler error, but that could be because of a mistake on my part) , and I'm not entirely sure if the library is still maintained (Even if vorbis-rs is still maintained the link error also involves vorbis-encoder which looks unmaintained to me)

I'm not sure what a good path forward would be

est31 commented 3 years ago

vorbisfile PR: https://github.com/tomaka/vorbisfile-sys/pull/2

est31 commented 3 years ago

@nico-abram regarding the build error, it's a bit weird. The CI for lewton also runs on windows.

nico-abram commented 3 years ago

I opened an issue for it here https://github.com/tomaka/vorbisfile-sys/issues/3

lewton and /dev/cmp both compile fine for me on windows, the problem was the vorbisfile-sys master branch (I think there were probably changes in it's build script since the version from crates io that /dev/cmp is using)

nico-abram commented 3 years ago

It seems the windows issue was recently fixed but for vorbisfile-sys to work it needs a vorbis-rs release to point to

est31 commented 2 years ago

There is PR with a fix: https://github.com/tomaka/vorbis-rs/pull/20

As it's not merged yet, I've made f655fa268ab2b8dcdc87f5529ed3fb8c2865c43b to make the CI apply the PR as a workaround.