Intel-Media-SDK / MediaSDK

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

Fill HDR Side Data for HEVC Decoding #2597

Open softworkz opened 3 years ago

softworkz commented 3 years ago

Structures for HDR side data have recently been added for HEVC encoding: https://github.com/Intel-Media-SDK/MediaSDK/commit/a9685bf658d1ec97ff44b614dcf75818ecc62819 :

What missing is to fill those structures when decoding HEVC video.

Could this be added?

dmitryermilov commented 3 years ago

@softworkz , just to be on the same page: would you like to have this reporting from DecodeHeader and together with each output frame (if there're per frame SEI), right?

softworkz commented 3 years ago

@softworkz , just to be on the same page: would you like to have this reporting from DecodeHeader and together with each output frame (if there're per frame SEI), right?

Correct. From all videos that I've seen so far, even with static HDR (like HDR10), the data is in every frame (SEI type 137 and 144 IIRC).

Purposes:

dmitryermilov commented 3 years ago

Thanks for details. Yes, we can do it but please expect some delay, ~2 weeks.

softworkz commented 3 years ago

2 weeks? That's not a delay - that would be awesome 😆

wangyan-intel commented 3 years ago

@pengxin99 Could you please take a look? Thanks.

softworkz commented 2 years ago

Thanks for details. Yes, we can do it but please expect some delay, ~2 weeks.

@dmitryermilov @pengxin99 - Is there any progress in sight on this issue?

wangyan-intel commented 2 years ago

@softworkz Sorry for slow response. As you know, we are transferring from MSDK legacy to oneVPL (https://github.com/oneapi-src/oneVPL-intel-gpu). For legacy branch, it has been implemented by @pengxin99 with extension buffer. But this flow cannot be used in VPL architecture. We are evaluating the path and will update it ASAP.

softworkz commented 2 years ago

Only the "legacy" path is of interest at this time. Will the implementation from @pengxin99 be merged?

softworkz commented 2 years ago

For Linux we could easily merge this patch ourselves, but not for Windows - that's the big problem. Or is there a way how we could build our own libmfxhw64.dll for Windows?

wangyan-intel commented 2 years ago

@softworkz In general, we will not add new feature into legacy path and just bug fix. Of course, you could use @pengxin99's legacy PR. Windows code isn't included in current repo (https://github.com/Intel-Media-SDK). @dmitryermilov @onabiull Could we provide binary build beyond the normal release build?

softworkz commented 2 years ago

Or even better - could we get the Windows code (maybe under NDA) to build our own binaries? Thanks, sw

dmitryermilov commented 2 years ago

Binary for NDA user - I assume it's possible. Code for NDA user - there should be a very strong request, unlikely to be approved but I can double check. Please send me a formal email including company name.

softworkz commented 2 years ago

Hi Dmitry, thanks I'll follow up offline!

dmitryermilov commented 2 years ago

Fixed by https://github.com/Intel-Media-SDK/MediaSDK/pull/2726. Manual update: https://github.com/Intel-Media-SDK/MediaSDK/pull/2876.

Windows changes will be merged today. Please expect a few (2-8) weeks delay to get updated msdk/vpl run-time library in public graphics driver packages.

dmitryermilov commented 2 years ago

@softworkz , the fix landed into 101.1404 Windows Graphics driver

softworkz commented 2 years ago

Hey Dmitry,

thanks that's great news!

Now we will just need to let it land in ffmpeg ;-)

I'll check with @xhaihao whether he wants to do it. Otherwise I'll try to find some time.

Thanks again, softworkz

xhaihao commented 2 years ago

@softworkz Yes, we will check the latest driver. @dmitryermilov it is available in the latest oneVPL GPU runtime on both Linux & Windows, right ? Does MediaSDK support it ?

wangyan-intel commented 2 years ago

@xhaihao Yes, It is common code for both Windows and Linux. MediaSDK also supports it.

dmitryermilov commented 2 years ago

Right

xhaihao commented 2 years ago

@dmitryermilov @wangyan-intel If user provides MFX_EXTBUFF_MASTERING_DISPLAY_COLOUR_VOLUME & MFX_EXTBUFF_CONTENT_LIGHT_LEVEL_INFO extended buffers, is there a flag indicating valid HDR data is returned from the SDK?

dmitryermilov commented 2 years ago

Good question, @xhaihao . I have to admit it looks as a gap. I can propose only to rely on checking non zero MaxDisplayMasteringLuminance/MaxContentLightLevel ..

xhaihao commented 2 years ago

however 0 is a valid value for max_content_light_level and max_pic_average_light_level

xhaihao commented 2 years ago

BTW is this added for av1 decoding ?

wangyan-intel commented 2 years ago

Not yet

wangyan-intel commented 2 years ago

@dmitryermilov @wangyan-intel If user provides MFX_EXTBUFF_MASTERING_DISPLAY_COLOUR_VOLUME & MFX_EXTBUFF_CONTENT_LIGHT_LEVEL_INFO extended buffers, is there a flag indicating valid HDR data is returned from the SDK?

@pengxin99 Could you please clarify it? Thanks

xhaihao commented 2 years ago

Are RGB x coordinates stored in DisplayPrimariesX[3] in R=>G=>B order, or G=>B=>R order ?

pengxin99 commented 2 years ago

hi, @xhaihao , At first, we wanted to useInsertPayloadToggle in the struct mfxExtMasteringDisplayColourVolume to indicate whether the decoded clips have HDR SEI, but the InsertPayloadToggle was designed for encoding originally, so we did not use this plan. The current way is that if the value of all members ​​in the struct mfxExtMasteringDisplayColourVolume are 0, it means that there is no HDR sei in the decoded clips, and vice versa.

xhaihao commented 2 years ago

is it possible to use a reserved byte in mfxExtMasteringDisplayColourVolume and mfxExtContentLightLevelInfo to indicate whether the SDK writes valid data back ?

pengxin99 commented 2 years ago

Are RGB x coordinates stored in DisplayPrimariesX[3] in R=>G=>B order, or G=>B=>R order ?

For the DisplayPrimariesX[3], We just parse the SEI of clips storage information in order, and according to the spec, it should be G=>B=>R order.

pengxin99 commented 2 years ago

is it possible to use a reserved byte in mfxExtMasteringDisplayColourVolume and mfxExtContentLightLevelInfo to indicate whether the SDK writes valid data back ?

I think this way can be considered, I will discuss it with @wangyan-intel @dmitryermilov for this. If it is fine, I will continue to add this flag.

xhaihao commented 2 years ago

Are RGB x coordinates stored in DisplayPrimariesX[3] in R=>G=>B order, or G=>B=>R order ?

For the DisplayPrimariesX[3], We just parse the SEI of clips storage information in order, and according to the spec, it should be G=>B=>R order.

Thanks, so for encoding, user should pass RGB coordinates to the SDK in G=>B=>R order too, right ?

dmitryermilov commented 2 years ago

Yes, we can extend the API but then the solution would apply only for VPL API and only for VPL-run-times (>=TGL). In other words it won't work on BDW, APL, SKL, ICL systems.
What option I can suggest: 1) New API as discussed above 2) Rely on existing InsertPayloadToggle api field but in this case we break semantic of this API field.

xhaihao commented 2 years ago

Yes, we can extend the API but then the solution would apply only for VPL API and only for VPL-run-times (>=TGL). In other words it won't work on BDW, APL, SKL, ICL systems. What option I can suggest:

1. New API as discussed above

2. Rely on existing InsertPayloadToggle api field but in this case we break semantic of this API field.

If so, I think option 2 would be a better way because user may use this API on legacy devices.

xhaihao commented 2 years ago

@softworkz https://github.com/intel-media-ci/ffmpeg/pull/518 may fetch HDR data when decoding and keep HDR data when encoding in FFmpeg-QSV, you may have a try if you're interested.

Note the SDK failed to fetch HDR data for some special clips.

softworkz commented 2 years ago

Oh, that was quick! Thanks a lot for taking care of this. I'll surely try in the next few days, I got some ffmpeg tasks anyway!

xhaihao commented 2 years ago

Note the SDK failed to fetch HDR data for some special clips.

@pengxin99 The SDK failed to parse SEI when SEI nal unit is located before a valid SPS, hence m_Headers.m_SEIParams.GetHeader(SEI_MASTERING_DISPLAY_COLOUR_VOLUME) return NULL.

pengxin99 commented 2 years ago

Note the SDK failed to fetch HDR data for some special clips.

@pengxin99 The SDK failed to parse SEI when SEI nal unit is located before a valid SPS, hence m_Headers.m_SEIParams.GetHeader(SEI_MASTERING_DISPLAY_COLOUR_VOLUME) return NULL.

@xhaihao thanks for clarifying this, does this mean the special clip is an error clip?

softworkz commented 2 years ago

Hi,

long time after I had asked here for providing the HDR data, I had discovered the existence of the MFXVideoDECODE_GetPayload API. In December I had sent an ffmpeg patch to make use of this for retrieving various kinds of SEI data. The weird thing is that I can get some SEI data this way, but I couldn’t get it working consistently for the HDR information (CLL and MDCV). Here's an excerpt;

while (1) {
    int start;
    memset(&data[0], 0, sizeof(data));

    ret = MFXVideoDECODE_GetPayload(q->session, &ts, &payload);
    if (ret != MFX_ERR_NONE) {
        av_log(avctx, AV_LOG_WARNING, "error getting SEI payload: %d \n", ret);
        return ret;
    }

    if (payload.NumBit == 0 || payload.NumBit >= sizeof(data) * 8) {
        break;
    }

    start = find_start_offset(data);

    switch (payload.Type) {
        case SEI_TYPE_BUFFERING_PERIOD:
        case SEI_TYPE_PIC_TIMING:
            continue;
        case SEI_TYPE_MASTERING_DISPLAY_COLOUR_VOLUME:
        case SEI_TYPE_CONTENT_LIGHT_LEVEL_INFO:
            // There seems to be a bug in MSDK for these
            payload.NumBit -= 8;
            break;
    }

    if (init_get_bits(&gb, &data[start], payload.NumBit - start * 8) < 0)
        av_log(avctx, AV_LOG_ERROR, "Error initializing bitstream reader");
    else
        ret = ff_hevc_decode_nal_sei(&gb, avctx, &sei, &ps, HEVC_NAL_SEI_PREFIX);
  .....
}

While I couldn't get the data properly, what does work though (IIRC) is to get the information whether the data exists. So maybe it would be possible to use a combined approach:

• Use MFXVideoDECODE_GetPayload for checking whether the data exists • Use MFX ext buffers to retrieve the data

It’s not really elegant, but maybe it could work right now without needing to wait for any changes from the MSDK side?

softworkz

dmitryermilov commented 2 years ago

@softworkz , Code looks good except I don't see how you init data. App is responsible for memory allocation for mfxPayload. @wangyan-intel , can you please ask someone to take a look at GetPayload when bitstream has HDR SEI?

pengxin99 commented 2 years ago

Hi @softworkz ,

I couldn’t get it working consistently for the HDR information (CLL and MDCV)

do you mean the HDR sei get from MFXVideoDECODE_GetPayload is not aligned with the HDR sei from clips? if so, does all the HDR sei from MFXVideoDECODE_GetPayload are error values?

wangyan-intel commented 2 years ago

@pengxin99 will help this. Thanks

xhaihao commented 2 years ago

@dmitryermilov @pengxin99 @wangyan-intel MFXVideoDECODE_DecodeFrameAsync output frame in display order, is the payload returned from MFXVideoDECODE_GetPayload in display order too ?

dmitryermilov commented 2 years ago

GetPayload returns payloads in FIFO order. Since bitstream is encodedOrder, payloads will be in encodedOrder as well. App is expected to associate payloads with frames by output timestamps "mfxU64 *ts".

xhaihao commented 2 years ago

GetPayload returns payloads in FIFO order. Since bitstream is encodedOrder, payloads will be in encodedOrder as well. App is expected to associate payloads with frames by output timestamps "mfxU64 *ts".

APP will have to maintain a payload list If user want to find out the right payload for the output frame returned from MFXVideoDECODE_DecodeFrameAsync, right ?

dmitryermilov commented 2 years ago

Right

softworkz commented 2 years ago

@softworkz , Code looks good except I don't see how you init data. App is responsible for memory allocation for mfxPayload. @wangyan-intel , can you please ask someone to take a look at GetPayload when bitstream has HDR SEI?

The full code is here (scroll down to "Patch"): https://patchwork.ffmpeg.org/project/ffmpeg/patch/DM8P223MB03655F44A7BF01132BB2959DBA669@DM8P223MB0365.NAMP223.PROD.OUTLOOK.COM/ (complete patch series: https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=5353)

softworkz commented 2 years ago

Hi @softworkz ,

I couldn’t get it working consistently for the HDR information (CLL and MDCV)

do you mean the HDR sei get from MFXVideoDECODE_GetPayload is not aligned with the HDR sei from clips? if so, does all the HDR sei from MFXVideoDECODE_GetPayload are error values?

I don't remember exactly, but I think It was something regarding alignment. I'm not doing the side data decoding myself. Instead I'm just plumbing it together with ffmpeg's SEI decoding and this works fine for several SEI data types, but not for HDR data.

softworkz commented 2 years ago

GetPayload returns payloads in FIFO order. Since bitstream is encodedOrder, payloads will be in encodedOrder as well. App is expected to associate payloads with frames by output timestamps "mfxU64 *ts".

APP will have to maintain a payload list If user want to find out the right payload for the output frame returned from MFXVideoDECODE_DecodeFrameAsync, right ?

My primary use case so far has been to decode closed captions side data (EIA-608/708) for

This has been working fine in all private and public tests with MPEG2, H.264 and H.265 without needing to re-order or re-associate side data to frames, so I'm wondering whether any re-ordering will be required.

The sequence is roughly like this:


MFXVideoDECODE_DecodeFrameAsync(q->session, avpkt->size ? &bs : NULL, insurf, &outsurf, sync);

...

out_frame = find_frame(q, outsurf);

...

Loop:

MFXVideoDECODE_GetPayload(q->session, &ts, &payload); "Decode Payload" "Add payload data to out_frame"

End loop


Are you saying that this could lead to incorrect association between frames and side data?

dmitryermilov commented 2 years ago

Hi @softworkz ,

I think it's not fully right. Firstly, there're might be several payloads per frame. Plus, let's keep in mind frames reordering. MFXVideoDECODE_GetPayload returns payload internally detected by decoder during bitstream parsing( which is always in decoded order). Let's consider IB1P1B2P2 pattern in display order. In decoded order it will be IP1B1P2B2. So e.g. when app gets B1 frame to displaying, it means P1 frame is already decoded so CC data for P1/B1 are already consumed by decoder. If so, the first GetPayload() call return CC for P1, then 2nd GetPayload() returns CC for B1. Etc.. And app needs to maintain this association.

softworkz commented 2 years ago

there're might be several payloads per frame

Note the "Loop".

Plus, let's keep in mind frames reordering. MFXVideoDECODE_GetPayload returns payload internally detected by decoder during bitstream parsing( which is always in decoded order). Let's consider IB1P1B2P2 pattern in display order. In decoded order it will be IP1B1P2B2. So e.g. when app gets B1 frame to displaying, it means P1 frame is already decoded so CC data for P1/B1 are already consumed by decoder. If so, the first GetPayload() call return CC for P1, then 2nd GetPayload() returns CC for B1. Etc.. And app needs to maintain this association.

In another conversation with HaiHao, we determined that the reason why I haven't seen any issues with CC so far is probably because B-frames usually don't occur in TV broadcast signals.

Thanks for the explanation, I will update my patch accordingly.

What remains is the issue about HDR side data. I will shortly look at in again, then I might be able to provide more information.

Thanks, softworkz