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 785917 - vaapi: h264/h265: support MB(Macroblock) bitrate control
vaapi: h264/h265: support MB(Macroblock) bitrate control
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer-vaapi
git master
Other Linux
: Normal enhancement
: 1.13.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-08-07 02:37 UTC by Hyunjun Ko
Modified: 2017-09-13 09:18 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
libs: encoder: Add mbbrc property (4.76 KB, patch)
2017-08-07 08:39 UTC, Hyunjun Ko
none Details | Review
libs: encoder: h264/h265: adjust mbbrc to rate control params if CBR/VBR. (1.91 KB, patch)
2017-08-07 08:39 UTC, Hyunjun Ko
none Details | Review
libs: encoder: h264: Add mbbrc property (3.57 KB, patch)
2017-09-13 04:34 UTC, Hyunjun Ko
committed Details | Review
libs: encoder: h265: Add mbbrc property (3.70 KB, patch)
2017-09-13 04:34 UTC, Hyunjun Ko
committed Details | Review

Description Hyunjun Ko 2017-08-07 02:37:11 UTC
This can be supported by providing new property "mbbrc", which can have value 0(off) or 1(on).

Note that this can be adjusted when CBR/VBR not CQP mode.
Comment 1 Hyunjun Ko 2017-08-07 02:39:02 UTC
In addition, "auto" can be added, which can be determined according to quality level.
Comment 2 Hyunjun Ko 2017-08-07 08:39:01 UTC
Created attachment 357088 [details] [review]
libs: encoder: Add mbbrc property

This property supports Macroblock level Bitrate Control as the following:
0: auto
1: on
2: off
Comment 3 Hyunjun Ko 2017-08-07 08:39:26 UTC
Created attachment 357089 [details] [review]
libs: encoder: h264/h265: adjust mbbrc to rate control  params if CBR/VBR.
Comment 4 Víctor Manuel Jáquez Leal 2017-08-23 10:57:03 UTC
Review of attachment 357088 [details] [review]:

Adding this property in the base case all the encoders will expose it, though only H264 and H265 handle it. Is this something that we want? if so, why it is not set in gst_vaapi_encoder_reconfigure_internal() for all the encoders?

::: gst-libs/gst/vaapi/gstvaapiencoder.c
@@ +159,3 @@
+   * GstVaapiEncoder:mbbrc:
+   *
+   * Macroblock level bitrate cotnrol.

typo: control

@@ +166,3 @@
+      g_param_spec_uint ("mbbrc",
+          "Macroblock level Bitrate Control",
+          "Macroblock level Bitrate Control (0: auto, 1: on, 2: off)", 0, 2,

I prefer an enum type for this ON/OFF/AUTO
Comment 5 Hyunjun Ko 2017-09-12 06:10:43 UTC
(In reply to Víctor Manuel Jáquez Leal from comment #4)
> Review of attachment 357088 [details] [review] [review]:
> @@ +166,3 @@
> +      g_param_spec_uint ("mbbrc",
> +          "Macroblock level Bitrate Control",
> +          "Macroblock level Bitrate Control (0: auto, 1: on, 2: off)", 0, 2,
> 
> I prefer an enum type for this ON/OFF/AUTO

I intended to use same value as the driver. We should add map table or something like that if we change it.
If you meant that you agree with this, I'll be working on it, but I don't think it's worth doing so.
Comment 6 Hyunjun Ko 2017-09-12 06:34:00 UTC
(In reply to Víctor Manuel Jáquez Leal from comment #4)
> Review of attachment 357088 [details] [review] [review]:
> 
> Adding this property in the base case all the encoders will expose it,
> though only H264 and H265 handle it. Is this something that we want? if so,
> why it is not set in gst_vaapi_encoder_reconfigure_internal() for all the
> encoders?
> 
My bad.
mbbrc should be exposed only in h264/h265 encoder.
Comment 7 Hyunjun Ko 2017-09-13 04:34:23 UTC
Created attachment 359681 [details] [review]
libs: encoder: h264: Add mbbrc property

This property supports Macroblock level Bitrate Control as the 
following:
0: auto
1: on
2: off
Comment 8 Hyunjun Ko 2017-09-13 04:34:55 UTC
Created attachment 359682 [details] [review]
libs: encoder: h265: Add mbbrc property

This property supports Macroblock level Bitrate Control as the 
following (same as h264 encoder):
0: auto
1: on
2: off
Comment 9 Víctor Manuel Jáquez Leal 2017-09-13 09:16:53 UTC
Attachment 359681 [details] pushed as 6816d7c - libs: encoder: h264: Add mbbrc property
Attachment 359682 [details] pushed as e7c099b - libs: encoder: h265: Add mbbrc property