After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 773104 - rtpbasedepayload: Drop gap events before first buffer
rtpbasedepayload: Drop gap events before first buffer
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
unspecified
Other All
: Normal normal
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-10-17 13:57 UTC by Stian Selnes (stianse)
Modified: 2018-11-03 11:50 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
rtpbasedepayload: Drop gap events before first buffer (4.55 KB, patch)
2016-10-17 13:57 UTC, Stian Selnes (stianse)
none Details | Review

Description Stian Selnes (stianse) 2016-10-17 13:57:34 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.
Comment 1 Stian Selnes (stianse) 2016-10-17 13:57:38 UTC
Created attachment 337855 [details] [review]
rtpbasedepayload: Drop gap events before first buffer
Comment 2 Nicolas Dufresne (ndufresne) 2016-10-17 14:22:41 UTC
Won't that break sub-title streams sent to a muxer ?
Comment 3 Olivier Crête 2016-10-18 19:24:22 UTC
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.
Comment 4 Stian Selnes (stianse) 2016-10-24 10:16:15 UTC
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?
Comment 5 Nicolas Dufresne (ndufresne) 2016-10-24 14:16:54 UTC
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.
Comment 6 Tim-Philipp Müller 2017-11-21 13:54:54 UTC
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?
Comment 7 GStreamer system administrator 2018-11-03 11:50:53 UTC
-- 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.