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

[Michael Niedermayer]

On Tue, Aug 17, 2021 at 09:24:38PM +0200, Niklas Haas wrote:
> 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 thought i saw it fail but it seems not to replicate now, either way there
where all kinds of other patches applied too so it might have been something
else

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I have never wished to cater to the crowd; for what I know they do not
approve, and what they approve I do not know. -- Epicurus
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20210818/e5cc6a28/attachment.sig>