GNOME Bugzilla – Bug 726669
omxh264enc: Properly accumulate headers and provide them to the base class
Last modified: 2014-07-23 08:17:00 UTC
I don't know if this problem is BCM(rpi) openmax only but openmax encoder provide sps + pps buffer as separate buffer than follow with IDR buffer http://cgit.freedesktop.org/gstreamer/gst-omx/tree/omx/gstomxh264enc.c#n517 But gst_omx_video_find_nearest_frame assign one frame for SPS/PPS buffer and one for IDR and then we always miss one frame I don't know if we can simple ignore buffer with sps/pps and simple use gst_video_encoder_set_headers and gstvideoencoder handle/add sps/pps to next IDR buffer or we should do this in omxvideoenc? Any experience from other openmax ?
btw from my discusion with Christian König from AMD AMD openmax implementation generate one buffer with sps+pps+idr
SPS/PPS should be put into a buffer that goes in gst_video_encoder_set_headers(), or alternatively has to be merged with the actual frame buffer. finish_frame() always has to be a complete frame, not a part of it. Otherwise the book keeping in the encoder base class gets confused. Are you planning to work on that?
Looking into it now myself btw, at least for some time now
commit 6617d6f577e72cabbc299f68b6e92baefd6b7397 Author: Sebastian Dröge <sebastian@centricular.com> Date: Tue Jun 24 11:11:28 2014 +0200 omxh264enc: Don't let baseclass finish frames for SPS/PPS buffers Otherwise we a) send them twice, and b) finish a frame for something that does not even include a frame. https://bugzilla.gnome.org/show_bug.cgi?id=726669
it looks like it don't work on rpi omxh264enc inline-header=true ! 'video/x-h264, profile=high' ! h264parse ! fakesink PPS isn't properly parsed after your modification 0:00:02.049687894 4046 0x212b320 WARN codecparsers_h264 gsth264parser.c:1731:gst_h264_parse_pps: couldn't find associated sequence parameter set with id: 0 0:00:02.051182006 4046 0x212b320 WARN h264parse gsth264parse.c:553:gst_h264_parse_process_nal:<h264parse0> failed to parse PPS: 0:00:02.053579186 4046 0x212b320 WARN h264parse gsth264parse.c:957:gst_h264_parse_handle_frame:<h264parse0> no SPS/PPS yet, nal Type: 5 Slice IDR, Size: 3395 will be dropped
Created attachment 279305 [details] [review] RPI generate SPS, PPS and IDR,.. as separate OMXbuffers RPI generate SPS, PPS and IDR,.. as separate OMXbuffers AMD generate sps,pps and IDR as one single OMXbuffer code review plz
Review of attachment 279305 [details] [review]: Ah now it makes sense. Want to improve based on the comments below or should I finish that? ::: omx/gstomxh264enc.c @@ +85,3 @@ videoenc_class->set_format = GST_DEBUG_FUNCPTR (gst_omx_h264_enc_set_format); videoenc_class->get_caps = GST_DEBUG_FUNCPTR (gst_omx_h264_enc_get_caps); + videoenc_class->reset = GST_DEBUG_FUNCPTR (gst_omx_h264_enc_reset); You can just override GstVideoEncoder::reset here and then chain up to the parent class' reset. no need for a new vfunc :) @@ +554,3 @@ GST_READ_UINT32_BE (buf->omx_buf->pBuffer + buf->omx_buf->nOffset) == 0x00000001) { +#ifdef USE_OMX_TARGET_RPI I think this should be done for all platforms... but: We should parse the buffer and check if it is including a complete frame or not. If it does not include a complete frame we should accumulate all buffers for the headers, if it includes a complete frame we should just handle it like a frame.
plz finish it thx
I don't have any other device to compare if Qualcomm/samsung implement OMX encoding in similar way. And I can't find OMX FLAG for data only. http://cgit.freedesktop.org/gstreamer/gst-omx/tree/omx/openmax/OMX_Core.h#n393 And broadcom has extension for end of NAL data aaaaaaaaaa........
Basically you would assume that there are always one or more complete NALs in the buffer. And then use the codecparsers to parse them and check what their type is. Will work on that in the next days then unless you're faster.
commit 33e083fff6dffd179ee0f4e6a4c3a3b814ba3aa8 Author: Sebastian Dröge <sebastian@centricular.com> Date: Sun Jun 29 19:10:19 2014 +0200 omxh264enc: Properly accumulate headers and push before the next frame Fixes output of encoding on RPi, where each header buffer (SPS and PPS) is in a separate OMX buffer. https://bugzilla.gnome.org/show_bug.cgi?id=726669
This will not work on AMD openmax they have sps,pps and IDR in one buffer Whey we can't just forward all buffers to h264parse ? Is it really important to have headers in videoencoder? Do you have any other device with openmax?
AMD has the OMX_BUFFERFLAG_CODECCONFIG flag on a buffer that contains a complete frame and not just the codec config? That's wrong according to the spec: OpenMAX IL 1.1.2 Section 3.1.2.7.1: >>> The OMX_BUFFERFLAG_CODECCONFIG is an optional flag that is set by an output port when all bytes in the buffer form part or all of a set of codec specific configuration data. >>> Note the *all bytes* in there. It's important to set them as headers because of how the videoencoder baseclass can be requested to resend the headers.
Michal, can you confirm this?
Look at my conversation with amd in your email
Thanks, that doesn't really answer it but I'll ask Christian myself :)
Hi said that they generate SPS+PPS+IDR as one buffer :) I don't have HW to test it, sorry.
It's reported to work on AMD hardware too. AMD hardware correctly doesn't set the CODECCONFIG flag.