On Sat, Feb 13, 2016 at 08:46:34PM +0100, wm4 wrote:
> On Sat, 13 Feb 2016 19:38:01 +0100
> Michael Niedermayer <mich...@niedermayer.cc> wrote:
>
> > This fixes the layout that is stored in pal8 nut with odd width*height
> >
> > Signed-off-by: Michael Niedermayer <mich...@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.
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
