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 775962 - omxh264enc: Occassional addition of codec_data to caps when stream-format: byte-stream
omxh264enc: Occassional addition of codec_data to caps when stream-format: by...
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gst-omx
git master
Other Mac OS
: Normal normal
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-12-11 22:11 UTC by minfrin
Modified: 2018-11-03 13:00 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Log clearly when the stream is aborted due to a refusal to negotiate codec_data in the caps. (996 bytes, patch)
2016-12-11 22:19 UTC, minfrin
reviewed Details | Review
Use caps instead of stream contents when detecting a byte-stream. (3.08 KB, patch)
2016-12-18 19:42 UTC, minfrin
none Details | Review
Fix faulty bytestream detection when headers are fragmented (1.97 KB, patch)
2017-01-01 22:01 UTC, minfrin
none Details | Review

Description minfrin 2016-12-11 22:11:51 UTC
When attempting to transcode a highly lossy MPEG transport stream using omxh264dec followed by omxh264enc, from time to time codec_data will be added to the caps when stream-format is byte-stream.

This triggers a downstream negotiation failure, which terminates the transcode.

0:55:35.355504114  9358 0x74b01b80 DEBUG           videoencoder gstvideoencoder.c:2117:gst_video_encoder_finish_frame:<omxh264enc-omxh264enc0> Sending headers
0:55:35.355882808  9358 0x74b01b80 DEBUG            omxvideoenc gstomxvideoenc.c:762:gst_omx_video_enc_loop:<omxh264enc-omxh264enc0> Finished frame: not-negotiated
0:55:35.356075305  9358 0x74b01b80 DEBUG            omxvideoenc gstomxvideoenc.c:770:gst_omx_video_enc_loop:<omxh264enc-omxh264enc0> Read frame from component
0:55:35.356120045  9358 0x74b01b80 WARN             omxvideoenc gstomxvideoenc.c:843:gst_omx_video_enc_loop:<omxh264enc-omxh264enc0> error: Internal data stream error.
0:55:35.356148690  9358 0x74b01b80 WARN             omxvideoenc gstomxvideoenc.c:843:gst_omx_video_enc_loop:<omxh264enc-omxh264enc0> error: stream stopped, reason not-negotiated
0:55:35.402848003  9358 0x75528030 DEBUG            omxvideoenc gstomxvideoenc.c:1460:gst_omx_video_enc_handle_frame:<omxh264enc-omxh264enc0> Handling frame
ERROR: from element /GstPipeline:pipeline0/GstTranscoder:transcoder/GstEncodeBin:encodebin0/GstOMXH264Enc-omxh264enc:omxh264enc-omxh264enc0: Internal data stream error.
Additional debug info:
gstomxvideoenc.c(843): gst_omx_video_enc_loop (): /GstPipeline:pipeline0/GstTranscoder:transcoder/GstEncodeBin:encodebin0/GstOMXH264Enc-omxh264enc:omxh264enc-omxh264enc0:
stream stopped, reason not-negotiated
Execution ended after 0:55:35.377031725
Setting pipeline to PAUSED ...
Setting pipeline to READY ...

The decision on whether to inline the codec_data or add it to the caps is made here in gst_omx_h264_enc_handle_output_frame():

  if (buf->omx_buf->nFlags & OMX_BUFFERFLAG_CODECCONFIG) {
    /* The codec data is SPS/PPS with a startcode => bytestream stream format
     * For bytestream stream format the SPS/PPS is only in-stream and not
     * in the caps!
     */
    if (buf->omx_buf->nFilledLen >= 4 &&
        GST_READ_UINT32_BE (buf->omx_buf->pBuffer +
            buf->omx_buf->nOffset) == 0x00000001) {
// inline, else caps

From time to time this if statement is false when it should be true.

This check seems bogus - if the buffer is too short, or if a particular value is exactly 0x00000001, we inline, otherwise we falls back to the buggy behaviour. The lossy stream might account for broken data.

Ideally this check should be based on the caps, not the content of the stream (which could be corrupt / missing) - does this make sense?
Comment 1 minfrin 2016-12-11 22:19:02 UTC
Created attachment 341784 [details] [review]
Log clearly when the stream is aborted due to a refusal to  negotiate codec_data in the caps.

Make sure we log clearly when this bug occurs.
Comment 2 Tim-Philipp Müller 2016-12-11 23:02:00 UTC
I think this check makes sense in principle. Could you dump the bytes when the check fails and it subsequently errors out?
Comment 3 Tim-Philipp Müller 2016-12-11 23:02:42 UTC
(e.g. with gst_util_dump_mem(buf->omx_buf->pBuffer + buf->omx_buf->nOffset, buf->omx_buf->nFilledLen))
Comment 4 Tim-Philipp Müller 2016-12-11 23:03:29 UTC
Maybe it sometimes decides to do a shorter 3-byte start code instead (00 00 01).
Comment 5 minfrin 2016-12-12 15:25:23 UTC
I tried as follows:

diff --git a/omx/gstomxh264enc.c b/omx/gstomxh264enc.c
index aa33ae5..1996610 100644
--- a/omx/gstomxh264enc.c
+++ b/omx/gstomxh264enc.c
@@ -546,6 +595,16 @@ gst_omx_h264_enc_handle_output_frame (GstOMXVideoEnc * enc, GstOMXPort * port,
   GstOMXH264Enc *self = GST_OMX_H264_ENC (enc);
 
   if (buf->omx_buf->nFlags & OMX_BUFFERFLAG_CODECCONFIG) {
+
+    if (buf->omx_buf->nFilledLen >= 4) {
+      GST_ERROR_OBJECT (self, "OMX_BUFFERFLAG_CODECCONFIG: length %d, bytes %08x", buf->omx_buf->nFilledLen, GST_READ_UINT32_BE (buf->omx_buf->pBuffer +
+              buf->omx_buf->nOffset));
+    }
+    else {
+      GST_ERROR_OBJECT (self, "OMX_BUFFERFLAG_CODECCONFIG: length %d", buf->omx_buf->nFilledLen);
+    }
+
+
     /* The codec data is SPS/PPS with a startcode => bytestream stream format
      * For bytestream stream format the SPS/PPS is only in-stream and not
      * in the caps!

And got results as follows:

0:06:56.050027453 24259 0x73fe0920 ERROR             omxh264enc gstomxh264enc.c:601:gst_omx_h264_enc_handle_output_frame:<omxh264enc-omxh264enc0> OMX_BUFFERFLAG_CODECCONFIG: length 9, bytes 00000001
0:06:56.072296135 24259 0x73fe0920 ERROR             omxh264enc gstomxh264enc.c:601:gst_omx_h264_enc_handle_output_frame:<omxh264enc-omxh264enc0> OMX_BUFFERFLAG_CODECCONFIG: length 19, bytes 00000001
0:06:56.072925658 24259 0x73fe0920 ERROR             omxh264enc gstomxh264enc.c:601:gst_omx_h264_enc_handle_output_frame:<omxh264enc-omxh264enc0> OMX_BUFFERFLAG_CODECCONFIG: length 9, bytes 00000001
0:06:56.111636218 24259 0x73fe0920 ERROR             omxh264enc gstomxh264enc.c:601:gst_omx_h264_enc_handle_output_frame:<omxh264enc-omxh264enc0> OMX_BUFFERFLAG_CODECCONFIG: length 19, bytes 00000001
0:06:56.112307095 24259 0x73fe0920 ERROR             omxh264enc gstomxh264enc.c:601:gst_omx_h264_enc_handle_output_frame:<omxh264enc-omxh264enc0> OMX_BUFFERFLAG_CODECCONFIG: length 9, bytes 00000001
0:06:56.131532481 24259 0x73fe0920 ERROR             omxh264enc gstomxh264enc.c:601:gst_omx_h264_enc_handle_output_frame:<omxh264enc-omxh264enc0> OMX_BUFFERFLAG_CODECCONFIG: length 12, bytes 00000001
0:06:56.132180754 24259 0x73fe0920 ERROR             omxh264enc gstomxh264enc.c:601:gst_omx_h264_enc_handle_output_frame:<omxh264enc-omxh264enc0> OMX_BUFFERFLAG_CODECCONFIG: length 7, bytes 09361803

// we're definitely not 00000001, and therefore...

/GstPipeline:pipeline0/GstTranscoder:transcoder/GstEncodeBin:encodebin0/GstOMXH264Enc-omxh264enc:omxh264enc-omxh264enc0.GstPad:src: caps = video/x-h264, stream-format=(string)byte-stream, alignment=(string)au, profile=(string)high, level=(string)4, width=(int)544, height=(int)576, pixel-aspect-ratio=(fraction)64/33, framerate=(fraction)25/1, codec_data=(buffer)09361803c489a8, interlace-mode=(string)progressive, colorimetry=(string)bt601, chroma-site=(string)jpeg
/GstPipeline:pipeline0/GstTranscoder:transcoder/GstEncodeBin:encodebin0/GstStreamCombiner:streamcombiner0.GstPad:src: caps = video/x-h264, stream-format=(string)byte-stream, alignment=(string)au, profile=(string)high, level=(string)4, width=(int)544, height=(int)576, pixel-aspect-ratio=(fraction)64/33, framerate=(fraction)25/1, codec_data=(buffer)09361803c489a8, interlace-mode=(string)progressive, colorimetry=(string)bt601, chroma-site=(string)jpeg
0:06:56.134679004 24259 0x73fe0920 ERROR             omxh264enc gstomxh264enc.c:601:gst_omx_h264_enc_handle_output_frame:<omxh264enc-omxh264enc0> OMX_BUFFERFLAG_CODECCONFIG: length 9, bytes 00000001
/GstPipeline:pipeline0/GstTranscoder:transcoder/GstEncodeBin:encodebin0/GstStreamCombiner:streamcombiner0.GstStreamCombinerPad:encodingsink: caps = video/x-h264, stream-format=(string)byte-stream, alignment=(string)au, profile=(string)high, level=(string)4, width=(int)544, height=(int)576, pixel-aspect-ratio=(fraction)64/33, framerate=(fraction)25/1, codec_data=(buffer)09361803c489a8, interlace-mode=(string)progressive, colorimetry=(string)bt601, chroma-site=(string)jpeg
ERROR: from element /GstPipeline:pipeline0/GstTranscoder:transcoder/GstEncodeBin:encodebin0/GstOMXH264Enc-omxh264enc:omxh264enc-omxh264enc0: Internal data stream error.
Additional debug info:
gstomxvideoenc.c(845): gst_omx_video_enc_loop (): /GstPipeline:pipeline0/GstTranscoder:transcoder/GstEncodeBin:encodebin0/GstOMXH264Enc-omxh264enc:omxh264enc-omxh264enc0:
stream stopped, reason not-negotiated
Execution ended after 0:06:55.163305257
Setting pipeline to PAUSED ...
Setting pipeline to READY ...
Setting pipeline to NULL ...
Freeing pipeline ...
Comment 6 minfrin 2016-12-12 15:44:52 UTC
To point out the pattern, it looks like we're expecting a 19 byte packet, but we're getting the 19 bytes split across 12 bytes then 7 bytes:

0:06:56.131532481 24259 0x73fe0920 ERROR             omxh264enc gstomxh264enc.c:601:gst_omx_h264_enc_handle_output_frame:<omxh264enc-omxh264enc0> OMX_BUFFERFLAG_CODECCONFIG: length 12, bytes 00000001
0:06:56.132180754 24259 0x73fe0920 ERROR             omxh264enc gstomxh264enc.c:601:gst_omx_h264_enc_handle_output_frame:<omxh264enc-omxh264enc0> OMX_BUFFERFLAG_CODECCONFIG: length 7, bytes 09361803
Comment 7 minfrin 2016-12-12 15:52:16 UTC
Looking further at the omxh264enc code,

static GstCaps *
gst_omx_h264_enc_get_caps (GstOMXVideoEnc * enc, GstOMXPort * port,
    GstVideoCodecState * state)
{
  GstOMXH264Enc *self = GST_OMX_H264_ENC (enc);
  GstCaps *caps;
  OMX_ERRORTYPE err;
  OMX_VIDEO_PARAM_PROFILELEVELTYPE param;
  const gchar *profile, *level;

  caps = gst_caps_new_simple ("video/x-h264",
      "stream-format", G_TYPE_STRING, "byte-stream",
      "alignment", G_TYPE_STRING, "au", NULL);

We seem to hardcode ourselves to stream-format = "byte-stream", are we always byte-stream or could someone negotiate avc if they wanted to?
Comment 8 Tim-Philipp Müller 2016-12-12 16:08:43 UTC
Ah, so it *sometimes* gives us SPS/PPS in different buffers. Not quite sure why the behaviour is not consistent.

Most hardware encoders output byte-stream, and I suspect omx spec might prescribe that too (but I haven't checked).
Comment 9 Sebastian Dröge (slomo) 2016-12-13 09:35:29 UTC
It does not define if it's AVC or byte-stream unfortunately, but I can't remember seeing AVC ever. Except for maybe with bellagio, but that's hopelessly broken anyway.
Comment 10 minfrin 2016-12-18 19:42:58 UTC
Created attachment 342181 [details] [review]
Use caps instead of stream contents when detecting a  byte-stream.

Use caps instead of stream contents when detecting a
 byte-stream. Fixes an accasional codec_data being negotiated in the caps in a
 byte-stream when a config packet is fragmented.
Comment 11 Tim-Philipp Müller 2016-12-19 07:07:15 UTC
Comment on attachment 341784 [details] [review]
Log clearly when the stream is aborted due to a refusal to  negotiate codec_data in the caps.

We don't know why exactly downstream refused the caps. It could be all kinds of reasons. I'd just log it as

  "Downstream element refused caps %" GST_PTR_FORMAT, caps

or so (didn't check if there's a caps var here though).
Comment 12 Sebastian Dröge (slomo) 2016-12-19 09:00:52 UTC
Review of attachment 342181 [details] [review]:

::: omx/gstomxh264enc.c
@@ +553,3 @@
      * in the caps!
      */
+    caps = gst_pad_get_current_caps (GST_VIDEO_ENCODER_SRC_PAD (self));

Instead of parsing the caps on every codecconfig buffer, it would make more sense to remember if it was byte-stream or not when the caps were set.

Otherwise makes sense

@@ +575,3 @@
+          memcpy (map.data,
+              buf->omx_buf->pBuffer + buf->omx_buf->nOffset,
+              buf->omx_buf->nFilledLen);

Could use gst_buffer_fill() here
Comment 13 minfrin 2017-01-01 22:01:46 UTC
Created attachment 342701 [details] [review]
Fix faulty bytestream detection when headers are fragmented

It turns out that we hardcode the caps to bytestream, and so trying to detect whether the caps is bytestream is pointless.

Switch the memcpy to gst_buffer_fill - can you verify I did this correctly?

(I have found a new bug where force-key-unit messages are getting lost for no obvious reason, and therefore cannot test this in my original configuration)
Comment 14 Robert Mast 2017-02-12 22:01:49 UTC
I also saw something alike with first decoding an MPEG-stream instead of H264 before encoding H264. (RPi3, OSMC, GST-Versions 1.10.3, except for libvpx, which is 1.5.0)

I implemented both these patches and I still see no moving picture. There were many non-breaking Error-messages 'No corresponding frame found' in the log, but at least the pipeline kept running.
I couldn't find an open bug at 'No corresponding frame found', so I'm not sure it's a bug or just my inexperience.

gst-launch-1.0  --gst-debug omx*:5 souphttpsrc location="http://127.0.0.1:9981/stream/channel/41ceb7b6ef96f869d7d6c78b6b3a27a6?ticket=24A946694272F14A9994F5F2280E466AB495F745?profile=pass" num-buffers=-1 ! tsdemux name=demux demux. ! queue ! mpegvideoparse ! omxmpeg2videodec ! videoconvert ! omxh264enc target-bitrate=2097152 control-rate=variable ! video/x-h264,stream-format=byte-stream,profile=high,width=720,height=576,framerate=25/1 ! h264parse ! matroskamux name=stream streamable=true demux.audio_0_07dc ! queue ! mpegaudioparse ! mpg123audiodec ! audioconvert dithering=0 ! audio/x-raw,channels=2 ! voaacenc bitrate=32768 ! udpsink port=5000
Comment 15 Guillaume Desmottes 2017-10-16 09:21:00 UTC
Review of attachment 342701 [details] [review]:

Looks good to me. Can we merge?
Comment 16 Guillaume Desmottes 2017-10-18 12:29:40 UTC
I tested the patch on the pi and it still detects the codecconfig.
Comment 17 GStreamer system administrator 2018-11-03 13:00:54 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-omx/issues/9.