[FFmpeg-devel] avpicture_layout() writes passed the buffer length for pseudo-paletted formats (bug)

[Michael Niedermayer]

On Wed, Aug 24, 2011 at 08:19:39PM -0400, Matthew Einhorn wrote:
> On Sun, Aug 21, 2011 at 2:09 PM, Matthew Einhorn <moiein2000 at gmail.com> wrote:
> > Hi,
> >
> > The problem is that in the docs avpicture_layout() says it writes a
> > picture with size avpicture_get_size(). So one would make the buffer
> > input to avpicture_layout() of that size. However, for pseudo-paletted
> > formats (as listed in avpicture_get_size()) the amount written to
> > buffer is avpicture_get_size()+1024 (which is also the
> > avpicture_fill() size).
> >
> > If the buffer size is less than avpicture_get_size() (as indicated by
> > dest_size in avpicture_layout()) an error is returned by
> > avpicture_layout(). However, for these pseudo-paletted formats no
> > error is returned if the buffer is less than avpicture_get_size()+1024
> > and the function simply writes passed the actual buffer length for a
> > total of avpicture_get_size()+1024.
> >
> > The reason for this it seems is that avpicture_layout() wasn't
> > supposed to write the palette for these pseudo-paletted formats, but
> > the function still writes it resulting in the problem.
> >
> >
> > The solution is to:
> > *Either remove the exception for pseudo-paletted formats so that
> > avpicture_get_size() now completely relies on avpicture_fill() (will
> > probably break backwards compatibility) and both return the same size.
> > *Make an exception in avpicture_layout() as well for the
> > pseudo-paletted formats so that the palette is not written for them.
> > *If this "is" intended behavior update avpicture_layout() docs to
> > indicate that for these formats the buffer needs to be larger than
> > avpicture_get_size() by 1024.
> > *Or maybe there's some other deeper issue here (other then to prevent
> > copying of a standard palette) as to why these pseudo-paletted formats
> > are treated specially?
> >
> > I'd probably select option 2 otherwise it'll probably break backward
> > compatibility.
> >
> > I can write the patch for whatever is selected.
> >
> > Thanks,
> > Matt
> >
> 
> In order to speed up the process, a patch using option two from above
> (avpicture_layout() is edited to match avpicture_get_size()) is
> attached.
> 
> Thanks,
> Matt
> 
> P.S. Am I supposed to include the patch text in the email or is
> attaching it fine?

>  imgconvert.c |   10 ++++++++++
>  1 file changed, 10 insertions(+)
> 9bf0aec6bbb4daa03c4acf120985afde49fa4c8e  0001-Fixes-avpicture_layout-to-not-write-past-buffer-end.patch
> From ece858c593ed04e81335dbfc4493e34f6e7f05ba Mon Sep 17 00:00:00 2001
> From: Matthew Einhorn <moiein2000 at gmail.com>
> Date: Wed, 24 Aug 2011 20:14:03 -0400
> Subject: [PATCH] Fixes avpicture_layout to not write past buffer end.

applied, thanks

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Dictatorship naturally arises out of democracy, and the most aggravated
form of tyranny and slavery out of the most extreme liberty. -- Plato
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20111019/83a8a285/attachment.asc>