On Sat, 13 Feb 2016 21:51:48 +0100 Michael Niedermayer <mich...@niedermayer.cc> wrote:
> 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 Why 4? Also, why do av_image_get_buffer_size() and avpicture_get_size() do different things? I'd totally expect each line _and_ the start of the palette to be aligned to the requested slignment. > > > > > 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() ? Either that or a minor bump and a warning in APIchanges, depending on how this plays out. > 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 Maybe one could argue that it shouldn't include the palette? It's kind of a special case, and many uses do not store the palette directly along with the image data, but somewhere else. > > > > > 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() I thought these functions build on each other: one to get the buffer size, and another one to get the offsets into the buffer. Does it make sense that they are not consistent? > > maybe stefano could comment as i think he wrote a lot of this > > [...] > _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel