GNOME Bugzilla – Bug 765796
rtp depayloaders output bad segment events when input stream is a non time segment
Last modified: 2016-11-01 19:09:56 UTC
Created attachment 326991 [details] Patch to fix and test to validate This happens when doing appsrc ! <depayloader> ! ... depayloaders then generate a new segment event with base=-1 which makes buffers it outputs afterwards not be valid for the segment.
Created attachment 326992 [details] [review] Patch to fix and test to validate
It would be great to have this patch applied to 1.6 as well as current also.
A non-TIME segment for RTP data is not really valid, I expect you'll run into more problems with that :)
Meaning, the depayloader should probably completely reject the segment. (There will also be no further 1.6 release, but backporting a fix to the 1.6 branch should be possible, depending on the fix)
I don't think it's a good idea to make rtp depayloaders accept non-TIME segments, you configure appsrc to create a TIME segment IMHO.
currently the depayloaders do accept non-TIME segments. Just they output a bad new segment event. I'd be fine if we patched the basedepayloader to completely reject non-TIME segments too but the current status-quo made it take quite a while to debug some other weirdness further down the pipeline from the depayloader.
Yes it should just reject the segment, see comment 4. It doesn't make sense to work with RTP on a non-TIME segment.
Having said that, we should probably apply the patch to 1.8/1.6 and log a GST_ERROR() if it's a non-TIME segment; in master we can then take more drastic measures to reject it if we want (whatever those might be).
Created attachment 327130 [details] [review] rtpbasedepayload: Reject non-TIME segments
Created attachment 327131 [details] [review] basertpdepayload: create valid segment when given non-time segment This will become an error in 1.10.
Something like this? I removed the unit test as it will just fail in git master and it's unsupported behaviour
See #766438 for a patch that makes this work through the JB, so you can do something like appsrc ! rtpjitterbuffer ! rtp*depay ..
Comment on attachment 327131 [details] [review] basertpdepayload: create valid segment when given non-time segment Attachment 327131 [details] pushed as 0f609bc - basertpdepayload: create valid segment when given non-time segment
For 1.12 we can merge the other patch then, or not.
Olivier, what should we do here?
I'd say let's push the patch, the jitterbuffer now has code to generate a TIME segment so you can do "multifilesrc ! rtpjitterbuffer ! depay ! ..." and that should work.
Attachment 327130 [details] pushed as d84879d - rtpbasedepayload: Reject non-TIME segments