[FFmpeg-devel] [PATCH 1/6] avcodec/h264_slice: Check input SPS in ff_h264_update_thread_context()

[Michael Niedermayer]

On Mon, Mar 22, 2021 at 10:12:47PM +0100, Hendrik Leppkes wrote:
> On Mon, Mar 22, 2021 at 10:05 PM Michael Niedermayer
> <michael at niedermayer.cc> wrote:
> >
> > Fixes: crash
> > Fixes: check_pkt.mp4
> >
> > Found-by: Rafael Dutra <rafael.dutra at cispa.de>
> > Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> > ---
> >  libavcodec/h264_slice.c | 5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c
> > index 14b945756b..910d8b8848 100644
> > --- a/libavcodec/h264_slice.c
> > +++ b/libavcodec/h264_slice.c
> > @@ -304,9 +304,8 @@ int ff_h264_update_thread_context(AVCodecContext *dst,
> >      if (dst == src)
> >          return 0;
> >
> > -    // We can't fail if SPS isn't set at it breaks current skip_frame code
> > -    //if (!h1->ps.sps)
> > -    //    return AVERROR_INVALIDDATA;
> > +    if (inited && !h1->ps.sps)
> > +        return AVERROR_INVALIDDATA;
> >
> 
> Any considerations for the removed comment?

Without the inited, i could reproduce a skip_frame failure, replicating what
i assumed the comment meant.

With inited, the next lines would determine if we need to reinit but 
without h1->ps.sps the current set of checks have a good chance of
dereferencing a null pointer. So this looks like a clean failure is
better. And unlikely that was occuring in any use case.

So i think the comment, at least as written is not correct.

Thanks

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

If you think the mosad wants you dead since a long time then you are either
wrong or dead since a long time.
-------------- 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/20210323/86cb5c2f/attachment.sig>