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 779655 - h264parse: Does not always produce valid AVC stream
h264parse: Does not always produce valid AVC stream
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal normal
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-03-06 14:53 UTC by Vincent Penquerc'h
Modified: 2018-11-03 14:05 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
qtmux: do not require PTS on a header buffer (924 bytes, patch)
2017-03-06 14:53 UTC, Vincent Penquerc'h
none Details | Review
Stream the partially reproduce the issues (152.02 KB, application/gzip)
2017-03-07 17:31 UTC, Nicolas Dufresne (ndufresne)
  Details
GDP Of the stream (226.71 KB, application/octet-stream)
2017-03-07 19:16 UTC, Nicolas Dufresne (ndufresne)
  Details
Patch against gst-omx to workaround the issue (1.77 KB, patch)
2017-03-07 19:27 UTC, Nicolas Dufresne (ndufresne)
none Details | Review

Description Vincent Penquerc'h 2017-03-06 14:53:11 UTC
Created attachment 347318 [details] [review]
qtmux: do not require PTS on a header buffer

qtmux: do not require PTS on a header buffer
Comment 1 Tim-Philipp Müller 2017-03-06 17:19:05 UTC
Could you describe the exact scenario this fixes? What codec etc?
Comment 2 Vincent Penquerc'h 2017-03-06 17:27:44 UTC
In this particular case, this fixes qtmux erroring out with "Buffer has no PTS." on an encoding pipeline with a proprietary OMX H264 encoder with a pipeline like:

gst-launch-1.0 cam3src num-buffers=120 ! video/x-raw,framerate=30/1 ! omxh264enc ! h264parse ! mp4mux ! filesink location=/tmp/out.mp4

(the source is also proprietary and sends YUV)
Comment 3 Tim-Philipp Müller 2017-03-06 17:34:55 UTC
And what's in this header buffer? ( ... ! identity dump=true ! ...)

I'm just asking because it seems strange that it receives one of these in the first place, since qtmux takes AVC and full AUs, so headers should be in codec_data in the caps.
Comment 4 Vincent Penquerc'h 2017-03-06 17:37:26 UTC
00000000 (0x1732a0): 00 00 00 07 41 e2 02 00 01 2c 18                 ....A....,.     

I don't know the H264 format, but hopefully you might :)
Comment 5 Nicolas Dufresne (ndufresne) 2017-03-07 01:57:43 UTC
My impression on this is that in AVC, there should never be a HEADER flag set on any of the buffers. All headers should be put in the CODEC data. So this looks like an H264 parse bug.

If I look at what omxhh264enc produces, with Qualcomm CODEC, it looks like:

  - empty buffer
  - 30 bytes nal
  - empty buffer
  - more nals
  - A NAL with CODECCONFIG flag
  - The IDR NAL

The encoder code will aggregate all buffers the has CODECCONFIG (skipping the empty one), and prepand (1 buffer 1 push) them to the first frame. Now, first issue, the CODECCONFIG flag is not set properly by the component. I can work around that by assuming that if frame == NULL, then it's a CODECCONFIG, a bit bugus, but works for this one.

But then the second problem, is that all those headers have no timestamp (first timestamp is set on the IDR). h264parse accumulate the data, but instead of looking at the IDR NAL timestamp, it seems to pick the first seen timestamp. I think this need to be improved in the parser. We can also work around this by updating all headers timestamp before calling gst_video_encoder_set_headers().
Comment 6 Sebastian Dröge (slomo) 2017-03-07 10:58:24 UTC
There are various variants of "avc", 4 IIRC. avc1 to avc4. And they differ in where the SPS/PPS are, and for some they can be in-band.

What we call "avc" however is where all headers are out of band (note how h264parse also supports "avc3").


So needs some further investigation here. The patch as is does not seem to make sense though.
Comment 7 Tim-Philipp Müller 2017-03-07 11:13:19 UTC
qtmux/mp4mux only support avc1 though at the moment, as far as I'm aware.

Most likely a bug in h264parse or the encoder imho, as Nicolas said already.
Comment 8 Nicolas Dufresne (ndufresne) 2017-03-07 17:31:56 UTC
Created attachment 347411 [details]
Stream the partially reproduce the issues

I've captured the stream to try and reproduce the problem without the encoder. We can partially reproduce it. We don't get this 30 bytes frame that causes further issues, and we also can't reproduce the timestamp issue, but we can notice that first frame flags are from, 0x440, which TAG_MEMORY (that's fine) and GST_BUFFER_FLAG_HEADER, which in AVC would be a lie, since it should only contain the IDR iirc.

GST_DEBUG="h264parse:7,codecparsers_h264:7,*SCHED*:7" gst-launch-1.0 multifilesrc location="%04d.h264" ! h264parse ! video/x-h264,stream-format=avc ! fakesink 2>&1 | less -RS

I'll try and find another way to capture the rest of the issues, with the DELTA_UNIT set on the first frame, and bad TS.
Comment 9 Tim-Philipp Müller 2017-03-07 17:34:02 UTC
Maybe you could capture a few frames of the output of your encoder with 

  ... ! omxh264enc ! gdppay ! filesink location=omxh264enc.gdp

and check if it can be reproduced that way?
Comment 10 Nicolas Dufresne (ndufresne) 2017-03-07 19:16:44 UTC
Created attachment 347415 [details]
GDP Of the stream

I've revert back to the original omxh264enc code, it apparently makes the timestamp re-appear. And then notice that the base class will set TS base on the OMX TS.

If you look at this stream, you'll notice that h264parse failed to fix the buffer flags properly, all frames are marked as delta units. There might be other issue I don't see.

The encoder is still a bit faulty, I think it sets OMX CODECCONFIG flags on the IDR, rather then on the bits and pieces that forms the sps/pps.
Comment 11 Nicolas Dufresne (ndufresne) 2017-03-07 19:27:28 UTC
Created attachment 347416 [details] [review]
Patch against gst-omx to workaround the issue

In this OMX patch, I set timestamps on the headers, and assume that anything that has no associated frame is a header. This seems to make it work properly, mp4mux files are now playable. It should not have been required though.
Comment 12 GStreamer system administrator 2018-11-03 14:05:19 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-bad/issues/528.