GNOME Bugzilla – Bug 741280
matroskademux: Send GAP events for non sparse streams
Last modified: 2016-02-16 17:26:26 UTC
There doesn't seem to be any way to determine and send EOS events for individual streams. So when audio stream is significantly shorter than video, the pipeline will get stuck at the end because all multiqueues will fill-up on video stream. This can be prevented by sending GAP events on audio streams when it lags too much.
Created attachment 292343 [details] [review] Patch
Review of attachment 292343 [details] [review]: ::: gst/matroska/matroska-demux.c @@ +2474,3 @@ + tolerance = GST_SECOND / 2; + } else { + tolerance = GST_SECOND * 5; 5 seconds is quite a lot, and that can easily be enough to fill up queues downstream and stall the pipeline. I think other demuxers are also using 0.5s as a tolerance for all streams. But in any case a smaller value than 5s would probably be good. Did you notice problems with 0.5s? What about 2s?
I think I have some files where 0.5s was generating GAP events mid-stream for audio (even though the stream was continuos). I think it is okay for subtitles (at least I haven't encountered any issues with 0.5 for subtitles), but for audio it is too strict. All my queues are setup to handle at least 10 second lag (better safe than sorry, I've seen some really badly muxed files), but I agree that we can't expect every multi queue to handle this. I will test 2s, I think it *should* be sufficient.
It would be good to make it low enough for the default limits in playbin :)
Isn't it a concern that we might be sending gap events for not-perfectly-interleaved audio/video then, introducing audio artefacts? (I'm pretty sure none of our code will handle an audio buffer with ts=T if we have received a gap event with ts=T before)
It is a concern of course :) That's why we need to add tolerance that is high enough so that this is minimized. If we don't send gap event at the end of stream then the entire pipeline just gets stuck, because there is no data and as far as I can tell you can not determine EOS on individual streams.
Also if we send no GAP events for audio if the interleaving is too bad, we will just stall the pipeline because the video queue runs full. Maybe it's time to implement the buffering query or something like it to detect how much buffering downstream has, and select the tolerance based on that.
(In reply to comment #5) > Isn't it a concern that we might be sending gap events for > not-perfectly-interleaved audio/video then, introducing audio artefacts? (I'm > pretty sure none of our code will handle an audio buffer with ts=T if we have > received a gap event with ts=T before) Elements like audiorate and videorate at least should put neutral data (silence, previous frame) in there for gap events... so such a buffer would be arriving "too late" and be clipped or dropped.
Exactly, thus introducing audio artefacts (silence). 2 seconds just seems a bit on the low side, but I have absolutely no empirical data to back that up, nor have I checked the matroska spec to see if it stipulates anything about that.
Well, you make the tolerance too small you get possible artifacts. You make it too big you get pipeline stalls. How much lag can playbin handle before it stalls?
Depends on the buffering settings. I think to solve this properly we need to implement something like the BUFFERING query :)
Are there plans for such thing in 1.6?
Not that I'm aware of, unless you want to start working on that :)
This looks like what I just pushed (without first checking bugzilla, sorry): commit 77403d0afee635f2de6c2e53a23e1f50ad0d00fa Author: Tim-Philipp Müller <tim@centricular.com> Date: Fri Aug 21 14:15:18 2015 +0100 matroska-demux: send GAP events for lagging audio and video streams too Send GAP events for non-subtitle streams too if they lag too much behind, but use a higher threshold than for subtitles. This helps with fixing prerolling with a file where one of the audio streams only has data starting from 19s onwards. It's not a complete fix yet, it also requires changes elsewhere, such as in baseparse, to make sure caps are propagated. https://bugzilla.gnome.org/show_bug.cgi?id=614460 https://bugzilla.gnome.org/show_bug.cgi?id=753899 My magic value use 3 seconds now, let's see how it goes with that. That patch was for initial prerolling, I hope it fixes things for you too. If not, please re-open.