Intel-Media-SDK / MediaSDK

The Intel® Media SDK
MIT License
927 stars 457 forks source link

ROI query with MFXVideoENCODE_Query changes the HEVC encoding result #2077

Closed guoyejun closed 4 years ago

guoyejun commented 4 years ago

I'm adding ROI support for ffmpeg/qsv which invokes MediaSDK, and found that roi query changes the HEVC encoding result (with no ROI). I found this issue because the PSNR changes much in my internal test. My understanding is that the encoded result should be the same if I just do roi query.

I narrowed down to one frame, and to reproduce it: $ git clone https://github.com/guoyejun/ffmpeg.git $ cd ffmpeg/ $ git checkout qsvroi $ mkdir build $ cd build/ $ ../configure --enable-libmfx $ make

use the following file for a test, unzip and get out325x288.yuv. out352x288.zip

tried the following commands for several times, and the file size of qsv.h265 does not change. $ ./ffmpeg -s 352x288 -i out352x288.yuv -c:v hevc_qsv -y qsv.h265 $ ls -l qsv.h265 -rw-rw-r-- 1 yguo18 yguo18 22077 4月 15 12:47 qsv.h265

Actually, the above command does not enable ROI encoding, the only relationship with ROI is that it queries the max supported ROI number via function MFXVideoENCODE_Query. See https://github.com/guoyejun/ffmpeg/blob/qsvroi/libavcodec/qsvenc.c#L805 which prepare the query and https://github.com/guoyejun/ffmpeg/blob/qsvroi/libavcodec/qsvenc.c#L1204 which call the query function.

The issue is that such query changes the encoding result! So, I did some change to disable the ROI query, see code change below.

$ git diff
diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c
index 8f6c7e6..26dcd87 100644
--- a/libavcodec/qsvenc.c
+++ b/libavcodec/qsvenc.c
@@ -805,18 +805,6 @@ FF_ENABLE_DEPRECATION_WARNINGS
 #if QSV_HAVE_ROI_ENCODING
     if (avctx->codec_id == AV_CODEC_ID_HEVC || avctx->codec_id == AV_CODEC_ID_H264) {
         if (QSV_RUNTIME_VERSION_ATLEAST(q->ver, 1, 32)) {
-            q->extroi.Header.BufferId = MFX_EXTBUFF_ENCODER_ROI;
-            q->extroi.Header.BufferSz = sizeof(q->extroi);
-            q->extroi.ROIMode = MFX_ROI_MODE_QP_DELTA;
-            // 256 to query the maximum supported value
-            q->extroi.NumROI = 256;
-            // due to the requirement of msdk, we must set non_empty rect for query
-            for (int i = 0; i < FF_ARRAY_ELEMS(q->extroi.ROI); ++i) {
-                q->extroi.ROI[i].Right  = 16;
-                q->extroi.ROI[i].Bottom = 16;
-            }
-            q->roi_index_in_internal_buffer = q->nb_extparam_internal;
-            q->extparam_internal[q->nb_extparam_internal++] = (mfxExtBuffer *)&q->extroi;
         }
     }
 #endif

then, rebuild ffmpeg and run again. I also tried for several time, and the size of encoded qsv.h265 does not change. $ make $ ./ffmpeg -s 352x288 -i out352x288.yuv -c:v hevc_qsv -y qsv.h265 $ ls -l qsv.h265 -rw-rw-r-- 1 yguo18 yguo18 22065 4月 15 12:55 qsv.h265

So, we can see the file size of qsv.h265 changed (22077 VS 22065). In all, just a query to get the supported ROI number, it changes the encoding result for HEVC.

btw, there's no change for h264 with/without the query.

alexelizarov commented 4 years ago

Hi @guoyejun , Could you please provide both qsv.h265 coded with ROI query and qsv.h265 coded without ROI query?

guoyejun commented 4 years ago

qsv.noquery.h265.zip

qsv.withquery.zip

lakulako commented 4 years ago

@guoyejun, Query verifies and corrects encoding parameters. If you want it to have no effect from Query use different structures in Query(src, dst). Return value tells if parameters were corrected when possible or rejected. You can compare src and dst to find out what is incorrect. In your encoded streams one can see that after query ROI affected the encoding. Without Query it doesn't, most likely because it has invalid parameters.

guoyejun commented 4 years ago

let's simplify the issue.

case 1: query(many things + roi) encode

case 2: query(many things) encode

All other things are exactly the same except the 'roi' query. And I double checked that the roi query parameter are right, and so no correction needed here.

For the 'many things' in the query, case 1 and case 2 should behavior exactly the same, no matter 'many things' have invalid parameters or not.

Since there is no difference between the two cases, I expect the rendering result should be the same.

thanks

lakulako commented 4 years ago

Query may have different "opinion". Please check how Query parameters.

alexelizarov commented 4 years ago

Hi, @guoyejun I checked your encoded files. File encoded without ROI query have QP=15 in all CUs. File encoded with ROI query have different QP values from LCU to LCU. Different QP values caused different file sizes, it is expected. Seems that ROI really applied not just queried. Could you check what ROI rectangles (coordinates and QP deltas) are you passed to Query?

guoyejun commented 4 years ago

Query may have different "opinion". Please check how Query parameters.

they are exactly the 'same' in my case, not 'different', thanks.

guoyejun commented 4 years ago

Hi, @guoyejun I checked your encoded files. File encoded without ROI query have QP=15 in all CUs. File encoded with ROI query have different QP values from LCU to LCU. Different QP values caused different file sizes, it is expected. Seems that ROI really applied not just queried. Could you check what ROI rectangles (coordinates and QP deltas) are you passed to Query?

please see the code at https://github.com/guoyejun/ffmpeg/blob/qsvroi/libavcodec/qsvenc.c#L805, and i also copy here.

    if (avctx->codec_id == AV_CODEC_ID_HEVC || avctx->codec_id == AV_CODEC_ID_H264) {
        if (QSV_RUNTIME_VERSION_ATLEAST(q->ver, 1, 32)) {
            q->extroi.Header.BufferId = MFX_EXTBUFF_ENCODER_ROI;
            q->extroi.Header.BufferSz = sizeof(q->extroi);
            q->extroi.ROIMode = MFX_ROI_MODE_QP_DELTA;
            // 256 to query the maximum supported value
            q->extroi.NumROI = 256;
            // due to the requirement of msdk, we must set non_empty rect for query
            for (int i = 0; i < FF_ARRAY_ELEMS(q->extroi.ROI); ++i) {
                q->extroi.ROI[i].Right  = 16;
                q->extroi.ROI[i].Bottom = 16;
            }
            q->roi_index_in_internal_buffer = q->nb_extparam_internal;
            q->extparam_internal[q->nb_extparam_internal++] = (mfxExtBuffer *)&q->extroi;
        }
    }
guoyejun commented 4 years ago

any update? thanks.

guoyejun commented 4 years ago

any update? thanks.

lakulako commented 4 years ago

Hi, @guoyejun Please follow recommendations from April 24.

  1. Check return status from Query. It is possible that ROI is unsupported for given system/configuration/codec.
  2. If return status is not OK, check the difference between dst and src in attached external ROI buffer after Query. Note that your code uses inplace parameters. Once you have results, we can help understanding the problems.
guoyejun commented 4 years ago

hi,

thanks for the response, for your question, it returns ok.

actually, this issue is simple, clear and easy to be reproduced. My system is skylake + ubunt 16.04, and i guess it is a generic issue for hevc encoding.

I just want to query the maximum supported value of the number of ROI. So I wrote code as described in my comment in April 27 according to msdk's document at https://github.com/Intel-Media-SDK/MediaSDK/blob/master/doc/mediasdk-man.md#mfxextencoderroi, i also copy here.

NumROI | Number of ROI descriptions in array. The Query function mode 2 returns maximum supported value (set it to 256 and Query will update it to maximum supported value).

and i get the maximum supported value successfully with the query, and then I do encoding without any new change (i don't enable the roi encoding), and the encoding result changed! I would say it is obviously a bug.

lakulako commented 4 years ago

Hi @guoyejun, What do you mean by "i don't enable the roi encoding"? You have added ROI extBuffer to the attached array of buffers, but where you delete it? It goes to Init. When you call query it changes at least NumROI. It can't return OK (==0) with huge NumROI. If Query was called, parameters are fixed and Init accepts ROI mode. Without Query the ROI parameters for Init are incorrect and ROI is refused. Encoding in ROI mode is different - i.e. it allows QP changes in CU, we see the difference in your encoded streams.

guoyejun commented 4 years ago

hi, thanks.

i mean i do not set mfxEncodeCtrl.ExtParam relative for the roi settings. I do add into mfxVideoParam.ExtParam for query purpose.

The weird thing surprised me is that: the behavior of 'query supported roi value' impacts the latter encoding for hevc!

If such thing is by design, i'll assume both h264 and hevc have the same behavior. But the fact is that only hevc has this issue. As for h264, the behavior of 'query supported roi value' does not impact the latter encoding, as I reported originally.

So, I would say it is not a feature, but a bug. It would be nice if we could first reproduce it, so we can on the same page. thanks.

alexelizarov commented 4 years ago

Hi @guoyejun,

Could you share driver version with which you reproduce this issue?

guoyejun commented 4 years ago

i built the driver from source code, see detail below. thanks. https://github.com/intel/gmmlib.git c01f0041811a3f1ff7913e620ba93e6c3ec10820 https://github.com/intel/media-driver.git 4ee83e66325430411a13c72ad1ee7af317ec5406 https://github.com/intel/libva ef8cbc9f03f7a9a043a29438a9b74e75a0c49bb8

alexelizarov commented 4 years ago

Hi @guoyejun,

I created reproducer using sample_encode. (https://github.com/Intel-Media-SDK/MediaSDK/pull/2238) In this reproducer ROI buffer attached the same way as in your code.

You can use it in following way: ./sample_encode h265 -i foreman_352x288_300.yuv -o result.hevc -w 352 -h 288 -cqp -qpi 25

In resulting bitstream we can see that all qp=25.

So, you behavior not reproduced. ROI buffer (even with delta qp 0 as in you example) triggered ROI mode. In this mode some other parameters may trigger qp changes. So, we need to know other parameters (which you refer as 'many things').

guoyejun commented 4 years ago

hi,

I do see the file size changed.

without your change: ./sample_encode h265 -i out352x288.yuv -o result_nochange_cqp.hevc -w 352 -h 288 -cqp -qpi 25 ./sample_encode h265 -i out352x288.yuv -o result_nochange_default.hevc -w 352 -h 288

after apply your change: ./sample_encode h265 -i out352x288.yuv -o result_change_cqp.hevc -w 352 -h 288 -cqp -qpi 25 ./sample_encode h265 -i out352x288.yuv -o result_change_default.hevc -w 352 -h 288

$ ll result_*
-rw-rw-r-- 1 yguo18 yguo18 10272 8月 3 12:38 result_change_cqp.hevc -rw-rw-r-- 1 yguo18 yguo18 15349 8月 3 12:38 result_change_default.hevc -rw-rw-r-- 1 yguo18 yguo18 9792 8月 3 12:23 result_nochange_cqp.hevc -rw-rw-r-- 1 yguo18 yguo18 15242 8月 3 12:22 result_nochange_default.hevc

my msdk version is 66ef7345a0e954808732d6a8fd934b6a6dcee7c4

and i also attached the files here. result.zip

lakulako commented 4 years ago

Hi @guoyejun , Let me explain again. In your code when ROI buffer is attached to Query, it (ROI buffer) also goes to Init. Init with ROI enables pps.cu_qp_delta_enabled_flag. It increases number of syntax elements even if delta_qp is always zero, hence file size is changed.

In case of AVC the ROI buffer doesn't affect encoding. Neither case contradicts API. Different behavior that you observed is not a bug but encoders' specifics.

guoyejun commented 4 years ago

i was not convinced it is a feature. It is a common behavior above the encoder.

guoyejun commented 4 years ago

don't think it is resolved.