GNOME Bugzilla – Bug 773104
rtpbasedepayload: Drop gap events before first buffer
Last modified: 2018-11-03 11:50:53 UTC
Before a gap event is pushed downstream a segment event must be pushed since the gap event can cause packet concealment downstream and hence data flow. Since concealment before receiving any data packets usually doesn't make any sense, the gap event is not sent downstream. Alternatively one could generate a default caps and segment event, but no need to complicate things until it's proven necessary.
Created attachment 337855 [details] [review] rtpbasedepayload: Drop gap events before first buffer
Won't that break sub-title streams sent to a muxer ?
I think that can also break "playback" cases where you want to unblock the sink based on GAP events. Maybe the decoder should just forward the GAP events instead of trying to do concealment before the first buffer.
Nicolas, Olivier, I'm not familiar with the use cases you describe. But let me describe more in detail what the patch does, because the commit message perhaps wasn't specific enough. The patch only changes the behavior of gst_rtp_base_depayload_packet_lost() which is called when the rtpjitterbuffer sends an GstRTPPacketLost event downstream about actually lost RTP packets. If no packets are lost this code path will not be entered. The gap event is dropped only if the depayloader has not yet sent a segment event downstream, which sounds reasonable since the gap event's timestamp and duration has little value without the segment event. Does this behavior conflict with what you're describing?
Sounds like a really familiar issue. Some code will craft a caps + segment for that purpose. The question is did sparse streams worked before ? Could we mux a stream with one stream sending only gaps ? (that last one is more due to CollectPads then this, but sending gaps is the old way to fix this). We should probably write a unit test for that.
There should already be an input segment at this point, so the main problem is the caps event, right? As far as I can tell, rtpjitterbuffer only sends packetlost events when it detects that packets are actually lost. It won't send these for sparse streams, an it won't send any if it doesn't receive packets initially. The patch seems fine to me, but we could limit it to audio/video if needed, but it's not clear to me that that's needed, as per above. We know that muxers don't handle live inputs well. So any pipeline with collectpads-based muxers is already pretty much broken for this use case, so this particular patch won't make much difference to them. Also, for most muxers it's more important to get accurate caps once, and getting incomplete caps or changing caps later is a big no-no, so making up some bogus caps seems very suboptimal for that use case. We did make sure to pass through gap events so that e.g. audiosinks could preroll and start up in cases where there's no audio at the beginning, but I believe this is only relevant for non-live pipelines?
-- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gst-plugins-base/issues/301.