diff mbox

[FFmpeg-devel,v2,1/2] pixdesc: Improve scoring for opaque/unknown pixel formats

Message ID a1a15bac-c69b-614e-c941-42a81b61f2cf@jkqxz.net
State Accepted
Commit 8a442d7a8a687a469ca502a18a0c68f5302b15e0
Headers show

Commit Message

Mark Thompson July 8, 2017, 2:12 p.m. UTC 
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.
---
On 07/07/17 01:14, Michael Niedermayer wrote:
> On Thu, Jul 06, 2017 at 10:59:24PM +0100, Mark Thompson wrote:
>> ...
>> +    if ((src_desc->flags & AV_PIX_FMT_FLAG_HWACCEL) ||
>> +        (dst_desc->flags & AV_PIX_FMT_FLAG_HWACCEL))
>> +        return 0;
> 
> this breaks ffmpegs choose_pixel_fmt()
> src_desc being NULL

Where does it ever get called with invalid src?  I think it would probably be better to fix those cases in the caller.  (It would be asking "of the pixel formats A and B, which is a better match for conversion from <invalid>", which is nonsensical.)

In any case, I've changed the invalid format check to src_desc && dst_desc to make this more robust.

>>  
>>      /* compute loss */
>>      *lossp = loss = 0;
> 
> shouldnt this be set on a 0 return ?
> 
> i also wonder if src and dst being identical should score different
> than if not if either is AV_PIX_FMT_FLAG_HWACCEL

Changed to:

-1  Matching hardware formats.
-2  Non-matching hardware formats.
-3  One of the input formats doesn't have enough metadata to return anything useful.
-4  One of the input formats is completely invalid.


 libavutil/pixdesc.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)
diff mbox

Patch

diff --git a/libavutil/pixdesc.c b/libavutil/pixdesc.c
index 46a7eff06d..1983ce9ef5 100644
--- a/libavutil/pixdesc.c
+++ b/libavutil/pixdesc.c
@@ -2511,8 +2511,16 @@  static int get_pix_fmt_score(enum AVPixelFormat dst_pix_fmt,
     int ret, loss, i, nb_components;
     int score = INT_MAX - 1;
 
-    if (dst_pix_fmt >= AV_PIX_FMT_NB || dst_pix_fmt <= AV_PIX_FMT_NONE)
-        return ~0;
+    if (!src_desc || !dst_desc)
+        return -4;
+
+    if ((src_desc->flags & AV_PIX_FMT_FLAG_HWACCEL) ||
+        (dst_desc->flags & AV_PIX_FMT_FLAG_HWACCEL)) {
+        if (dst_pix_fmt == src_pix_fmt)
+            return -1;
+        else
+            return -2;
+    }
 
     /* compute loss */
     *lossp = loss = 0;
@@ -2521,9 +2529,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 -3;
     if ((ret = get_pix_fmt_depth(&dst_min_depth, &dst_max_depth, dst_pix_fmt)) < 0)
-        return ret;
+        return -3;
 
     src_color = get_color_type(src_desc);
     dst_color = get_color_type(dst_desc);