[FFmpeg-devel] [PATCH] avutil/imgutils: av_image_get_buffer_size(): do not insert padding between stride*height and palette

Michael Niedermayer michael at niedermayer.cc
Sat Feb 13 21:51:48 CET 2016


On Sat, Feb 13, 2016 at 08:46:34PM +0100, wm4 wrote:
> On Sat, 13 Feb 2016 19:38:01 +0100
> Michael Niedermayer <michael at niedermayer.cc> wrote:
> 
> > This fixes the layout that is stored in pal8 nut with odd width*height
> > 
> > Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> > ---
> >  libavcodec/avpicture.c |    2 ++
> >  libavutil/imgutils.c   |    4 ++++
> >  2 files changed, 6 insertions(+)
> > 
> > diff --git a/libavcodec/avpicture.c b/libavcodec/avpicture.c
> > index 56435f4..cac9287 100644
> > --- a/libavcodec/avpicture.c
> > +++ b/libavcodec/avpicture.c
> > @@ -51,6 +51,8 @@ int avpicture_layout(const AVPicture* src, enum AVPixelFormat pix_fmt, int width
> >  
> >  int avpicture_get_size(enum AVPixelFormat pix_fmt, int width, int height)
> >  {
> > +    if (pix_fmt == AV_PIX_FMT_PAL8)
> > +        return FFALIGN(width*height, 4) + 1024;
> >      return av_image_get_buffer_size(pix_fmt, width, height, 1);
> >  }
> >  
> > diff --git a/libavutil/imgutils.c b/libavutil/imgutils.c
> > index adf6fdd..976bd23 100644
> > --- a/libavutil/imgutils.c
> > +++ b/libavutil/imgutils.c
> > @@ -372,6 +372,10 @@ int av_image_get_buffer_size(enum AVPixelFormat pix_fmt,
> >      if (desc->flags & AV_PIX_FMT_FLAG_PSEUDOPAL)
> >          return FFALIGN(width, align) * height;
> >  
> > +    // For paletted images we do not align the palette as in AVFrames
> > +    if (desc->flags & AV_PIX_FMT_FLAG_PAL)
> > +        return FFALIGN(width, align) * height + 1024;
> > +
> >      return av_image_fill_arrays(data, linesize, NULL, pix_fmt,
> >                                  width, height, align);
> >  }
> 
> It seems wrong to litter the code with specific checks for an obscure
> pixel format, and change behavior, just to make these function's
> behavior line up with what NUT needs?

I think my commit message was poorly worded

we need to either fix the code or fix/clarify the documentation

av_image_get_buffer_size() is documented as

 * Return the size in bytes of the amount of data required to store an
 * image with the given parameters.

it does not do this, after the patch it does.

the docs could be changed to something like:

 * Return the size in bytes of the amount of data required to store an
 * image with the given parameters. Except for PAL8 where the size is
 * rounded up to the next multiple of 4 even if the user asks for
 * less alignment

and likely there are other ways it could be documented


> 
> It's also possible that this is actually what the API user wants; there
> are many implications.

yes, that is possible, should i add a av_image_get_buffer_size2() ?
but iam not sure what use av_image_get_buffer_size() has then
in its current form
its not the AVFrame required size and its not the storage size in
any container (for all pixel formats) i think, either would have
exceptions

> 
> Also, is this consistent with av_image_fill_arrays etc.?

no, and that also was not teh case before the patch. for example
av_image_fill_arrays() sets pointers up for pseudopal formats
that would be outside the array space given from
av_image_get_buffer_size()

maybe stefano could comment as i think he wrote a lot of this

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Frequently ignored answer#1 FFmpeg bugs should be sent to our bugtracker. User
questions about the command line tools should be sent to the ffmpeg-user ML.
And questions about how to use libav* should be sent to the libav-user ML.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20160213/da5b0f69/attachment.sig>


More information about the ffmpeg-devel mailing list