Re: [FFmpeg-devel] [PATCH v2] avcodec/av1_vaapi: add direct film grain mode

[Mark Thompson <sw@jkqxz.net>]

On 22/11/2022 20:26, Mark Thompson wrote:
> On 22/11/2022 19:18, Dong, Ruijing wrote:
>> [AMD Official Use Only - General]
>>
>> Hi Mark,
>>
>> Sorry for being late to reply to you.
>>
>> Your understanding is correct, and I have sent a new patch [v4] for addressing the current issue and to use
>> driver quirk mechanism to specify only AMD VAAPI driver has this behavior, then this could be more specific.
>>
>> For AMD hardware, it allocates GPU memory internally for the DPB management, the output is always the final one with or without applied film-grain.
> 
> I don't see why this requires you to write the output to the wrong surface.  Why not write it to the correct one instead?

Indeed, this seems to be a trivial fix in Mesa: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/19938>.

It would be helpful if someone with suitable hardware could test that.

Thanks,

- Mark

>> -----Original Message-----
>> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Mark Thompson
>> Sent: Sunday, November 20, 2022 11:44 AM
>> To: ffmpeg-devel@ffmpeg.org
>> Subject: Re: [FFmpeg-devel] [PATCH v2] avcodec/av1_vaapi: add direct film grain mode
>>
>> On 20/11/2022 02:59, Ruijing Dong wrote:
>>> Adding direct film grain mode for av1 decoder, which outputs alongside
>>> film grain.
>>>
>>> AV_HWACCEL_FLAG_DIRECT_FILM_GRAIN is the new flag introduced to enable
>>> this path.
>>>
>>> issue:
>>> By using AMD av1 decoder via VAAPI, when used with film grain content,
>>> the output displays black screen with incorrect frame order.
>>>
>>> The issue being discussed in here:
>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitl
>>> ab.freedesktop.org%2Fmesa%2Fmesa%2F-%2Fissues%2F6903%23note_1613807&am
>>> p;data=05%7C01%7Cruijing.dong%40amd.com%7C6f75be6a4f8044fe037d08dacb16
>>> 8529%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638045595864762041%7
>>> CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1
>>> haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=Ms7PCNEjOs09JVQd2KB46St5
>>> w3V8Idbc2shZC80VefI%3D&amp;reserved=0
>>>
>>> example:
>>> This flag can be used in ffmpeg command:
>>>
>>> ffmpeg -hwaccel vaapi -vaapi_device /dev/dri/renderD128
>>>         -hwaccel_flags 8
>>>         -i input_with_film_gram.obu
>>>         output_with_film_grain.yuv
>>>
>>> Signed-off-by: Ruijing Dong <ruijing.dong@amd.com>
>>> ---
>>> update: add new option direct_film_grain in optons_table.h
>>>
>>>    libavcodec/avcodec.h       | 17 +++++++++++++++++
>>>    libavcodec/options_table.h |  1 +
>>>    libavcodec/vaapi_av1.c     |  6 ++++--
>>>    3 files changed, 22 insertions(+), 2 deletions(-)
>>
>> My understanding of this is as follows, please correct me if any of this is wrong:
>>
>> * For AV1 with film grain enabled, VAAPI decode is specified with two output surfaces: one for pre-grain (reference) output and one for post-grain (display) output.
>> * The current driver in Mesa always writes to the pre-grain surface and ignores the post-grain surface entirely.
>> * To fix this, you intend to modify the VAAPI code in libavcodec to allow the user to manually override the expected VAAPI behaviour and instead assume that the post-grain output has been written to the pre-grain surface.
>>
>> Is that right?
>>
>>
>> [rdong]:
>>
>> If it is, could you perhaps explain why this manual option is preferable to the more obvious solution of Mesa being fixed to write the post-grain output to the post-grain surface?
>>
>> Thanks,
>>
>> - Mark
>>
>>
>>> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h index
>>> 3edd8e2636..9420e7820d 100644
>>> --- a/libavcodec/avcodec.h
>>> +++ b/libavcodec/avcodec.h
>>> @@ -2253,6 +2253,23 @@ typedef struct AVHWAccel {
>>>     */
>>>    #define AV_HWACCEL_FLAG_ALLOW_PROFILE_MISMATCH (1 << 2)
>>>
>>> +/**
>>> + * The film grain synthesis could be seperate from decoding process.
>>> + * The downstream device would apply the film grain parameters seperately.
>>> + * The desired film grain parameters can be found in SEI section in
>>> +H264
>>> + * or H265 bitstreams.
>>> + *
>>> + * In AV1, film grain is mandatorily specified, av1 decoders like AMD
>>> + * av1 decoder process film grain content internally, and the output
>>> + * includes applied film grain. For the purpose of supporting these
>>> +av1
>>> + * decoders, this flag needs to be utilized.
>>> + *
>>> + * @warning If the stream has no film grain content, this flag will
>>> + *          be ignored in the supported av1 decoders. It is advised
>>> + *          that this flag should only be used in av1 decoders
>>> + *          that support it.
>>> + */
>>> +#define AV_HWACCEL_FLAG_DIRECT_FILM_GRAIN (1 << 3)
>>>    /**
>>>     * @}
>>>     */
>>> diff --git a/libavcodec/options_table.h b/libavcodec/options_table.h
>>> index cd02f5096f..0302f89280 100644
>>> --- a/libavcodec/options_table.h
>>> +++ b/libavcodec/options_table.h
>>> @@ -399,6 +399,7 @@ static const AVOption avcodec_options[] = {
>>>    {"ignore_level", "ignore level even if the codec level used is unknown or higher than the maximum supported level reported by the hardware driver", 0, AV_OPT_TYPE_CONST, { .i64 = AV_HWACCEL_FLAG_IGNORE_LEVEL }, INT_MIN, INT_MAX, V | D, "hwaccel_flags" },
>>>    {"allow_high_depth", "allow to output YUV pixel formats with a different chroma sampling than 4:2:0 and/or other than 8 bits per component", 0, AV_OPT_TYPE_CONST, {.i64 = AV_HWACCEL_FLAG_ALLOW_HIGH_DEPTH }, INT_MIN, INT_MAX, V | D, "hwaccel_flags"},
>>>    {"allow_profile_mismatch", "attempt to decode anyway if HW
>>> accelerated decoder's supported profiles do not exactly match the
>>> stream", 0, AV_OPT_TYPE_CONST, {.i64 =
>>> AV_HWACCEL_FLAG_ALLOW_PROFILE_MISMATCH }, INT_MIN, INT_MAX, V | D,
>>> "hwaccel_flags"},
>>> +{"direct_film_grain", "allow decoder to directly apply film grain to
>>> +the output", 0, AV_OPT_TYPE_CONST, {.i64 =
>>> +AV_HWACCEL_FLAG_DIRECT_FILM_GRAIN }, INT_MIN, INT_MAX, V | D,
>>> +"hwaccel_flags"},
>>>    {"extra_hw_frames", "Number of extra hardware frames to allocate for the user", OFFSET(extra_hw_frames), AV_OPT_TYPE_INT, { .i64 = -1 }, -1, INT_MAX, V|D },
>>>    {"discard_damaged_percentage", "Percentage of damaged samples to discard a frame", OFFSET(discard_damaged_percentage), AV_OPT_TYPE_INT, {.i64 = 95 }, 0, 100, V|D },
>>>    {NULL},
>>> diff --git a/libavcodec/vaapi_av1.c b/libavcodec/vaapi_av1.c index
>>> d0339b2705..6db910f2bf 100644
>>> --- a/libavcodec/vaapi_av1.c
>>> +++ b/libavcodec/vaapi_av1.c
>>> @@ -127,6 +127,7 @@ static int vaapi_av1_start_frame(AVCodecContext *avctx,
>>>        int8_t bit_depth_idx;
>>>        int err = 0;
>>>        int apply_grain = !(avctx->export_side_data &
>>> AV_CODEC_EXPORT_DATA_FILM_GRAIN) && film_grain->apply_grain;
>>> +    int direct_film_grain = avctx->hwaccel_flags &
>>> + AV_HWACCEL_FLAG_DIRECT_FILM_GRAIN;
>>>        uint8_t remap_lr_type[4] = {AV1_RESTORE_NONE, AV1_RESTORE_SWITCHABLE, AV1_RESTORE_WIENER, AV1_RESTORE_SGRPROJ};
>>>        uint8_t segmentation_feature_signed[AV1_SEG_LVL_MAX] = {1, 1, 1, 1, 1, 0, 0, 0};
>>>        uint8_t segmentation_feature_max[AV1_SEG_LVL_MAX] = {255,
>>> AV1_MAX_LOOP_FILTER, @@ -136,7 +137,7 @@ static int vaapi_av1_start_frame(AVCodecContext *avctx,
>>>        if (bit_depth_idx < 0)
>>>            goto fail;
>>>
>>> -    if (apply_grain) {
>>> +    if (apply_grain && !direct_film_grain) {
>>>            if (ctx->tmp_frame->buf[0])
>>>                ff_thread_release_buffer(avctx, ctx->tmp_frame);
>>>            err = ff_thread_get_buffer(avctx, ctx->tmp_frame,
>>> AV_GET_BUFFER_FLAG_REF); @@ -375,6 +376,7 @@ static int vaapi_av1_end_frame(AVCodecContext *avctx)
>>>        VAAPIAV1DecContext *ctx = avctx->internal->hwaccel_priv_data;
>>>
>>>        int apply_grain = !(avctx->export_side_data &
>>> AV_CODEC_EXPORT_DATA_FILM_GRAIN) && film_grain->apply_grain;
>>> +    int direct_film_grain = avctx->hwaccel_flags &
>>> + AV_HWACCEL_FLAG_DIRECT_FILM_GRAIN;
>>>        int ret;
>>>        ret = ff_vaapi_decode_issue(avctx, pic);
>>>        if (ret < 0)
>>> @@ -385,7 +387,7 @@ static int vaapi_av1_end_frame(AVCodecContext *avctx)
>>>                if (ctx->ref_tab[i].frame->buf[0])
>>>                    ff_thread_release_buffer(avctx,
>>> ctx->ref_tab[i].frame);
>>>
>>> -            if (apply_grain) {
>>> +            if (apply_grain && !direct_film_grain) {
>>>                    ret = av_frame_ref(ctx->ref_tab[i].frame, ctx->tmp_frame);
>>>                    if (ret < 0)
>>>                        return ret;
>>> -- 
>>> 2.25.1
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fffmpeg.org%2Fmailman%2Flistinfo%2Fffmpeg-devel&amp;data=05%7C01%7Cruijing.dong%40amd.com%7C6f75be6a4f8044fe037d08dacb168529%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638045595864762041%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=p3ypzkjWCGCqBPCsHLm4rGHI1%2BwwxY0pyK1l8IQkaWs%3D&amp;reserved=0
>>
>> To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>>
>>
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".