GNOME Bugzilla – Bug 620154
[rtph264depay] Seeking with RTP payloaders corrupts images sometimes
Last modified: 2010-06-16 14:00:38 UTC
I have an application that seeks in a matroska file and sends it using RTP, sometimes i notice that the visualization gets corrupt. With this pipeline i can reproduce this problem, using left/right arrows from navseek sometimes causes the image corruption. gst-launch filesrc location=/tmp/E01_100406T132816906+120-133316949+120_0000040_7502.mkv ! matroskademux ! queue ! decodebin ! x264enc ! rtph264pay ! rtph264depay ! queue ! decodebin ! navseek ! xvimagesink Video: http://www.megaupload.com/?d=UJIQFM30
I have done further researching and these pipelines works fine: gst-launch filesrc location=/tmp/E01_100406T132816906+120-133316949+120_0000040_7502.mkv ! matroskademux ! queue ! decodebin ! x264enc ! queue ! decodebin ! navseek ! xvimagesink gst-launch filesrc location=/tmp/E01_100406T132816906+120-133316949+120_0000040_7502.mkv ! matroskademux ! queue ! decodebin ! jpegenc ! rtpjpegpay ! rtpjpegdepay ! queue ! decodebin ! navseek ! xvimagesink So this means that probably the bug is in rtph264pay or rtph264depay.
I have found a possible bug in rtph264depay. It seems that when it outputs NALU they are never marked as with delta unit flag. Then because after a flush the ffdec_h264 is waiting for a buffer not marked as delta unit it will take the NALU as the keyframe it was waiting and continue to decode, but it is possible that the next frame is not a keyframe. Part of the output of a identity element just before a rtph264depay: /GstPipeline:pipeline0/GstIdentity:identity0: last-message = "chain ******* (identity0:sink)i (6 bytes, timestamp: 0:00:00.280000000, duration: 99:99:99.999999999, offset: -1, offset_end: -1, flags: 0) 0x939aea0" /GstPipeline:pipeline0/GstIdentity:identity0: last-message = "chain ******* (identity0:sink)i (10488 bytes, timestamp: 0:00:00.280000000, duration: 99:99:99.999999999, offset: -1, offset_end: -1, flags: 256) 0xb66bb240" /GstPipeline:pipeline0/GstIdentity:identity0: last-message = "chain ******* (identity0:sink)i (6 bytes, timestamp: 0:00:00.320000000, duration: 99:99:99.999999999, offset: -1, offset_end: -1, flags: 0) 0x936f8f0" /GstPipeline:pipeline0/GstIdentity:identity0: last-message = "chain ******* (identity0:sink)i (10532 bytes, timestamp: 0:00:00.320000000, duration: 99:99:99.999999999, offset: -1, offset_end: -1, flags: 256) 0x936f850" /GstPipeline:pipeline0/GstIdentity:identity0: last-message = "chain ******* (identity0:sink)i (6 bytes, timestamp: 0:00:00.360000000, duration: 99:99:99.999999999, offset: -1, offset_end: -1, flags: 0) 0x939b1e0" /GstPipeline:pipeline0/GstIdentity:identity0: last-message = "chain ******* (identity0:sink)i (10312 bytes, timestamp: 0:00:00.360000000, duration: 99:99:99.999999999, offset: -1, offset_end: -1, flags: 256) 0x936f940" /GstPipeline:pipeline0/GstIdentity:identity0: last-message = "chain ******* (identity0:sink)i (6 bytes, timestamp: 0:00:00.400000000, duration: 99:99:99.999999999, offset: -1, offset_end: -1, flags: 0) 0x939af40" /GstPipeline:pipeline0/GstIdentity:identity0: last-message = "chain ******* (identity0:sink)i (10548 bytes, timestamp: 0:00:00.400000000, duration: 99:99:99.999999999, offset: -1, offset_end: -1, flags: 256) 0x939ad60" /GstPipeline:pipeline0/GstIdentity:identity0: last-message = "chain ******* (identity0:sink)i (6 bytes, timestamp: 0:00:00.440000000, duration: 99:99:99.999999999, offset: -1, offset_end: -1, flags: 0) 0x939b000" /GstPipeline:pipeline0/GstIdentity:identity0: last-message = "chain ******* (identity0:sink)i (10564 bytes, timestamp: 0:00:00.440000000, duration: 99:99:99.999999999, offset: -1, offset_end: -1, flags: 256) 0x936f650" /GstPipeline:pipeline0/GstIdentity:identity0: last-message = "chain ******* (identity0:sink)i (6 bytes, timestamp: 0:00:00.480000000, duration: 99:99:99.999999999, offset: -1, offset_end: -1, flags: 0) 0xb66bb1a0" /GstPipeline:pipeline0/GstIdentity:identity0: last-message = "chain ******* (identity0:sink)i (10717 bytes, timestamp: 0:00:00.480000000, duration: 99:99:99.999999999, offset: -1, offset_end: -1, flags: 256) 0x936f560"
Created attachment 163552 [details] [review] RTP h264 depay fix for delta unit detection
Created attachment 163557 [details] [review] RTP h264 depay fix for delta unit detection There was a bug about the return value in my previous patch.
Review of attachment 163552 [details] [review]: There is a bug about the return value.
Created attachment 163564 [details] [review] rtph264depay: also consider AU and SEI NALUs as DELTA_UNIT
commit dde38254055f220c228386eb3bf6f5e67b1d6313 Author: Mark Nauwelaerts <mark.nauwelaerts@collabora.co.uk> Date: Mon Jun 14 11:46:32 2010 +0200 rtph264depay: also consider AU and SEI NALUs as DELTA_UNIT Fixes #620154.
I think that your patch is very complex for what is needed, also there are NAL units marked as keyframes when they are not. I asked in x264 channel and the easiest keyframes to detect are nal_unit_type=5 (IDR) and nal_unit_type=6 (SEI) with subtype=recovery_point, there are more cases but with these are enough. The rest of cases are considered non keyframes.
If the objective is to have sufficient info (for the decoder) following some seek (whereby DELTA marked ones are dropped), it would seem at least advisable to not have SP/PPS dropped. So, that would then make the simple version one that marks IDR, SPS and PPPS as key, and all others as DELTA.
I am not sure about marking SPS and PPPS as keyframes. Because even though a decoder have them, if a delta frame arrives a decoder will output trash because it needs a keyframe to continue decoding correctly.
True. But if SPS/PPS are dropped, then it might not be able to output anything "ever again", as opposed to possibly temporarily some trashy output. Also, there is a good chance SPS/PPS are followed by IDR. Btw, it might also help to select AU as output, to arrive at a more "normal stream" (e.g. one that would eliminate some "chance" in the above).
I don't know if now is supported in GStreamer but then, SPS/PPS should be marked as metadata and not as keyframe. If a decoder receives metadata it just stores it. I don't understand what you try to say about AU (access unit delimiter?), but they are not a keyframe. There could be an AU and following a DELTA frame, resulting in trash at decoder output.
(In reply to comment #12) > I don't know if now is supported in GStreamer but then, SPS/PPS should be > marked as metadata and not as keyframe. If a decoder receives metadata it just > stores it. Unfortunately, there is no specific "metadata flag" for this particular purpose that I know of. > > I don't understand what you try to say about AU (access unit delimiter?), but > they are not a keyframe. There could be an AU and following a DELTA frame, > resulting in trash at decoder output. AU = Access Unit, that is, have rtph264depay output AU (access units) rather than individual NALUs (though that may often coincide). That way, e.g. SPS and PPS should get merged into one buffer with other NALUs and some DELTA_UNIT (or not) ambiguity would be lifted.
I have tested the code without patching with rtph264depay with 'access-unit' set to true and seek works fine.
What about having the depayloader but the sps/pps in the caps instead of putting them in the stream.
That would (more or less) come down to using rtph264depay in byte-stream=false mode.
Don't you think is better to mark IDR, SPS and PPS as keyframes and the rest as delta frames? The way is done now a NAL unit type >= 10 is marked as a keyframe.
As suggested in Comment #9 and Comment #17: commit 6a9c70486ff3f1d3771105b2fccedb912e78f6fe Author: Mark Nauwelaerts <mark.nauwelaerts@collabora.co.uk> Date: Wed Jun 16 15:52:57 2010 +0200 rtph264depay: tweak DELTA_UNIT labeling Consider SPS, PPS and IDR as keyframe, all others as DELTA_UNIT. See #620154.