GNOME Bugzilla – Bug 737173
h264parse wrongly insert DISCONT flags when converting into AVC
Last modified: 2016-07-24 15:58:05 UTC
Running the following pipeline produces wrongly ordered frames being rendered. GST_DEBUG=*parse*:6 gst-launch-1.0 -fv filesrc location= ~/media/interlaced_short.mts ! tsdemux ! h264parse ! video/x-h264, stream-format="avc", alignment="au" ! vtdec ! queue ! glimagesink 2> ~/gst.log Clip at https://dl.dropboxusercontent.com/u/4577541/interlaced_short.mts Digging in the log seems that skipped data like SEIs is interpreted as a DISCONT which seems a wrong behaviour. Other issues might be somewhere else in the vtenc or video decoder base class.
Created attachment 286992 [details] [review] attempt to fix the issue
The first part of the proposed changes is intended to include the AU delimiter NALU when and AU is collected instead of letting the parser skip it when dealing with the following AU. The second change is adding one to frame size, which I can't explain from were it comes, but there's exactly one byte still skipping which causes a DISCONT flag being set.
This issue is a regression as GStreamer 1.2 series doesn't exhibit this behavior.
Comment on attachment 286992 [details] [review] attempt to fix the issue The unexplained +1 on the size is from the h264 codec parser not setting the start code position properly for all NAL types, making the next iteration have to skip one byte. It isn't correct and should not be needed. I'll have another patch soon.
Actually, I wont. I give up - it's all a nasty mess. If AU delimiters are output, they must be the first thing in the next AU, according to 7.4.1.2.3 of the spec. They can't be gathered into the previous AU. The weird +1 thing comes from commit 6af387cd5ab2c946025e5499903e75ee87b063a9 - which tries to handle (valid) cases where the start code is only 3 bytes long, which makes the first loop on the next buffer skip 1 zero byte if there's a 4-byte start code. I don't think that detection code is entirely correct, but it shouldn't matter - it should be sufficient for the parser to detect a 3-byte start code, and generate 4-byte start codes where needed. The real problem is that it's skipping that first byte, which makes base_parse mark things as discont when they are not. We need a way to make base_parse advance without making things discont.
I agree with Jan. The detection code is not perfect (I wrote in the original bug comment that it doesn't cover all cases from 7.4.1.2.3) but making the parser skip one zero byte without marking the buffer as DISCONT would imho be less work than covering all cases from 7.4.1.2.3 in the NAL scanner.
Closing because it seems to be fixed in current master.