GNOME Bugzilla – Bug 744806
audiodecoder: early call to drain() causes segment event to be sent before caps
Last modified: 2015-08-28 18:27:47 UTC
commit 7c0f885ad246efd18b21c35a9c7f3200828a652c introduced a regression that causes vorbisdec (and potentially other audio decoders) to send the segment event before the caps if they receive a discont buffer early on the stream, before the output caps is decided. The attached patch aims to fix it.
Created attachment 297290 [details] [review] audiodecoder: only send pending events on drain() if we know the output caps Otherwise it will push the segment event before the caps, causing an assertion. To reproduce this, the decoder receives a discont buffer before it has discovered the caps, which causes the drain() call.
Which was: commit 7c0f885ad246efd18b21c35a9c7f3200828a652c Author: Jan Schmidt <jan@centricular.com> Date: Sat Jan 31 05:09:46 2015 +1100 audiodecoder: Fix reverse playback when there's only one gather set. The decoder can fail to drain on EOS if there was only one gather set, because it will never have sent the segment event downstream and set the output segment, and fail to detect that the rate < 0.0 Make sure to send pending events before sending all the gather data for decode. diff --git a/gst-libs/gst/audio/gstaudiodecoder.c b/gst-libs/gst/audio/gstaudiodecoder.c index 2ba58aa..0b768b1 100644 --- a/gst-libs/gst/audio/gstaudiodecoder.c +++ b/gst-libs/gst/audio/gstaudiodecoder.c @@ -1444,6 +1444,9 @@ gst_audio_decoder_drain (GstAudioDecoder * dec) if (dec->priv->drained && !dec->priv->gather) return GST_FLOW_OK; else { + /* Send any pending events before draining, as that + * may update the pending segment info */ + send_pending_events (dec); /* dispatch reverse pending buffers */ /* chain eventually calls upon drain as well, but by that time * gather list should be clear, so ok ... */
OK - obviously the code needs to get a bit smarter about the timing of updating the stored output segment versus sending events. My hack isn't sufficient. Happy to review a patch, or I'll look at it when I get time.
Review of attachment 297290 [details] [review]: I think we should revert my patch and replace it with one that updates the output segment before decoding buffers, but don't send events yet, keep the push_pending_events() until just before the first buffer is output. I'm worried this patch will lead to a case where we still get things wrong in reverse playback and never send EOS.
Created attachment 297572 [details] [review] audiodecoder: Don't send pending events before decode Make sure to update the output segment to track the segment we're decoding in, but don't actually push it downstream until after buffers are decoded.
I think this patch should do what we want - can you test it for your test-case? As always, wishing I'd worked the file that made me make the original change into a test case. If I rediscover it, I will.
Pushed my patch - please reopen if you still have any trouble.