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

opus: implement range decoder. (v2) #313

Open
wants to merge 2 commits into
base: dev-0.6
Choose a base branch
from

Conversation

a1phyr
Copy link
Contributor

@a1phyr a1phyr commented Sep 21, 2024

Rebase of #116 onto dev branch.

I have just rebased the commits and adapted the code to new APIs, but did not add anything significant to the work of @thinking-tower.

cc #8
Close #116

@a1phyr
Copy link
Contributor Author

a1phyr commented Oct 9, 2024

I added another commit to fix the CI on MacOS

@pdeljanov
Copy link
Owner

Could you submit the CI change as a separate PR against master?

Copy link

@hasezoey hasezoey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not familiar what state the opus decoder is currently in, so i just tried running it on a opus file via symphonia-play, but got a divide by zero error:

thread 'main' panicked at symphonia-core/src/audio/buf.rs:84:25:
attempt to divide by zero
stack backtrace:
   0: rust_begin_unwind
             at /rustc/90b35a6239c3d8bdabc530a6a0816f7ff89a0aaf/library/std/src/panicking.rs:665:5
   1: core::panicking::panic_fmt
             at /rustc/90b35a6239c3d8bdabc530a6a0816f7ff89a0aaf/library/core/src/panicking.rs:74:14
   2: core::panicking::panic_const::panic_const_div_by_zero
             at /rustc/90b35a6239c3d8bdabc530a6a0816f7ff89a0aaf/library/core/src/panicking.rs:181:21
   3: symphonia_core::audio::buf::AudioBuffer<S>::new
             at /mnt/ssd/projects/rust/Symphonia/symphonia-core/src/audio/buf.rs:84:25
   4: symphonia_codec_opus::OpusDecoder::try_new
             at /mnt/ssd/projects/rust/Symphonia/symphonia-codec-opus/src/lib.rs:64:21
   5: <symphonia_codec_opus::OpusDecoder as symphonia_core::codecs::registry::RegisterableAudioDecoder>::try_registry_new
             at /mnt/ssd/projects/rust/Symphonia/symphonia-codec-opus/src/lib.rs:119:21
   6: symphonia_core::codecs::registry::CodecRegistry::register_audio_decoder_at_tier::{{closure}}
             at /mnt/ssd/projects/rust/Symphonia/symphonia-core/src/codecs/registry.rs:246:41
   7: core::ops::function::FnOnce::call_once
             at /home/hasezoey/.local/rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:250:5
   8: symphonia_core::codecs::registry::CodecRegistry::make_audio_decoder
             at /mnt/ssd/projects/rust/Symphonia/symphonia-core/src/codecs/registry.rs:314:16
   9: symphonia_play::play_track
             at ./src/main.rs:422:9
  10: symphonia_play::play
             at ./src/main.rs:381:15
  11: symphonia_play::run
             at ./src/main.rs:234:17
  12: symphonia_play::main
             at ./src/main.rs:124:22
  13: core::ops::function::FnOnce::call_once
             at /home/hasezoey/.local/rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:250:5

@@ -35,6 +35,7 @@ wav = ["dep:symphonia-format-riff", "symphonia-format-riff/wav"]
ape = ["symphonia-metadata/ape"]
id3v1 = ["symphonia-metadata/id3v1"]
id3v2 = ["symphonia-metadata/id3v2"]
opus = ["dep:symphonia-codec-opus"]
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i dont know the full state of this PR, but shouldnt this also be in all-codecs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so, the code doesn't do much as this point.

@a1phyr a1phyr force-pushed the opus-dev branch 2 times, most recently from 57482c3 to 59b29bf Compare December 12, 2024 09:16
@hasezoey
Copy link

Retried with the updated code, the "divide by zero" is gone, now its:

thread 'main' panicked at symphonia-play/src/output.rs:61:13:
assertion failed: pa_spec.is_valid()
stack backtrace:
   0: rust_begin_unwind
             at /rustc/90b35a6239c3d8bdabc530a6a0816f7ff89a0aaf/library/std/src/panicking.rs:665:5
   1: core::panicking::panic_fmt
             at /rustc/90b35a6239c3d8bdabc530a6a0816f7ff89a0aaf/library/core/src/panicking.rs:74:14
   2: core::panicking::panic
             at /rustc/90b35a6239c3d8bdabc530a6a0816f7ff89a0aaf/library/core/src/panicking.rs:148:5
   3: symphonia_play::output::pulseaudio::PulseAudioOutput::try_open
             at ./symphonia-play/src/output.rs:61:13
   4: symphonia_play::output::try_open
             at ./symphonia-play/src/output.rs:440:5
   5: symphonia_play::play_track
             at ./symphonia-play/src/main.rs:460:42
   6: symphonia_play::play
             at ./symphonia-play/src/main.rs:381:15
   7: symphonia_play::run
             at ./symphonia-play/src/main.rs:234:17
   8: symphonia_play::main
             at ./symphonia-play/src/main.rs:124:22
   9: core::ops::function::FnOnce::call_once
             at /home/hasezoey/.local/rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:250:5

and when adding debug log for spec:

DEBUG: AudioSpec {
    rate: 0,
    channels: None,
}

Which i guess is now the more correct output as there is no decoding done (and so the audio buffer is not configured), right?

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

Successfully merging this pull request may close these issues.

4 participants