Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[HEVCe] RC method forced to CBR with HRD param, overwrites previous RC method #865

Closed
uartie opened this issue Mar 4, 2020 · 11 comments
Closed
Assignees

Comments

@uartie
Copy link
Contributor

uartie commented Mar 4, 2020

  1. In gstreamer-vaapi, the base encoder always sends a HRD parameter to the driver if RC method != CQP (https://gitlab.freedesktop.org/gstreamer/gstreamer-vaapi/-/blob/bf2299279f61/gst-libs/gst/vaapi/gstvaapiencoder.c#L75).
  2. In the driver, when it parses the HRD parameter, it forces the RC method to CBR
    case VAEncMiscParameterTypeHRD:
    {
    VAEncMiscParameterHRD *vaEncMiscParamHRD = (VAEncMiscParameterHRD *)miscParamBuf->data;
    seqParams->VBVBufferSizeInBit = vaEncMiscParamHRD->buffer_size;
    seqParams->InitVBVBufferFullnessInBit = vaEncMiscParamHRD->initial_buffer_fullness;
    seqParams->RateControlMethod = RATECONTROL_CBR;
    break;
    }
  3. In gstreamer-vaapi, RC parameter is added before the HRD parameter.
  4. In the driver, the RC parameter will be parsed before the HRD parameter. Thus, the requested RC method is lost.

Based on the behavior in 1 thru 4, HEVC encoding with gst-vaapi will always use CBR in driver when user sets rate-control to any method != CQP. If ICQ/QVBR was requested, this causes ICQ quality setting to be ignored in the codechal. Also, this causes same encode output when user requests either QVBR or VBR.

QVBR/ICQ are added in gst-vaapi at https://gitlab.freedesktop.org/gstreamer/gstreamer-vaapi/-/merge_requests/292

@XinfengZhang
Copy link
Contributor

@Xiaogang-Li , HRD parameter should be valid with other BRC mode, such as VBR, should not just set it to CBR

uartie added a commit to uartie/gstreamer-vaapi that referenced this issue Mar 19, 2020
This is a work around for intel-media-driver bug
intel/media-driver#865

This driver will force the BRC to CBR for HEVCe when it
parses the HRD params.  Thus, any BRC method submitted
in the BRC params prior to parsing the HRD params will
be lost.  Therefore, VBR can't be effectively enabled
if BRC params precede the HRD params.

To work around this issue, set the HRD params before
the BRC params so the driver will parse BRC "after"
HRD params.
@uartie
Copy link
Contributor Author

uartie commented Mar 19, 2020

@Xiaogang-Li any progress on fixing this?

@Xiaogangli-intel
Copy link
Contributor

@uartie Sorry for late response. HRD should not limited to CBR, will correct it ASAP.

intel-media-ci pushed a commit to intel-media-ci/gstreamer-vaapi that referenced this issue Mar 20, 2020
This is a workaround for intel-media-driver bug
intel/media-driver#865

The driver will force the RC method to CBR for HEVCe
when it parses the HRD param.  Thus, any RC method
param submitted "prior" to the HRD param will be lost.
Therefore, VBR, ICQ and QVBR for HEVCe can't be
effectively enabled if the RC method param "precedes"
the HRD param.

To work around this issue, set the HRD param before
the RC method param so the driver will parse the RC
method param "after" the HRD param.

Afaict, other codecs in the driver (and other drivers)
do not appear to be dependent on the order of HRD and
RC param submission.
@Xiaogangli-intel
Copy link
Contributor

Hi @uartie, could you provide several test cases to us, then we can validate it.

@uartie
Copy link
Contributor Author

uartie commented Mar 24, 2020

We added a workaround in gstreamer-vaapi to temporarily fix this (https://gitlab.freedesktop.org/gstreamer/gstreamer-vaapi/-/merge_requests/307). Unfortunately, the workaround does not seem to fix issue #723 (so that must be a different bug).

In order to test a driver level fix for this bug, you will want to revert gstreamer-vaapi commit 861b4cc4b63a

gst-launch-1.0 -vf videotestsrc num-buffers=100 \
 ! video/x-raw,format=NV12,width=320,height=240 \
 ! filesink location=test.yuv

gst-launch-1.0 -vf filesrc location=test.yuv \
 ! rawvideoparse width=320 height=240 format=nv12 \
 ! vaapih265enc rate-control=qvbr ! h265parse \
 ! filesink location=qvbr.h265

gst-launch-1.0 -vf filesrc location=test.yuv \
 ! rawvideoparse width=320 height=240 format=nv12 \
 ! vaapih265enc rate-control=vbr ! h265parse \
 ! filesink location=vbr.h265

...without any fix/workaround, the above results are something like this:

md5sum *.h265
0e03b23b029dae845422fe624d2eb815  qvbr.h265
0e03b23b029dae845422fe624d2eb815  vbr.h265

Also, without a fix/workaround, you can try setting different vaapih265enc quality-factor parameter for icq and qvbr modes and observe the output is always the same.

@Xiaogangli-intel
Copy link
Contributor

Thank you.
Code ready, in validation and code review

@uartie
Copy link
Contributor Author

uartie commented Mar 26, 2020

Do you have a link to the new code?

@Xiaogangli-intel
Copy link
Contributor

Not yet, we are in the internal review. It should not take more time.

@Xiaogangli-intel
Copy link
Contributor

@uartie This issue is fixed, please help to check it on your side.

@uartie
Copy link
Contributor Author

uartie commented Apr 23, 2020

Please point me to the commit/PR that fixes this please!

@uartie
Copy link
Contributor Author

uartie commented Apr 23, 2020

Confirmed this is fixed by d9ce426

@uartie uartie closed this as completed Apr 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants