[Bug 749950] encoder: VP8: Add CBR Encoding mode

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=749950

Víctor Manuel Jáquez Leal <vjaquez at igalia.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #345391|none                        |reviewed
             status|                            |

--- Comment #14 from Víctor Manuel Jáquez Leal <vjaquez at igalia.com> ---
Review of attachment 345391:
 --> (https://bugzilla.gnome.org/review?bug=749950&attachment=345391)

Excepting what I just commented, it lgtm.... well, another nitpick, I would set
a more detailed commit log comment, saying a little bit more detailed what was
done.

Anyhow, as these are just small things, I'll update the patch and commit it.
Thanks.

::: gst-libs/gst/vaapi/gstvaapiencoder_vp8.c
@@ +300,3 @@
+    misc = GST_VAAPI_ENC_MISC_PARAM_NEW (RateControl, encoder);
+    if (!misc)
+      return FALSE;

I would keep the same code style as above and move this out from the scope.

@@ +303,3 @@
+
+    rate_control = misc->data;
+    memset (rate_control, 0, sizeof (VAEncMiscParameterRateControl));

There's no need to set to zero the structure, it's already done in
GST_VAAPI_ENC_MISC_PARAM_NEW

@@ +312,3 @@
+
+    gst_vaapi_enc_picture_add_misc_param (picture, misc);
+    gst_vaapi_codec_object_replace (&misc, NULL);

ditto

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.