[FFmpeg-devel] [BUG] Theora decoder displays green line

[Aurelien Jacobs]

On Mon, 7 May 2007 12:34:06 +0200
Michael Niedermayer <michaelni at gmx.at> wrote:

> Hi
> 
> On Mon, May 07, 2007 at 02:40:06AM +0200, Aurelien Jacobs wrote:
> > On Sun, 6 May 2007 19:51:34 +0200
> > Michael Niedermayer <michaelni at gmx.at> wrote:
> > 
> > > Hi
> > > 
> > > On Sun, May 06, 2007 at 06:54:53PM +0200, Aurelien Jacobs wrote:
> > > > On Sun, 6 May 2007 17:45:10 +0200
> > > > Michael Niedermayer <michaelni at gmx.at> wrote:
> > > > 
> > > > > On Sun, May 06, 2007 at 05:23:25PM +0200, Aurelien Jacobs wrote:
> > > > > > On Sun, 6 May 2007 17:12:21 +0200
> > > > > > Michael Niedermayer <michaelni at gmx.at> wrote:
> > > > > > 
> > > > > > > Hi
> > > > > > > 
> > > > > > > On Sun, May 06, 2007 at 02:26:36PM +0200, Diego Biurrun wrote:
> > > > > > > > Try the following sample:
> > > > > > > > 
> > > > > > > > http://mirrors.creativecommons.org/getcreative/Creative_Commons_-_Get_Creative.ogg
> > > > > > > > 
> > > > > > > > ffplay displays a green line at the bottom (as does MPlayer with lavc
> > > > > > > > decoder and lavf demuxer), while MPlayer with native demuxer and
> > > > > > > > libtheora decoder works fine.
> > > > > > > 
> > > > > > > ffmpeg -i output could be usefull
> > > > > > 
> > > > > > coded size is 400x304, display size should be 400x300.
> > > > > > Attached patch fixes this issue.
> > > > > > 
> > > > > > Aurel
> > > > > 
> > > > > > Index: libavcodec/vp3.c
> > > > > > ===================================================================
> > > > > > --- libavcodec/vp3.c	(revision 8918)
> > > > > > +++ libavcodec/vp3.c	(working copy)
> > > > > > @@ -1950,8 +1950,8 @@
> > > > > >          s->version = 1;
> > > > > >  
> > > > > >      s->avctx = avctx;
> > > > > > -    s->width = (avctx->width + 15) & 0xFFFFFFF0;
> > > > > > -    s->height = (avctx->height + 15) & 0xFFFFFFF0;
> > > > > > +    s->width = (avctx->coded_width + 15) & 0xFFFFFFF0;
> > > > > > +    s->height = (avctx->coded_height + 15) & 0xFFFFFFF0;
> > > > > >      avctx->pix_fmt = PIX_FMT_YUV420P;
> > > > > >      if(avctx->idct_algo==FF_IDCT_AUTO)
> > > > > >          avctx->idct_algo=FF_IDCT_VP3;
> > > > > > @@ -2385,6 +2385,8 @@
> > > > > >  
> > > > > >      s->width = get_bits(gb, 16) << 4;
> > > > > >      s->height = get_bits(gb, 16) << 4;
> > > > > > +    avctx->width = get_bits_long(gb, 24);
> > > > > > +    avctx->height = get_bits_long(gb, 24);
> > > > > 
> > > > > is there anything which ensures that the allocated buffer will
> > > > > be large enough for the picture which gets decoded?
> > > > > 
> > > > > also, avcodec_set_dimensions() should be used
> > > > > 
> > > > > 
> > > > > [...]
> > > > > > Index: libavformat/oggparsetheora.c
> > > > > > ===================================================================
> > > > > > --- libavformat/oggparsetheora.c	(revision 8918)
> > > > > > +++ libavformat/oggparsetheora.c	(working copy)
> > > > > > @@ -70,13 +70,15 @@
> > > > > >              return -1;
> > > > > >          }
> > > > > >  
> > > > > > -        st->codec->width = get_bits(&gb, 16) << 4;
> > > > > > -        st->codec->height = get_bits(&gb, 16) << 4;
> > > > > > +        st->codec->coded_width = get_bits(&gb, 16) << 4;
> > > > > > +        st->codec->coded_height = get_bits(&gb, 16) << 4;
> > > > > > +        st->codec->width = get_bits_long(&gb, 24);
> > > > > > +        st->codec->height = get_bits_long(&gb, 24);
> > > > > 
> > > > > coded_width=width unless lowres or other decoder side tricks
> > > > > are used
> > > > > 
> > > > > the allocated buffer always has its w/h rounded up to the next multiple
> > > > > of 16 with yv12
> > > > > that is coded_width is not the allocated width (see avcodec_align_dimensions)
> > > > 
> > > > Ok. It seems my usage of coded_width is wrong.
> > > > Now what should be used when the displayed picture is smaller than the
> > > > encoded picture ?
> > > 
> > > normal codecs (not xiph codecs) have a width/height, use buffers with
> > > rounded up width/height and can encode one or more display rectangles
> > > 
> > > you could store the get_bits(&gb, 16) << 4 in width/height and use
> > > pan_scan for the display width/height this would theoretically be correct
> > > but as ffplay and others ignore pan_scan currently this would only be
> > > half of a fix ...
> > 
> > I gave a quick try to pan_scan. For now it's used in only one place:
> > MpegEnc. Not any single decoder use it. Using it would imply significant
> > modifications to ffplay, ffmpeg, vd_ffmpeg and every other apps using
> > libavcodec. I admit that it's the proper and most versatil solution,
> > but it's a huge work to implement.
> > So instead I fixed it in a far simpler way using the nice property of avcodec_align_dimensions. See attached patch.
> 
> patch looks ok

Applied.

Aurel