[FFmpeg-devel] [PATCH]Fix linesize for pix_fmt pal8

[Stefano Sabatini]

On date Thursday 2012-06-14 06:42:04 +0000, Carl Eugen Hoyos encoded:
> Stefano Sabatini <stefasab <at> gmail.com> writes:
> 
> > > +    else if (desc->flags & PIX_FMT_PAL) {
> > > +        // 2 would be sufficient for QuickTime,
> > > +        // but 4 works and is what avi and mov samples use
> > > +        linesize = FFALIGN(linesize, 4);
> > > +    }
> > >      return linesize;
> > 
> > Can you elaborate why this is needed?
> 
> $ ./ffmpeg -i tests/lena.pnm -s 257x257 -pix_fmt pal8 -vcodec rawvideo out.avi
> $ ./ffplay out.avi
> 
> > Note that the alignment is defined by av_image_alloc(), and in general
> > I don't see why a paletted image should need an aligned size, so I'd
> > prefer to avoid the special casing, or in other words this seems more
> > a libavcodec issue (and should be addressed there).
> 
> Please elaborate.

Elaborating more...

The rawvideo decoder uses avpicture_fill() for "filling" a picture
given a buffer, then it makes some assumptions on the alignment of the
linesize based on the pixel format
(libavcodec/rawvideo.c:raw_decode()).

So it uses avpicture_fill() which calls av_image_fill_linesizes(),
which assumes no alignment, and then fixes linesize[0] according to
the pixel format.

At the same time the rawvideo encoder
(libavcodec/rawenc.c:raw_encode()) is calling avpicture_layout() which
in turn calls av_image_fill_linesizes() which again assumes no
alignment.

So the inconsistency is not in av_image_fill_linesizes(), but in the
raw encoder.

So we have this information path:

raw_encode()
   avpicture_layout() (picture -> packet)
      av_image_fill_linesizes() assumes no alignment

...

raw_decode()
   avpicture_fill() (packet -> picture)
      av_image_fill_linesizes()
   fixes alignment based on pixel format


Changing the alignment behavior of av_image_fill_linesizes() doesn't
seem the right thing to do, indeed you're restricting the flexibility
of the function, the usual way to do that is to fix alignment *after*
the function was called (alternatively: we could extend the function
to take an additional align argument).

So the problem should be fixed at the libavcodec level.

Unfortunately there is no easy way, because you have no way to tell
avpicture layout which alignment to assume (depending on the pixel
format).

Possible solutions:
* add the alignment code in avpicture_layout(): seems wrong, as
  avpicture_layout() is supposed to be more general (and changing its
  behavior may break existing code)

* extend avpicture_layout() and avpicture_fill() to support an align
  parameter. This sounds more correct to me, and should also allow to
  choose more descriptive names for the two functions (I was always
  befuddled by those two) and maybe move them to lavu.
-- 
FFmpeg = Fiendish & Frenzy Mystic Pure Enlightening God