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

[Dong, Ruijing]

[AMD Official Use Only - General]

Thanks for the input.

I will try this way.

Ruijing

-----Original Message-----
From: Mark Thompson <sw at jkqxz.net>
Sent: Thursday, November 24, 2022 4:10 PM
To: Dong, Ruijing <Ruijing.Dong at amd.com>; FFmpeg development discussions and patches <ffmpeg-devel at ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH v2] avcodec/av1_vaapi: add direct film grain mode

On 24/11/2022 15:27, Dong, Ruijing wrote:
> [AMD Official Use Only - General]
>
> I might have misunderstood some of the questions, and I would like to explain more about the issue from my perspective, please correct me if anything wrong.

Are you perhaps missing that this is intended to be a common API between multiple implementations and users?

The important feature is that once the API has specified the behaviour then both the driver implementers (such as Mesa, but also others) and the users (such as FFmpeg, but also others) don't need to know what is going on on the other side, because the other side always behaves in the same way.

In this case, the API specifies that there are two output surfaces - one is written with the reference (pre-grain) frame, one is written with the display (post-grain) frame.

The Mesa driver is incorrect because it writes the display frame to the reference surface, and doesn't bother to write the display surface at all.  This is then very visible when any user tries to display the display surface, because it hasn't been written.

> This patch is NOT a hack, like @Mark Thompson <sw at jkqxz.net> mentioned.

Your proposed patch is trying to crystalise an alternative not-quite-VAAPI which uses the same library and appears compatible, but actually isn't because it differs in critical ways.  I don't think it is unreasonable to characterise this as a hack.

> Video codec, especially decoders will need to meet the requirements of
> video codecs first, if the reference picture management (DPB) was
> implemented wrongly, then it could not meet the fundamental decoder criteria. From this point of view, different hardware will need to follow the same standard for the implementation so that the decoders can generate the conformance outputs.
>
> The DPB is always an internal part of the decoder, the detail
> implementation could be differed with different benefits, if DPB is
> managed by the application, it can be more flexible and easily
> maintained, the other way is the DPB is managed by the driver and hardware itself, it could have more space for the optimization, for example the reference frame access, where the format of reference frames is NOT used for display output, and the display output cannot be used for the reference frames neither because reference frames could use a different format, which is more efficient for reference access however not good for display.
>  From my point of view VAAPI supports both of the above two ideas, and
> it is not necessary to force one to follow another, because that is limited by the initial design Idea.  In this case, there is no pre-grain output in the former decoder, it has only one display output.

Fair enough.  Luckily, the API clearly states where that display output should be written: <https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fintel%2Flibva%2Fblob%2Fmaster%2Fva%2Fva_dec_av1.h%23L300-L304&data=05%7C01%7CRuijing.Dong%40amd.com%7C632f4b36198b472f479e08dace604aaa%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638049210211836459%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=vDQ87%2BLUym3W7UsO6JJI6o9uGLAxCenCA7K2eRQHtgI%3D&reserved=0>.

You can get away with not writing the reference frame because it won't be displayed in this case, but you must write the display frame because that is the one the API says will be displayed to the user.

The unwritten reference frame will still get used in references because that is how the API is defined, but since Mesa deals with this by attaching hidden metadata to the frames there is no problem.

>  From the other side, most of the AMD AV1 decoding issues are resolved
> from the community, the film grain problem becomes more noticeable. And generally speaking it is usually a flexible part of the post processing phase after video decoding, and here it is strictly defined in AV1 spec, and it is part of the decoding standard.
> It is not practical to make changes in the DPB design idea for
> resolving this issue from AMD decoder side. And naturally output the applied firm grain is just another film grain process mode, I called it "direct film grain mode".
>
> I have asked the community to inspire me to have a better idea, and
> eventually I found out there is no good way other than to have the
> external user choice or detecting AMD driver.  I understand doing the string match to choose AMD driver is not a perfect idea, but we really need to have a method to resolve this issue. Please let me know If a better way come to your mind, which can resolve this issue and in the meanwhile not affecting other AV1 implementation path.

The proper fix is straighforward: correct the error in the Mesa driver so that it writes the display output to the display surface, as the API says it should be.

Thanks,

- Mark
-------------- next part --------------
A non-text attachment was scrubbed...
Name: winmail.dat
Type: application/ms-tnef
Size: 19244 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20221124/9f5a2dcd/attachment.bin>