[FFmpeg-devel] [PATCH] check for invalid roq decoding (issue 2521)

[Michael Niedermayer]

On Sun, Jan 09, 2011 at 08:29:00PM -0500, Daniel Kang wrote:
> On Sun, Jan 9, 2011 at 8:21 PM, Michael Niedermayer <michaelni at gmx.at>wrote:
> 
> >  On Sun, Jan 09, 2011 at 08:19:13PM -0500, Daniel Kang wrote:
> > > On Sun, Jan 9, 2011 at 8:13 PM, Michael Niedermayer <michaelni at gmx.at
> > >wrote:
> > >
> > > >  On Sun, Jan 09, 2011 at 08:06:11PM -0500, Daniel Kang wrote:
> > > > > On Sun, Jan 9, 2011 at 7:58 PM, Michael Niedermayer <
> > michaelni at gmx.at
> > > > >wrote:
> > > > >
> > > > > >  On Sun, Jan 09, 2011 at 05:59:34PM -0500, Daniel Kang wrote:
> > > > > > > ffmpeg does not check for invalid decoding (motion vector I
> > believe)
> > > > > > > when decoding roq files. The patch attached adds a check for
> > this. As
> > > > > > > with the fix for issue 2513 (r26279), ffmpeg will continue to
> > decode
> > > > > > > when it encounters this error.
> > > > > >
> > > > > > >  roqvideo.c |    4 ++++
> > > > > > >  1 file changed, 4 insertions(+)
> > > > > > > 9500ce3dabb508b67dad1f6a34b83fa4be1f8d0b
> > > >  roq_invalid_decode_check.diff
> > > > > > > From 94559de1e80e66baf7cd8a803313b114d481825a Mon Sep 17 00:00:00
> > > > 2001
> > > > > > > From: Daniel Kang <daniel.d.kang at gmail.com>
> > > > > > > Date: Sun, 9 Jan 2011 17:44:09 -0500
> > > > > > > Subject: [PATCH] Add check for invalid decode type
> > > > > > >
> > > > > > > ---
> > > > > > >  libavcodec/roqvideo.c |    4 ++++
> > > > > > >  1 files changed, 4 insertions(+), 0 deletions(-)
> > > > > > >
> > > > > > > diff --git a/libavcodec/roqvideo.c b/libavcodec/roqvideo.c
> > > > > > > index 830eb7b..bddd542 100644
> > > > > > > --- a/libavcodec/roqvideo.c
> > > > > > > +++ b/libavcodec/roqvideo.c
> > > > > > > @@ -118,6 +118,10 @@ static inline void
> > > > apply_motion_generic(RoqContext
> > > > > > *ri, int x, int y, int deltax
> > > > > > >      for(cp = 0; cp < 3; cp++) {
> > > > > > >          int outstride = ri->current_frame->linesize[cp];
> > > > > > >          int instride  = ri->last_frame   ->linesize[cp];
> > > > > > > +        if (ri->last_frame->data[cp] == NULL) {
> > > > > > > +            av_log(NULL, AV_LOG_ERROR, "Invalid decode type.
> > Invalid
> > > > > > header?\n");
> > > > > > > +            return;
> > > > > > > +        }
> > > > > >
> > > > > > if one of them is 0 the others are 0 too
> > > > >
> > > > >
> > > > > If that is the case, should I move the check outside the loop (e.g.
> > > > > check for ri->last_frame->data[0])?
> > > >
> > > > thats probably the easiest option here.
> > > > the alternative would be to allocate a last frame ...
> > >
> > >
> > > I have updated the patch with the check moved outside the loop.
> >
> > >  roqvideo.c |    5 +++++
> > >  1 file changed, 5 insertions(+)
> > > cb8ca4b92bcae39c0432cac9f307b00d4a9fcb11  roq_invalid_decode_check.diff
> > > From 009ca07ca542fc5b687ba72d9d33856d834ad3ba Mon Sep 17 00:00:00 2001
> > > From: Daniel Kang <daniel.d.kang at gmail.com>
> > > Date: Sun, 9 Jan 2011 17:44:09 -0500
> > > Subject: [PATCH] Add check for invalid decode type
> > >
> > > ---
> > >  libavcodec/roqvideo.c |    5 +++++
> > >  1 files changed, 5 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/libavcodec/roqvideo.c b/libavcodec/roqvideo.c
> > > index 830eb7b..33fc60f 100644
> > > --- a/libavcodec/roqvideo.c
> > > +++ b/libavcodec/roqvideo.c
> > > @@ -115,6 +115,11 @@ static inline void apply_motion_generic(RoqContext
> > *ri, int x, int y, int deltax
> > >          return;
> > >      }
> > >
> > > +    if (ri->last_frame->data[0] == NULL) {
> > > +        av_log(NULL, AV_LOG_ERROR, "Invalid decode type. Invalid
> > header?\n");
> >
> > do we have a better context than NULL in that function available?
> > if yes it should be used
> 
> 
> There is a better context that I missed. Patch updated.

>  roqvideo.c |    5 +++++
>  1 file changed, 5 insertions(+)
> e4dd56b7ef8298400573093c6c5a48e07c32bb9a  roq_invalid_decode_check.diff
> From cc36ffe7a9d7cecfbc53af4bf0086cf5a6586ab5 Mon Sep 17 00:00:00 2001
> From: Daniel Kang <daniel.d.kang at gmail.com>
> Date: Sun, 9 Jan 2011 17:44:09 -0500
> Subject: [PATCH] Add check for invalid decode type

lgtm

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

I am the wisest man alive, for I know one thing, and that is that I know
nothing. -- Socrates
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20110110/1eefddca/attachment.pgp>