[FFmpeg-devel] [PATCH 1/2] pixdesc: Improve scoring for opaque/unknown pixel formats
wm4
nfxjfg at googlemail.com
Fri Jul 7 11:16:50 EEST 2017
On Thu, 6 Jul 2017 22:59:24 +0100
Mark Thompson <sw at jkqxz.net> wrote:
> Hardware pixel formats do not tell you anything about their actual
> contents, but should still score higher than formats with completely
> unknown properties, which in turn should score higher than invalid
> formats.
>
> Do not return an AVERROR code as a score.
>
> Fixes a hang in libavfilter where format negotiation gets stuck in a
> loop because AV_PIX_FMT_NONE scores more highly than all other
> possibilities.
> ---
> The hang in libavfilter happens when trying to choose a pixfmt for output from a list of software formats when a hardware format is the input (the hwdownload filter does this). The matching begins with AV_PIX_FMT_NONE as an invalid value and compares against each possibility in turn, but unfortunately it scores -1 when considered as a conversion from an opaque hardware format, higher than the AVERROR(EINVAL) (== -22 on Linux) scored for all of the real formats. Hence the format selection code chooses AV_PIX_FMT_NONE and thinks it is making forward progress, but actually hasn't and therefore gets stuck in an infinite loop.
>
>
> libavutil/pixdesc.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/libavutil/pixdesc.c b/libavutil/pixdesc.c
> index 46a7eff06d..35b63f43c6 100644
> --- a/libavutil/pixdesc.c
> +++ b/libavutil/pixdesc.c
> @@ -2512,7 +2512,11 @@ static int get_pix_fmt_score(enum AVPixelFormat dst_pix_fmt,
> int score = INT_MAX - 1;
>
> if (dst_pix_fmt >= AV_PIX_FMT_NB || dst_pix_fmt <= AV_PIX_FMT_NONE)
> - return ~0;
> + return -2;
> +
> + if ((src_desc->flags & AV_PIX_FMT_FLAG_HWACCEL) ||
> + (dst_desc->flags & AV_PIX_FMT_FLAG_HWACCEL))
> + return 0;
>
> /* compute loss */
> *lossp = loss = 0;
> @@ -2521,9 +2525,9 @@ static int get_pix_fmt_score(enum AVPixelFormat dst_pix_fmt,
> return INT_MAX;
>
> if ((ret = get_pix_fmt_depth(&src_min_depth, &src_max_depth, src_pix_fmt)) < 0)
> - return ret;
> + return -1;
> if ((ret = get_pix_fmt_depth(&dst_min_depth, &dst_max_depth, dst_pix_fmt)) < 0)
> - return ret;
> + return -1;
>
> src_color = get_color_type(src_desc);
> dst_color = get_color_type(dst_desc);
I don't think it makes any sense to prefer one hw format over an
another without additional information, but I guess that also means
returning a random one doesn't do any harms.
Wouldn't it be better to fix the lavfi code, though?
More information about the ffmpeg-devel
mailing list