On Sun, Feb 14, 2016 at 02:00:24PM +0100, Stefano Sabatini wrote:
> On date Saturday 2016-02-13 23:08:50 +0100, wm4 encoded:
> > 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 what I would expect as well. Ive posted a patch that does this > > The plan was to drop the avpicture_ API I think, so we should not add > inconsistencies. the align expectation you wrote above also affects avpicture_* the fix for it for the new API also changes the behavior of avpicture_* as it uses the new API If we want to fix any bugs in the new API we either have to accept that the avpicture_* code will also slightly change in behavior or we would need special cases to keep it behaving the same [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Everything should be made as simple as possible, but not simpler. -- Albert Einstein
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
