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

[Stefano Sabatini]

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.

> 
> 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?).
-- 
FFmpeg = Faithless Fast Mere Perennial Energized Goblin