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

[Michael Niedermayer]

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

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

Observe your enemies, for they first find out your faults. -- Antisthenes
-------------- 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/20070507/86a7f994/attachment.pgp>