serenity-rs / songbird

An async Rust library for the Discord voice API
ISC License
381 stars 107 forks source link

The track ends sooner than expected #211

Open a-liashenko opened 7 months ago

a-liashenko commented 7 months ago

Songbird version: (version) Tested on 0.4 and master branch

Rust version (rustc -V): (version) rustc 1.74.0 (79e9716c9 2023-11-13)

Serenity/Twilight version: (version) Twilight version = "0.15"

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

Description: Songbird will abort the track if it has any decoder errors. I'm not sure how audio and video encoded internally, but looks like I found track with wrong frame ordering. All non-seekable inputs will lead to early track interruption. After adding decoder error logs here, I can see errors like: DecodeError("isomp4: packet out-of-bounds for a non-seekable stream"). Whole play workflow log:

[2023-12-08T15:17:44Z INFO  songbird::driver::tasks::ws] run; interconnect=Interconnect { core: Sender, events: Sender, mixer: Sender }
[2023-12-08T15:17:44Z INFO  songbird::driver::tasks::events] Event state for track 1 added
[2023-12-08T15:17:44Z INFO  songbird::driver::tasks::ws] Changing to SpeakingState(MICROPHONE)
[2023-12-08T15:17:44Z DEBUG songbird::driver::tasks::events] Changing state for track 0 of 1: Ready(Preparing)
[2023-12-08T15:17:44Z INFO  songbird::events::store] Firing Preparing for [0]
[2023-12-08T15:17:45Z DEBUG songbird::driver::tasks::events] Changing state for track 0 of 1: Ready(Playable)
[2023-12-08T15:17:45Z INFO  songbird::events::store] Firing Playable for [0]
// my new log message
[2023-12-08T15:17:47Z TRACE songbird::driver::tasks::mixer::mix_logic] input.format.next_packet() got error DecodeError("isomp4: packet out-of-bounds for a non-seekable stream")
//
[2023-12-08T15:17:47Z DEBUG songbird::driver::tasks::events] Changing state for track 0 of 1: Mode(End)
[2023-12-08T15:17:47Z INFO  songbird::events::store] Firing End for [0]
[2023-12-08T15:17:47Z INFO  songbird::driver::tasks::events] Event state for track 0 of 1 removed.
[2023-12-08T15:17:47Z INFO  songbird::driver::tasks::ws] Changing to SpeakingState(0x0)

As fallback option, I solved it with ignoring any decode errors from symphonia.
Is it possible to implement a better solution? I'm ready to create PR with my fix or some better option.

Steps to reproduce: Try to play this track via songbird https://www.youtube.com/watch?v=Gk2I2zs_BY0 I tried to use HttpRequest input with direct link and I have the error above. File input will work fine, because it is seekable source

FelixMcFelix commented 4 months ago

Sorry for the late reply. I was in fact able to play the file you linked without issue, which suggests that the format specifiers we use aren't being honored for some reason in your case; outdated yt-dlp version, rate-limiting, or possibly some geofencing behaviour? I believe I have run into a similar issue in the past with older videos not being served as WebM and triggering similar issues.

I think there is value in adding an entry to Config for controlling how we handle next packet/packet decode errors, but it's tricky to get right without allowing an arbitrary file to block the mixer thread for an unbounded time. Maybe you could hypothetically write up a way of allowing up to n lookup errors in a single mix before giving up.