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 698049 - omxh264enc: openmax API ignores output bitrate
omxh264enc: openmax API ignores output bitrate
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-omx
1.0.7
Other Linux
: Normal normal
: 1.2.0
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2013-04-15 10:30 UTC by m][sko
Modified: 2014-07-23 08:27 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description m][sko 2013-04-15 10:30:14 UTC
omxh264enc has input parameter target-bitrate
But it looks like raspberry pi openmax API ignore it
as it don't generate stream with proper bitrate

If you change target-bitrate to any value 
output bitrate is always same


all fine in this example
https://github.com/dickontoo/omxtx/blob/master/omxtx.c

test pipeline

gst-launch-1.0 -v uridecodebin uri="file:///home/pi/test2/input.ts" name=demux ! video/x-raw, format="I420", framerate=25/1, width=720, height=576 ! omxh264enc target-bitrate=500000 control-rate=2 ! video/x-h264, profile="high" ! mpegtsmux name=mux ! filesink location=output.ts demux. ! audio/x-raw, format="S16LE", rate=48000, channels=2 ! voaacenc bitrate=96000 ! mux.
Comment 1 Sebastian Dröge (slomo) 2013-04-16 09:03:47 UTC
Does it work if you additionally set control-rate=variable? By default it uses the component's default, which might be "disabled".
Comment 2 m][sko 2013-04-16 11:50:24 UTC
I check it with my debugs logs and gst-omx call openmax api that set up bitrate and control-rate
I changed it to constant and variable
nothing happen
Comment 3 m][sko 2013-06-06 20:22:34 UTC
I found solution
it looks like rpi openmax api need to specify targetbitrate on output port 

static gboolean
gst_omx_video_enc_set_format (GstVideoEncoder * encoder,
    GstVideoCodecState * state)
{

...........

  if (klass->set_format) {
    if (!klass->set_format (self, self->enc_in_port, state)) {
      GST_ERROR_OBJECT (self, "Subclass failed to set the new format");
      return FALSE;
    }
  }

  GST_DEBUG_OBJECT (self, "Updating outport port definition");
  if (gst_omx_port_update_port_definition (self->enc_out_port,
          NULL) != OMX_ErrorNone)
    return FALSE;


  if(self->target_bitrate != 0xffffffff){
	  GST_DEBUG_OBJECT (self, "Set up output bitrate");
	  self->enc_out_port->port_def.format.video.nBitrate = self->target_bitrate;
	  if (gst_omx_port_update_port_definition (self->enc_out_port,
			  &self->enc_out_port->port_def) != OMX_ErrorNone)
	    return FALSE;
  }
Comment 4 m][sko 2013-06-06 20:27:28 UTC
Now I think that we will need add same fix in function
static void
gst_omx_video_enc_set_property (GObject * object, guint prop_id,
    const GValue * value, GParamSpec * pspec)
Comment 5 Sebastian Dröge (slomo) 2013-06-07 09:15:55 UTC
Thanks, can you provide a real patch for this? It should only be done when initially updating the port definition, i.e. in gst_omx_video_enc_open() additional to the use of OMX_IndexParamVideoBitrate
Comment 6 Roman Arutyunyan 2013-06-20 10:02:44 UTC
Here's the patch that works for me
http://pastebin.com/8wvTaakv

Remember you should always specify both target-bitrate and control-rate.

omxh264enc target-bitrate=1000000 control-rate=variable
Comment 7 Sebastian Dröge (slomo) 2013-07-01 08:10:21 UTC
Is the first part of the patch really required? What happens if that code in set_property is called?
Comment 8 Sebastian Dröge (slomo) 2013-07-01 08:11:35 UTC
Arguably it's also a bug in the RPi gst-omx that it requires nBitrate to be set. To me it looks like this is supposed to be read-only.
Comment 9 Sebastian Dröge (slomo) 2013-07-01 08:14:21 UTC
https://github.com/raspberrypi/firmware/issues/193
Comment 10 Roman Arutyunyan 2013-07-01 09:52:47 UTC
(In reply to comment #7)
> Is the first part of the patch really required? What happens if that code in
> set_property is called?

There was an error like Bad Parameter, I'll try again later and post the exact error.
Comment 11 Roman Arutyunyan 2013-07-01 13:31:10 UTC
I've checked that again. It looks like I was wrong, it works without the first part of patch. In fact self->enc is NULL (component is not created yet) so that code is never executed.

So m][sko's code is enough for setting bitrate:
http://pastebin.com/2TUNyPef

Moreover I've found another way to set bitrate through OMX_IndexParamVideoBitrate parameter. This patch works good as well:
http://pastebin.com/jwAbMfv7

In original gst_omx code this parameter is set in gst_omx_video_enc_open but it has no effect since it seems to be lost if set before set_format call.
Comment 12 m][sko 2013-07-01 13:41:01 UTC
Do you have any plan sebastian how to add more parameters to omx?
every encoder have some specific parameters
I would like to add VDR frame interval that exist only in H264

I don't like current "caps solution" for encoder properties and I don't want to extend this list.
Comment 13 Sebastian Dröge (slomo) 2013-07-01 13:50:02 UTC
That new change definitely makes sense, is everything working as expected with this now?

commit 5ba55b6c9aad49858304a38e0ab475c1ecafd646
Author: Roman Arutyunyan <arutyunyan.roman@gmail.com>
Date:   Mon Jul 1 15:48:47 2013 +0200

    gstomxvideoenc: Set bitrate in setcaps
    
    Otherwise it gets lost whenever we configure new caps
    
    https://bugzilla.gnome.org/show_bug.cgi?id=698049
Comment 14 Sebastian Dröge (slomo) 2013-07-01 13:50:57 UTC
(In reply to comment #12)
> Do you have any plan sebastian how to add more parameters to omx?
> every encoder have some specific parameters
> I would like to add VDR frame interval that exist only in H264
> 
> I don't like current "caps solution" for encoder properties and I don't want to
> extend this list.

Which caps solution do you mean? Only profile and level are negotiated via the caps, other things will be handled by properties. Adding new properties for specific video codecs can easily be done in the classes for them, I don't plan on adding any right now but would accept patches to do so.
Comment 15 Roman Arutyunyan 2013-07-01 13:51:16 UTC
Here's the real reason of the problem.

After setting compression format eCompressionFormat = OMX_VIDEO_CodingAVC with port definition update in gst_omx_h264_enc_set_format all bitrate settings are lost. That's the way Raspberry Pi Openmax implementation works. It seems other implementations work differently.
Comment 16 Roman Arutyunyan 2013-07-01 14:51:25 UTC
(In reply to comment #13)
> That new change definitely makes sense, is everything working as expected with
> this now?

Yes