ogg/vorbis playback fails due to invalid modulo if source_packet.frames() returns 0

[BonsaiDen]

Songbird version: 0.4.0

Rust version (rustc -V): rustc 1.76.0 (07dca489a 2024-02-04)

Serenity/Twilight version: serenity 0.12

Output of ffmpeg -version, yt-dlp --version (if relevant): Not relevant

Description: Trying to play back ogg/vorbis files fails with the following panic:

[2024-02-20 02:13:21] [songbird::driver::tasks::events] [DEBUG] Changing state for track 0 of 1: Ready(Preparing)
[2024-02-20 02:13:21] [songbird::driver::tasks::events] [DEBUG] Changing state for track 0 of 1: Volume(1.1357472)
[2024-02-20 02:13:21] [songbird::driver::tasks::events] [INFO] Adding event to track 0.
[2024-02-20 02:13:21] [songbird::events::store] [INFO] Firing Preparing for [0]
[2024-02-20 02:13:21] [symphonia_core::probe] [DEBUG] found a possible format marker within [4f, 67, 67, 53, 0, 2, 0, 0, 0, 0, 0, 0, 0, 0, 51, cf] @ 0+2 bytes.
[2024-02-20 02:13:21] [symphonia_core::probe] [INFO] found the format marker [4f, 67, 67, 53] @ 0+2 bytes.
[2024-02-20 02:13:21] [symphonia_format_ogg::page] [DEBUG] grow page buffer to 8192 bytes
[2024-02-20 02:13:21] [symphonia_format_ogg::demuxer] [INFO] starting new physical stream
[2024-02-20 02:13:21] [symphonia_format_ogg::demuxer] [INFO] selected vorbis mapper for stream with serial=0x444ecf51
[2024-02-20 02:13:21] [symphonia_format_ogg::page] [DEBUG] grow page buffer to 16384 bytes
[2024-02-20 02:13:21] [symphonia_codec_vorbis] [DEBUG] vorbis: leftover bits in setup head extra data
thread '<unnamed>' panicked at crates/songbird/src/driver/tasks/mixer/mix_logic.rs:164:17:
attempt to calculate the remainder with a divisor of zero
stack backtrace:
[2024-02-20 02:13:21] [songbird::driver::tasks::events] [DEBUG] Changing state for track 0 of 1: Ready(Playable)
   0: rust_begin_unwind
             at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/std/src/panicking.rs:645:5
   1: core::panicking::panic_fmt
             at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/core/src/panicking.rs:72:14
   2: core::panicking::panic
             at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library/core/src/panicking.rs:144:5
   3: songbird::driver::tasks::mixer::mix_logic::mix_symph_indiv
             at ./crates/songbird/src/driver/tasks/mixer/mix_logic.rs:164:17
   4: songbird::driver::tasks::mixer::Mixer::mix_tracks
             at ./crates/songbird/src/driver/tasks/mixer/mod.rs:818:38
   5: songbird::driver::tasks::mixer::Mixer::mix_and_build_packet
             at ./crates/songbird/src/driver/tasks/mixer/mod.rs:526:23
   6: songbird::driver::scheduler::live::Live::run_once
             at ./crates/songbird/src/driver/scheduler/live.rs:217:19
   7: songbird::driver::scheduler::live::Live::run
             at ./crates/songbird/src/driver/scheduler/live.rs:188:15
   8: songbird::driver::scheduler::live::Live::spawn::{{closure}}
             at ./crates/songbird/src/driver/scheduler/live.rs:630:13
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

Steps to reproduce:

Used roughly the following setup:

        if let Some(manager) = manager { // Arc<Songbird>
            if let Some(handler_lock) = manager.get(self.guild_id) {
                let input: Input = songbird::input::File::new(file_path.clone()).into(); // An .ogg file
                let mut handler = handler_lock.lock().await;
                let handle = handler.play_input(input);
            }
        }

Interestingly, adding a simple if to avoid the modulo by 0 here makes everything work just fine, as only the first packet seems to have a 0 frames.

ffprobe has the following to say about the ogg file:

  Duration: 00:00:15.02, start: 0.000000, bitrate: 73 kb/s
    Stream #0:0: Audio: vorbis, 48000 Hz, mono, fltp, 80 kb/s
    Metadata:
      ENCODER         : Lavc56.1.100 libvorbis
      COMPATIBLE_BRANDS: isomiso2mp41
      MINOR_VERSION   : 512
      MAJOR_BRAND     : isom

[FelixMcFelix]

Nice find! I don't know too much about the Vorbis format, but I think skipping past 0-frame packets is probably the right call assuming the spec doesn't assign any other meaning.