[FFmpeg-devel] [PATCH v2 1/4] avcodec/aom_film_grain: add AOM film grain synthesis

[Niklas Haas]

On Fri, 08 Mar 2024 10:31:28 -0300 James Almer <jamrial at gmail.com> wrote:
> On 3/8/2024 10:21 AM, Niklas Haas wrote:
> > From: Niklas Haas <git at haasn.dev>
> > 
> > Implementation copied wholesale from dav1d, sans SIMD, under permissive
> > license. This implementation was extensively verified to be bit-exact,
> > so it serves as a much better starting point than trying to re-engineer
> > this from scratch for no reason. (I also authored the original
> > implementation in dav1d, so any "clean room" implementation would end up
> > looking much the same, anyway)
> > 
> > The notable changes I had to make while adapting this from the dav1d
> > code-base to the FFmpeg codebase include:
> > 
> > - reordering variable declarations to avoid triggering warnings
> > - replacing several inline helpers by avutil equivalents
> > - changing code that accesses frame metadata
> > - replacing raw plane copying logic by av_image_copy_plane
> > 
> > Apart from this, the implementation is basically unmodified.
> 
> Do we want this to be public? Both as a struct and the decoding functions.
> It could be used by libavfilter or even outside our libraries. The hevc 
> decoder would export the relevant T.35 SEI in the new struct if told to 
> not apply fg, like we already do in av1.

I'm not sure if the AFGS1 struct itself needs to be public, since it is
largely just a codec-internal wrapper for multiple param sets (for
scalable codecs).

If we want to add a public film grain synthesis helper, IMHO it should
be a wrapper around both ff_aom_apply_film_grain *and*
ff_h274_apply_film_grain, which directly ingests an AVFilmGrainParams
and resolves to the correct implementation.

So we can merge this series as-is and still add a public helper on top.

Incidentally, there is a strong precedent here: dav1d.h exports
dav1d_apply_grain() precisely for the reason that VLC needs to
initialize the decoder *before* it knows whether to apply synthesis on
GPU or CPU, so it has to set the equivalent of `-export_side_data
film_grain` at init time and manually apply CPU film grain synthesis if
it turns out that GPU fgs is unavailable.