GNOME Bugzilla – Bug 692787
rtph264pay: No way to clear SPS and PPS in case of a new stream
Last modified: 2013-11-04 22:08:11 UTC
Created attachment 234717 [details] [review] Patch When RTP payloading different streams successively with the same H264 payloader element in a pipeline, there is no way to clear sps and pps GList (at least in the case of packetized buffer). This leads to problem as the sps and pps of the new stream will be added in the list with the ones of the previous stream but they might have the same pps and sps ids as the 2 stream are not correlated. Let's imagine now that I switch back to the first stream, its sps and pps will not be added to the list as they are already there but on the receiver the depayloader will receive first the sps and pps of the first stream (the good one) and then the sps and pps of the second one with the same sps and pps ids and codec_data in the caps will be updated with the wrong sps and pps. Therefore the wrong information will be given to the decoder and it will fail decoding. In attachment the patch provided, correct this problem by handling STREAM_START events which are the marker of a new stream. gst_rtp_h264_pay_clear_sps_pps() is called when receiving such an event and therefore it will clear the sps and pps only for a new stream. Cheers, Paul HENRYS
I think it should work something like this: - (aggregated) SPS/PPS should be signalled in the caps - for AVC in codec_data - for byte-stream as "streamheader" field - an h.264 encoder should set that field - an h.264 parser should also set it - caps should get updated if SPS/PPS change - it should not be harmful to have unused old ones in there - when you switch streams in input-selector, you should get the caps segment etc. from the new stream sent. - but probably not the stream_start event, at least not by default - the payloader can then pick up the changed SPS/PPS from the changed caps It's tricky what to do here. I actually think it should not forward the first stream-start, but rather create a new stream-start event of its own, possibly made up of the input stream-start IDs. There are conceptually two different use cases for input-selector: create a new stream made up of bits of the input streams, and what you want to do (switch = new stream of some sort).
Paul, do you see any reason why this would not work for your use case ?
Hi Tim, Sorry to answer only now and thanks for your comment. Actually the only case, I would see it could lead to problems, is when using config-interval property (and that's what we are doing). In this case, rtppayload would send regularly the sps/pps based on the lists of sps and pps and the problem that I described previously will happen. But in the case this property is set to 0 (i.e. disable), I guess (I didn't check the code in this case) the new sps/pps are sent when received from the caps and there is no problem. In other word, there is no "risk" to send the sps/pps of the other "old" streams (stored in the list) after the sps/pps of the current stream.
So is the attached patch correct or what is needed here?
Don't we want to only clear the sps/pps if the stream-id changes ?
Yes, good point. But do we want to clear the SPS/PPS in other situations too?
Note that this is related to, or depends on, input-selector bug #692785 - also see discussion there. I don't think the patch is harmful, but I don't think it solves the reporter's problem. I think elements should aggregate SPS/PPS and then clear the list by signalling the aggregated list in the caps (codec_data for AVC, streamheader for byte-stream) if it changes for some reason.
Pushed commit 8eceb8f32755f7a9522c9df836fa79504088165a Author: Paul HENRYS <visechelle@gmail.com> Date: Tue Jan 29 10:51:07 2013 +0100 Add call to gst_rtp_h264_pay_clear_sps_pps() when receiving a STREAM_START e https://bugzilla.gnome.org/show_bug.cgi?id=692787
Olivier, any opinion on my comment #7 ?
I'm not sure we'll have streamheader implemented in caps for all of the encoders/parsers any time soon, but that would be a good idea in general, it would avoid forcing every other element to aggregate them.