[FFmpeg-devel] avcodec/mpegpicture: guard "stride changed" failures against unknown uvlinesize

Submitted by Aman Gupta on April 10, 2018, 5:20 p.m.

Details

Message ID 20180410172007.13360-1-ffmpeg@tmm1.net
State New
Headers show

Commit Message

Aman Gupta April 10, 2018, 5:20 p.m.
From: Aman Gupta <aman@tmm1.net>

Before adding uvlinesize check, I was seeing failures decoding
some samples (shown with extra logging added):

[mpeg2video @ 0x7fa193818c00] get_buffer() failed (stride changed: linesize=1280/1280 uvlinesize=0/640)
[mpeg2video @ 0x7fa193818c00] get_buffer() failed (stride changed: linesize=1280/1280 uvlinesize=0/640)
---
 libavcodec/mpegpicture.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

Comments

Michael Niedermayer April 10, 2018, 10:15 p.m.
On Tue, Apr 10, 2018 at 10:20:07AM -0700, Aman Gupta wrote:
> From: Aman Gupta <aman@tmm1.net>
> 
> Before adding uvlinesize check, I was seeing failures decoding
> some samples (shown with extra logging added):
> 
> [mpeg2video @ 0x7fa193818c00] get_buffer() failed (stride changed: linesize=1280/1280 uvlinesize=0/640)
> [mpeg2video @ 0x7fa193818c00] get_buffer() failed (stride changed: linesize=1280/1280 uvlinesize=0/640)
> ---
>  libavcodec/mpegpicture.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/libavcodec/mpegpicture.c b/libavcodec/mpegpicture.c
> index 2be670cdbc..80898c161c 100644
> --- a/libavcodec/mpegpicture.c
> +++ b/libavcodec/mpegpicture.c
> @@ -148,10 +148,13 @@ static int alloc_frame_buffer(AVCodecContext *avctx,  Picture *pic,
>          }
>      }
>  
> -    if (linesize && (linesize   != pic->f->linesize[0] ||
> -                     uvlinesize != pic->f->linesize[1])) {
> +    if (linesize && uvlinesize &&
> +        (linesize   != pic->f->linesize[0] ||
> +         uvlinesize != pic->f->linesize[1])) {

without checking if this can be reached with grayscale.
If it can then uvlinesize may be 0 in which case this change would be
wrong

[...]
Aman Gupta April 10, 2018, 10:25 p.m.
On Tue, Apr 10, 2018 at 3:15 PM, Michael Niedermayer <michael@niedermayer.cc
> wrote:

> On Tue, Apr 10, 2018 at 10:20:07AM -0700, Aman Gupta wrote:
> > From: Aman Gupta <aman@tmm1.net>
> >
> > Before adding uvlinesize check, I was seeing failures decoding
> > some samples (shown with extra logging added):
> >
> > [mpeg2video @ 0x7fa193818c00] get_buffer() failed (stride changed:
> linesize=1280/1280 uvlinesize=0/640)
> > [mpeg2video @ 0x7fa193818c00] get_buffer() failed (stride changed:
> linesize=1280/1280 uvlinesize=0/640)
> > ---
> >  libavcodec/mpegpicture.c | 9 ++++++---
> >  1 file changed, 6 insertions(+), 3 deletions(-)
> >
> > diff --git a/libavcodec/mpegpicture.c b/libavcodec/mpegpicture.c
> > index 2be670cdbc..80898c161c 100644
> > --- a/libavcodec/mpegpicture.c
> > +++ b/libavcodec/mpegpicture.c
> > @@ -148,10 +148,13 @@ static int alloc_frame_buffer(AVCodecContext
> *avctx,  Picture *pic,
> >          }
> >      }
> >
> > -    if (linesize && (linesize   != pic->f->linesize[0] ||
> > -                     uvlinesize != pic->f->linesize[1])) {
> > +    if (linesize && uvlinesize &&
> > +        (linesize   != pic->f->linesize[0] ||
> > +         uvlinesize != pic->f->linesize[1])) {
>
> without checking if this can be reached with grayscale.
> If it can then uvlinesize may be 0 in which case this change would be
> wrong
>

I forgot to mention that I was only seeing this bug with gray mode
(--enable-gray + AV_CODEC_FLAG_GRAY), and that after this patch it works as
expected.

Aman


>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Observe your enemies, for they first find out your faults. -- Antisthenes
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>
Michael Niedermayer April 10, 2018, 10:44 p.m.
On Tue, Apr 10, 2018 at 03:25:33PM -0700, Aman Gupta wrote:
> On Tue, Apr 10, 2018 at 3:15 PM, Michael Niedermayer <michael@niedermayer.cc
> > wrote:
> 
> > On Tue, Apr 10, 2018 at 10:20:07AM -0700, Aman Gupta wrote:
> > > From: Aman Gupta <aman@tmm1.net>
> > >
> > > Before adding uvlinesize check, I was seeing failures decoding
> > > some samples (shown with extra logging added):
> > >
> > > [mpeg2video @ 0x7fa193818c00] get_buffer() failed (stride changed:
> > linesize=1280/1280 uvlinesize=0/640)
> > > [mpeg2video @ 0x7fa193818c00] get_buffer() failed (stride changed:
> > linesize=1280/1280 uvlinesize=0/640)
> > > ---
> > >  libavcodec/mpegpicture.c | 9 ++++++---
> > >  1 file changed, 6 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/libavcodec/mpegpicture.c b/libavcodec/mpegpicture.c
> > > index 2be670cdbc..80898c161c 100644
> > > --- a/libavcodec/mpegpicture.c
> > > +++ b/libavcodec/mpegpicture.c
> > > @@ -148,10 +148,13 @@ static int alloc_frame_buffer(AVCodecContext
> > *avctx,  Picture *pic,
> > >          }
> > >      }
> > >
> > > -    if (linesize && (linesize   != pic->f->linesize[0] ||
> > > -                     uvlinesize != pic->f->linesize[1])) {
> > > +    if (linesize && uvlinesize &&
> > > +        (linesize   != pic->f->linesize[0] ||
> > > +         uvlinesize != pic->f->linesize[1])) {
> >
> > without checking if this can be reached with grayscale.
> > If it can then uvlinesize may be 0 in which case this change would be
> > wrong
> >
> 
> I forgot to mention that I was only seeing this bug with gray mode
> (--enable-gray + AV_CODEC_FLAG_GRAY), and that after this patch it works as
> expected.

The patch looks like it skips the linesize check in grayscale then.
This seems not correct

[...]

Patch hide | download patch | download mbox

diff --git a/libavcodec/mpegpicture.c b/libavcodec/mpegpicture.c
index 2be670cdbc..80898c161c 100644
--- a/libavcodec/mpegpicture.c
+++ b/libavcodec/mpegpicture.c
@@ -148,10 +148,13 @@  static int alloc_frame_buffer(AVCodecContext *avctx,  Picture *pic,
         }
     }
 
-    if (linesize && (linesize   != pic->f->linesize[0] ||
-                     uvlinesize != pic->f->linesize[1])) {
+    if (linesize && uvlinesize &&
+        (linesize   != pic->f->linesize[0] ||
+         uvlinesize != pic->f->linesize[1])) {
         av_log(avctx, AV_LOG_ERROR,
-               "get_buffer() failed (stride changed)\n");
+               "get_buffer() failed (stride changed: linesize=%d/%d uvlinesize=%d/%d)\n",
+               linesize,   pic->f->linesize[0],
+               uvlinesize, pic->f->linesize[1]);
         ff_mpeg_unref_picture(avctx, pic);
         return -1;
     }