[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