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 785923 - vaapi encoder: support to seperate QP setting for I/P/B frame
vaapi encoder: support to seperate QP setting for I/P/B frame
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 05:38 UTC by Hyunjun Ko
Modified: 2017-09-18 19:44 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
libs: encoder: h264/5: don't change the value of min_qp (4.56 KB, patch)
2017-08-17 09:52 UTC, Hyunjun Ko
none Details | Review
libs: encoder: h264: support I/P/B QP setting seperatedly (4.93 KB, patch)
2017-08-17 09:56 UTC, Hyunjun Ko
none Details | Review
libs: encoder: h265: support I/P/B QP setting seperatedly (4.92 KB, patch)
2017-08-21 06:53 UTC, Hyunjun Ko
none Details | Review
libs: encoder: h264/h265: keep min_qp as is unless it's over init_qp (4.60 KB, patch)
2017-09-13 04:36 UTC, Hyunjun Ko
committed Details | Review
libs: encoder: h264: support I/P/B QP setting seperatedly (4.99 KB, patch)
2017-09-13 04:37 UTC, Hyunjun Ko
committed Details | Review
libs: encoder: h265: support I/P/B QP setting seperatedly (4.93 KB, patch)
2017-09-13 04:37 UTC, Hyunjun Ko
committed Details | Review

Description Hyunjun Ko 2017-08-07 05:38:41 UTC
Support to seperate QP setting to each I/P/B frame.
2 new properties could be provided for this feature, which might be named as "diff-qp-ip(difference of QP between I and P)" and "idff-qp-ib(difference of QP between I and B)"

Suggestion for the name would be welcome!
Comment 1 sreerenj 2017-08-07 22:21:03 UTC
(In reply to Hyunjun Ko from comment #0)
> Support to seperate QP setting to each I/P/B frame.
> 2 new properties could be provided for this feature, which might be named as
> "diff-qp-ip(difference of QP between I and P)" and "idff-qp-ib(difference of
> QP between I and B)"
> 
> Suggestion for the name would be welcome!

Why diff-qp-FrameTypeX instead of qp-FrameTypeX ??
Comment 2 Hyunjun Ko 2017-08-08 00:34:10 UTC
(In reply to sreerenj from comment #1)
> (In reply to Hyunjun Ko from comment #0)
> > Support to seperate QP setting to each I/P/B frame.
> > 2 new properties could be provided for this feature, which might be named as
> > "diff-qp-ip(difference of QP between I and P)" and "idff-qp-ib(difference of
> > QP between I and B)"
> > 
> > Suggestion for the name would be welcome!
> 
> Why diff-qp-FrameTypeX instead of qp-FrameTypeX ??

It's just to be clear to users.

If we allow to set QP value instead of difference, I think we can name just qp-Frametype. (maybe qp-p and qp-b?)
But imho, value of difference would be simple for user.

Example pipeline of what I thought:
raw data ! vaapih264enc (init-qp=26) diff-qp-ip=-2 diff-qp-ib=-3 ! fakesink
same as
raw data ! vaapih264enc (init-qp=26) qp-p=24 qp-b=23 ! fakesink

But if we think documentation would be enough, we can use qp-ip and qp-ib for the value of difference like the below:
raw data ! vaapih264enc (init-qp=26) qp-ip=-2 qp-ib=-3 ! fakesink

So do you mean you prefer the last one?
Comment 3 Hyunjun Ko 2017-08-17 09:52:14 UTC
Created attachment 357785 [details] [review]
libs: encoder: h264/5: don't change the value of min_qp

Creates new variable for QP for I frame and keep it at configuration and 
use this for pic_init_qp and slice_qp_delta setting.
Since changing min qp doesn't make sense, keep min qp as is.
Comment 4 Hyunjun Ko 2017-08-17 09:56:13 UTC
Created attachment 357786 [details] [review]
libs: encoder: h264: support I/P/B QP setting seperatedly

Creates 2 properties, qp-ip and qp-ib for setting different QP for P/B frames
and set slice_qp_delta for each frame according to the value provided.
In addition, remove the limitation of (<= 4) when setting slice_qp_delta.
Comment 5 Hyunjun Ko 2017-08-21 06:53:23 UTC
Created attachment 358049 [details] [review]
libs: encoder: h265: support I/P/B QP setting seperatedly

Creates 2 properties, qp-ip and qp-ib for setting different QP for P/B frames
and set slice_qp_delta for each frame according to the value provided.
Comment 6 Víctor Manuel Jáquez Leal 2017-08-23 11:33:46 UTC
Review of attachment 357786 [details] [review]:

::: gst-libs/gst/vaapi/gstvaapiencoder_h264.c
@@ +3231,3 @@
+   *
+   * The difference of QP between I and P Frame.
+   * This can be adjusted on CQP mode.

If I understand correctly, these properties are only used if CQP mode is used. We might say also that here.
Comment 7 Víctor Manuel Jáquez Leal 2017-08-23 11:34:05 UTC
Review of attachment 357785 [details] [review]:

lgtm
Comment 8 Víctor Manuel Jáquez Leal 2017-08-23 11:34:33 UTC
Review of attachment 358049 [details] [review]:

::: gst-libs/gst/vaapi/gstvaapiencoder_h265.c
@@ +2626,3 @@
+   *
+   * The difference of QP between I and P Frame.
+   * This can be adjusted on CQP mode.

same as in H264
Comment 9 Hyunjun Ko 2017-09-13 04:36:16 UTC
Created attachment 359683 [details] [review]
libs: encoder: h264/h265: keep min_qp as is unless it's  over init_qp

Creates new variable for QP for I frame and keep it at configuration and 
use this for pic_init_qp and slice_qp_delta setting.

Since changing min qp doesn't make sense, keep min qp as is.
Comment 10 Hyunjun Ko 2017-09-13 04:37:05 UTC
Created attachment 359684 [details] [review]
libs: encoder: h264: support I/P/B QP setting seperatedly

Creates 2 properties, qp-ip and qp-ib for setting different QP for P/B 
frames
and set slice_qp_delta for each frame according to the value provided.
In addition, remove the limitation of (<= 4) when setting
slice_qp_delta.
Comment 11 Hyunjun Ko 2017-09-13 04:37:37 UTC
Created attachment 359685 [details] [review]
libs: encoder: h265: support I/P/B QP setting seperatedly

Creates 2 properties, qp-ip and qp-ib for setting different QP for P/B 
frames
and set slice_qp_delta for each frame according to the value provided.
Comment 12 Víctor Manuel Jáquez Leal 2017-09-13 09:21:04 UTC
Attachment 359684 [details] pushed as 64a38a1 - libs: encoder: h264: support I/P/B QP setting seperatedly
Attachment 359685 [details] pushed as 5333155 - libs: encoder: h265: support I/P/B QP setting seperatedly
Attachment 359683 [details] pushed as 5796750 - libs: encoder: h264/h265: keep min_qp as is unless it's  over init_qp
Comment 13 sreerenj 2017-09-18 19:44:33 UTC
@hynjiun: 

min-qp              : Minimum quantizer value
init-qp             : Initial quantizer value

Ideally we should have,

if min-qp > init-qp:
   init-qp = min-qp

WDT ?