[Bug 778732] Encoders: add VBR feature for AVC/VP8/HEVC/VP9

[GStreamer (GNOME Bugzilla)]

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

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

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

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

a couple comments

::: gst-libs/gst/vaapi/gstvaapiencoder_h265.c
@@ +1579,2 @@
     pic_param->pic_fields.bits.cu_qp_delta_enabled_flag = TRUE;
+

since coding unit quantization parameter changes are required for
non-constant-QP modes, I would rewrite this as

pic_param->pic_fields.bits.cu_qp_delta_enabled_flag =
    GST_VAAPI_ENCODER_RATE_CONTROL (encoder) != GST_VAAPI_RATECONTROL_CQP;

@@ +1805,3 @@
+    memset (rate_control, 0, sizeof (VAEncMiscParameterRateControl));
+    rate_control->bits_per_second = encoder->bitrate_bits;
+    rate_control->target_percentage = 70;

I'm a bit worried about this target_percentage, the hard-coded value (70) it is
copied again and again like a some kind of cargo cult.

    /* this is the bit-rate the rate control is targeting, as a percentage of
the maximum
     * bit-rate for example if target_percentage is 95 then the rate control
will target
     * a bit-rate that is 95% of the maximum bit-rate
     */

looking at the ffmpeg implementation of the vaapi encoder, the
target_percentage is 100% if the rate-control is CBR. Otherwise a ad-hoc
formula is used.

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