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

[Michael Niedermayer]

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 ...

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Freedom in capitalist society always remains about the same as it was in
ancient Greek republics: Freedom for slave owners. -- Vladimir Lenin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070506/90ff17ba/attachment.pgp>