[FFmpeg-devel] [PATCH 4/4] avcodec/h264dec: apply H.274 film grain

[Niklas Haas]

On Sun, 15 Aug 2021 19:11:42 +0200 Michael Niedermayer <michael at niedermayer.cc> wrote:
> On Sat, Aug 14, 2021 at 01:36:20PM +0200, Niklas Haas wrote:
> > From: Niklas Haas <git at haasn.dev>
> > 
> > Because we need access to ref frames without film grain applied, we have
> > to add an extra AVFrame to H264Picture to avoid messing with the
> > original. This requires some amount of overhead to make the reference
> > moves work out, but it allows us to benefit from frame multithreading
> > for film grain application "for free".
> > 
> > Unfortunately, this approach requires twice as much RAM to be constantly
> > allocated, due to the need for an extra buffer per H264Picture. In
> > theory, we could get away with freeing up this memory as soon as it's no
> > longer needed, but trying to call ff_thread_release_buffer() from
> > output_frame() simply deadlocks the decoder and I haven't figured out
> > why. Patches welcome(tm)
> > 
> > Tested on all three cases of (no fg), (fg present but exported) and (fg
> > present and not exported), with and without threading.
> > ---
> >  libavcodec/h264_picture.c | 24 +++++++++++++++++++-
> >  libavcodec/h264_slice.c   | 18 +++++++++++++--
> >  libavcodec/h264dec.c      | 48 +++++++++++++++++++++++++++++++--------
> >  libavcodec/h264dec.h      |  6 +++++
> >  4 files changed, 83 insertions(+), 13 deletions(-)
> 
> [...]
> > @@ -826,6 +836,21 @@ static int output_frame(H264Context *h, AVFrame *dst, H264Picture *srcp)
> >      AVFrame *src = srcp->f;
> >      int ret;
> >  
> > +    if (srcp->needs_fg) {
> 
> > +        AVFrameSideData *sd = av_frame_get_side_data(src, AV_FRAME_DATA_FILM_GRAIN_PARAMS);
> > +        av_assert0(sd);
> 
> Assertion is not correct to check for failure
> 
> This would kill the process with the lib and app

This is not a failure check. The point is that it should be impossible
for `srcp->needs_fg` to be true but the side data to be absent. The
assert is just there to communicate/enforce this.

I've added an extra comment in v2.