[FFmpeg-devel] [PATCH] lavfi: add a libplacebo filter

[James Almer]

On 3/9/2021 2:18 PM, Niklas Haas wrote:
> On Tue, 09 Mar 2021 13:53:20 -0300 James Almer <jamrial at gmail.com> wrote:
>> On 3/9/2021 1:38 PM, Niklas Haas wrote:
>>> +    if (s->skip_av1_grain)
>>> +        image.av1_grain = (struct pl_av1_grain_data) {0};
>>
>> [...]
>>
>>> +    if (!s->skip_av1_grain)
>>> +        av_frame_remove_side_data(out, AV_FRAME_DATA_FILM_GRAIN_PARAMS);
>>
>> [...]
>>
>>> +    { "skip_av1_grain", "Disable AV1 grain application", OFFSET(skip_av1_grain), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, DYNAMIC },
>>
>> How does this work? Does pl_upload_avframe() support parsing the
>> filmgrain side data? If so, that's pretty cool.
> 
> Yes. More specifically, `pl_frame_from_avframe` translates the film
> grain side data into `pl_av1_grain_data`, and `pl_render_image` applies
> film grain using a compute shader.
> 
>> Also, I'd prefer if this was inverted. So make it filmgrain or maybe
>> apply_filmgrain, and of course set it to 1 by default. And it doesn't
>> need to specify it's for AV1 since the AV_FRAME_DATA_FILM_GRAIN_PARAMS
>> side data type could be extended to support the H.274 implementation in
>> the future.
> 
> This makes sense. Changed. Upon further consideration, I'm also not sure
> if this should really be enabled by default. As I understand it, this
> side data is only present if the user specifically disabled film grain
> application in the decoder, so overriding that by just moving it to
> vf_libplacebo seems like a surprising default.

Yes, film grain is exported as side data instead of being applied by the 
decoder when the AV_CODEC_EXPORT_DATA_FILM_GRAIN flag is set in the 
AVCodecContext export_side_data field.

So yeah, disabled by default sounds better. The caller may not want 
vf_libplacebo to apply it, and instead leave that to some other filter 
in the chain, or just use it in some other way after getting the 
resulting AVFrame.