[FFmpeg-devel] [PATCH] Fixes avcodec_find_best_pix_fmt() with more than 64 pix fmts defined

[Michael Niedermayer]

On Wed, Aug 17, 2011 at 10:43:34AM +0200, Stefano Sabatini wrote:
> On date Wednesday 2011-08-17 02:38:37 -0400, Matthew Einhorn encoded:
> > Hi,
> > 
> > As stated, this patch (attached) should fix
> > avcodec_find_best_pix_fmt() so that now with more than 64 pix fmts,
> > the function should be able to work for the first 64 fmts. Currently
> > the function returns a bad formats, for example,
> > avcodec_find_best_pix_fmt(1ULL << PIX_FMT_GRAY8, PIX_FMT_YUV420P, 0,
> > &nLoss) returns 72 instead of 8.
> > 
> > Please be gentle as this is the first time I used git or submitted a
> > patch. If this works fine I'll try to submit a patch for a second
> > avcodec_find_best_pix_fmt_alt function that'll support more than 64
> > formats as input to the function.
> 
> You're welcome. And yes I agree we should replace the function with a
> sane variant, for example:
> 
> enum PixelFormat avcodec_find_best_pix_fmt2(int *pix_fmts, int pix_fmts_nb,
>                                             enum PixelFormat src_pix_fmt, ...);
> 
> you pass a list of pixel formats (specifying the size, or
> alternatively setting the last element to -1), no hardcoded limit on
> the number of pixel formats.

depending on how its used it might be simpler to have a function
comparing 2 pixel formats
int is_better_pix_fmt(enum PixelFormat *best, enum PixelFormat try,
                      enum PixelFormat src_pix_fmt, ...);

That would avoid having to build the array if it has to be build

                      
> 
> > 
> > Thanks,
> > Matt
> 
> > From 076c8f936a46ea42cd4242eb4df2db265653d972 Mon Sep 17 00:00:00 2001
> > From: Matthew Einhorn <moiein2000 at gmail.com>
> > Date: Wed, 17 Aug 2011 01:58:33 -0400
> > Subject: [PATCH] Fixes avcodec_find_best_pix_fmt() when there's more than 64
> >  pixel formats.
> > 
> > This fixed the problem where if there's more than 64 pixel formats defined
> > avcodec_find_best_pix_fmt() returns the wrong pixel format.
> > 
> > Signed-off-by: Matthew Einhorn <moiein2000 at gmail.com>
> > ---
> >  libavcodec/avcodec.h    |    4 +++-
> >  libavcodec/imgconvert.c |    2 +-
> >  2 files changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> > index 4f0ed2d..f7d1520 100644
> > --- a/libavcodec/avcodec.h
> > +++ b/libavcodec/avcodec.h
> > @@ -3496,9 +3496,11 @@ int avcodec_get_pix_fmt_loss(enum PixelFormat dst_pix_fmt, enum PixelFormat src_
> >   * The pixel formats from which it chooses one, are determined by the
> >   * pix_fmt_mask parameter.
> >   *
> > + * Note, only the first 64 pixel formats will fit in pix_fmt_mask
> > + *
> >   * @code
> >   * src_pix_fmt = PIX_FMT_YUV420P;
> > - * pix_fmt_mask = (1 << PIX_FMT_YUV422P) || (1 << PIX_FMT_RGB24);
> > + * pix_fmt_mask = (1 << PIX_FMT_YUV422P) | (1 << PIX_FMT_RGB24);
> >   * dst_pix_fmt = avcodec_find_best_pix_fmt(pix_fmt_mask, src_pix_fmt, alpha, &loss);
> >   * @endcode
> >   *
> > diff --git a/libavcodec/imgconvert.c b/libavcodec/imgconvert.c
> > index 9efed50..a4197cb 100644
> > --- a/libavcodec/imgconvert.c
> > +++ b/libavcodec/imgconvert.c
> > @@ -439,7 +439,7 @@ static enum PixelFormat avcodec_find_best_pix_fmt1(int64_t pix_fmt_mask,
> >      /* find exact color match with smallest size */
> >      dst_pix_fmt = PIX_FMT_NONE;
> >      min_dist = 0x7fffffff;
> > -    for(i = 0;i < PIX_FMT_NB; i++) {
> > +    for(i = 0;i < FFMIN(PIX_FMT_NB, 64); i++) {
> >          if (pix_fmt_mask & (1ULL << i)) {
> >              loss = avcodec_get_pix_fmt_loss(i, src_pix_fmt, has_alpha) & loss_mask;
> >              if (loss == 0) {
> 
> Looks fine to me, I'm going to apply soon if I read no comments (maybe
> Michael?).

LGTM

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Observe your enemies, for they first find out your faults. -- Antisthenes
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20110817/74a3a645/attachment.asc>