From patchwork Sat Sep 10 19:23:30 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Philip Langdale X-Patchwork-Id: 37835 Delivered-To: ffmpegpatchwork2@gmail.com Received: by 2002:a05:6a20:139a:b0:8f:1db5:eae2 with SMTP id w26csp1633649pzh; Sat, 10 Sep 2022 12:23:48 -0700 (PDT) X-Google-Smtp-Source: AA6agR6HbFkWP8boc+9Gnxz6AJeTFV/MOB9d6yLI1wKQ8ZgQLa6NZirDaf/cwSq5R/wLYOLtO/o6 X-Received: by 2002:a17:906:9746:b0:74f:945a:5d41 with SMTP id o6-20020a170906974600b0074f945a5d41mr13855925ejy.63.1662837828602; Sat, 10 Sep 2022 12:23:48 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1662837828; cv=none; d=google.com; s=arc-20160816; b=AnODM6jntSvRsi0DNg/KN1j3CmqLtfeuvQJBX8hgsyvDvGENZcL3QwgtggAVxInQEi TIU/2ROBz1DoQm4h8C7Z3IPfLS1y285K7JYgwxhbXwvBu98e1OgU/tcbX3moMQduYnPz ztLYCeHyUlHX1qcYOzX2RdyJJ20Fu+NjKrCrBHHd42QwLCb1OmjZja8BEK+rIzlPnaaJ 7ra6fujdDG+g0NzH3o5XPDS4dXyFNDf2qn4QZHgSsl2NrmMKz0FKw+EGpGX/og+ObYYe 6ZTTnm74pJT05dC3BZUtoCG5sWdEIrJ2xqNWtBFCq3OePwo7kuFQpCIpIS9Su2Qvtbss Ptgg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:content-transfer-encoding:cc:reply-to :list-subscribe:list-help:list-post:list-archive:list-unsubscribe :list-id:precedence:subject:mime-version:message-id:date:to:from :dkim-signature:delivered-to; bh=e0so+mkQ6XI72tUrwzjpB5A831FYGge7ygeW5CbDJog=; b=pkXgVjyRs5yR1nZExVl9xHMeLwpJbF8A0RPhp/fYMl46cgdYxyYXkChtzUFFZBzkEK /nl5euWXxmlQdo9/3u1wFeeH/bFO6wKXyunYUIOXFEU2P1AQkNaDLknPS49I6dZWJJP9 ngzPuvkH+NrU01ye1616eK3r4qb1xXTjpz7OJpfvX9Su1Olhj3gsJQaa4o+lXzyyzC/z MdbZNQuGeTtxwXrp3cc35eNyiY/TxVsDbF1FHM2I0ia2GyhwMHsx7VT0SMg3yYYAVP9Z a0ZcVmuVEaCjpdyXpMA5ocS3qW3HVUuA+C/P9FQU9gUeTuucROviXiUiXVLN0dZvs55q Ar1Q== ARC-Authentication-Results: i=1; mx.google.com; dkim=neutral (body hash did not verify) header.i=@overt.org header.s=mail header.b=IJbLZOXy; spf=pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) smtp.mailfrom=ffmpeg-devel-bounces@ffmpeg.org; dmarc=fail (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=overt.org Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org. [79.124.17.100]) by mx.google.com with ESMTP id g37-20020a056402322500b0044f0506d8d5si2837650eda.256.2022.09.10.12.23.48; Sat, 10 Sep 2022 12:23:48 -0700 (PDT) Received-SPF: pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) client-ip=79.124.17.100; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@overt.org header.s=mail header.b=IJbLZOXy; spf=pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) smtp.mailfrom=ffmpeg-devel-bounces@ffmpeg.org; dmarc=fail (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=overt.org Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 35B6168BB2B; Sat, 10 Sep 2022 22:23:45 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail.overt.org (mail.overt.org [157.230.92.47]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id D853F68B99E for ; Sat, 10 Sep 2022 22:23:38 +0300 (EEST) Received: from authenticated-user (mail.overt.org [157.230.92.47]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) by mail.overt.org (Postfix) with ESMTPSA id F33CB3F6C4; Sat, 10 Sep 2022 14:23:36 -0500 (CDT) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=overt.org; s=mail; t=1662837817; bh=Nl7l939z8+5zP1T/Rs9Wo452FB7jKHtIy7wKKT2xa5g=; h=From:To:Cc:Subject:Date:From; b=IJbLZOXysqeg3FV8ggXx+t7VKABdbNkaoMQ7q7GFOQ7YPyf4qLj5x1oGooQcSg4Xm uVKXUTlGzmsQoEYAUfOBs+w0kGXPnSqUcBQutAQUocYSGxHrlqlwW0CMkLB+nT989q OH0H1qVLZhmFQW8T5yXMfAO6JHpO2QqJXhe7FhShFSlUZ5f8TtFWAdA9V38wIxrGhA ct8wDABl65SqIKi06nOyNwzYAi2dT6iB0Kf+QgdZ849ajjRXgjqUdn8ORkaTkqwm8E GXlEuL6g3xgir9p7E/g05BwSwzAQRuPN+UmzwUVxWFHaKTxfmrBoylUzypuDlu+Zdp RTxEaJFyf3Bqw== From: Philip Langdale To: ffmpeg-devel@ffmpeg.org Date: Sat, 10 Sep 2022 12:23:30 -0700 Message-Id: <20220910192330.30872-1-philipl@overt.org> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH] v2: lavu/pixdesc: favour formats where depth and subsampling exactly match X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Cc: Philip Langdale Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" X-TUID: rOApwnESjpHc Since introducing the various packed formats used by VAAPI (and p012), we've noticed that there's actually a gap in how av_find_best_pix_fmt_of_2 works. It doesn't actually assign any value to having the same bit depth as the source format, when comparing against formats with a higher bit depth. This usually doesn't matter, because av_get_padded_bits_per_pixel() will account for it. However, as many of these formats use padding internally, we find that av_get_padded_bits_per_pixel() actually returns the same value for the 10 bit, 12 bit, 16 bit flavours, etc. In these tied situations, we end up just picking the first of the two provided formats, even if the second one should be preferred because it matches the actual bit depth. This bug already existed if you tried to compare yuv420p10 against p016 and p010, for example, but it simply hadn't come up before so we never noticed. But now, we actually got a situation in the VAAPI VP9 decoder where it offers both p010 and p012 because Profile 3 could be either depth and ends up picking p012 for 10 bit content due to the ordering of the testing. In addition, in the process of testing the fix, I realised we have the same gap when it comes to chroma subsampling - we do not favour a format that has exactly the same subsampling vs one with less subsampling when all else is equal. To fix this, I'm introducing a small score penalty if the bit depth or subsampling doesn't exactly match the source format. This will break the tie in favour of the format with the exact match, but not offset any of the other scoring penalties we already have. I have added a set of tests around these formats which will fail without this fix. v2: Rework penalty system to scale penalty based on how different the two formats are, and add new loss categories for them. Signed-off-by: Philip Langdale --- libavutil/pixdesc.c | 38 +++++++++++- libavutil/pixdesc.h | 15 +++-- libavutil/tests/pixfmt_best.c | 111 ++++++++++++++++++++++++++++------ tests/ref/fate/pixfmt_best | 2 +- 4 files changed, 139 insertions(+), 27 deletions(-) diff --git a/libavutil/pixdesc.c b/libavutil/pixdesc.c index d7c6ebfdc4..143c17e64a 100644 --- a/libavutil/pixdesc.c +++ b/libavutil/pixdesc.c @@ -3004,6 +3004,11 @@ static int get_pix_fmt_score(enum AVPixelFormat dst_pix_fmt, if ((ret = get_pix_fmt_depth(&dst_min_depth, &dst_max_depth, dst_pix_fmt)) < 0) return -3; + // Favour formats where bit depth exactly matches. If all other scoring is + // equal, we'd rather use a lower bit depth that matches the source. + if (src_min_depth != dst_min_depth || src_max_depth != dst_max_depth) + score -= 64; + src_color = get_color_type(src_desc); dst_color = get_color_type(dst_desc); if (dst_pix_fmt == AV_PIX_FMT_PAL8) @@ -3013,9 +3018,16 @@ static int get_pix_fmt_score(enum AVPixelFormat dst_pix_fmt, for (i = 0; i < nb_components; i++) { int depth_minus1 = (dst_pix_fmt == AV_PIX_FMT_PAL8) ? 7/nb_components : (dst_desc->comp[i].depth - 1); - if (src_desc->comp[i].depth - 1 > depth_minus1 && (consider & FF_LOSS_DEPTH)) { + int depth_delta = src_desc->comp[i].depth - 1 - depth_minus1; + if (depth_delta > 0 && (consider & FF_LOSS_DEPTH)) { loss |= FF_LOSS_DEPTH; score -= 65536 >> depth_minus1; + } else if (depth_delta < 0 && (consider & FF_LOSS_EXCESS_DEPTH)) { + // Favour formats where bit depth exactly matches. If all other + // scoring is equal, we'd rather use the bit depth that most closely + // matches the source. + loss |= FF_LOSS_EXCESS_DEPTH; + score -= 1 << -depth_delta; } } @@ -3024,10 +3036,12 @@ static int get_pix_fmt_score(enum AVPixelFormat dst_pix_fmt, loss |= FF_LOSS_RESOLUTION; score -= 256 << dst_desc->log2_chroma_w; } + if (dst_desc->log2_chroma_h > src_desc->log2_chroma_h) { loss |= FF_LOSS_RESOLUTION; score -= 256 << dst_desc->log2_chroma_h; } + // don't favor 422 over 420 if downsampling is needed, because 420 has much better support on the decoder side if (dst_desc->log2_chroma_w == 1 && src_desc->log2_chroma_w == 0 && dst_desc->log2_chroma_h == 1 && src_desc->log2_chroma_h == 0 ) { @@ -3035,6 +3049,28 @@ static int get_pix_fmt_score(enum AVPixelFormat dst_pix_fmt, } } + if (consider & FF_LOSS_EXCESS_RESOLUTION) { + // Favour formats where chroma subsampling exactly matches. If all other + // scoring is equal, we'd rather use the subsampling that most closely + // matches the source. + if (dst_desc->log2_chroma_w < src_desc->log2_chroma_w) { + loss |= FF_LOSS_EXCESS_RESOLUTION; + score -= 32 << (src_desc->log2_chroma_w - dst_desc->log2_chroma_w); + } + + if (dst_desc->log2_chroma_h < src_desc->log2_chroma_h) { + loss |= FF_LOSS_EXCESS_RESOLUTION; + score -= 32 << (src_desc->log2_chroma_h - dst_desc->log2_chroma_h); + } + + // don't favour 411 over 420, because 420 has much better support on the + // decoder side. + if (dst_desc->log2_chroma_w == 1 && src_desc->log2_chroma_w == 2 && + dst_desc->log2_chroma_h == 1 && src_desc->log2_chroma_h == 2) { + score += 128; + } + } + if(consider & FF_LOSS_COLORSPACE) switch(dst_color) { case FF_COLOR_RGB: diff --git a/libavutil/pixdesc.h b/libavutil/pixdesc.h index f8a195ffcd..48d9300bfe 100644 --- a/libavutil/pixdesc.h +++ b/libavutil/pixdesc.h @@ -357,12 +357,15 @@ void av_write_image_line(const uint16_t *src, uint8_t *data[4], */ enum AVPixelFormat av_pix_fmt_swap_endianness(enum AVPixelFormat pix_fmt); -#define FF_LOSS_RESOLUTION 0x0001 /**< loss due to resolution change */ -#define FF_LOSS_DEPTH 0x0002 /**< loss due to color depth change */ -#define FF_LOSS_COLORSPACE 0x0004 /**< loss due to color space conversion */ -#define FF_LOSS_ALPHA 0x0008 /**< loss of alpha bits */ -#define FF_LOSS_COLORQUANT 0x0010 /**< loss due to color quantization */ -#define FF_LOSS_CHROMA 0x0020 /**< loss of chroma (e.g. RGB to gray conversion) */ +#define FF_LOSS_RESOLUTION 0x0001 /**< loss due to resolution change */ +#define FF_LOSS_DEPTH 0x0002 /**< loss due to color depth change */ +#define FF_LOSS_COLORSPACE 0x0004 /**< loss due to color space conversion */ +#define FF_LOSS_ALPHA 0x0008 /**< loss of alpha bits */ +#define FF_LOSS_COLORQUANT 0x0010 /**< loss due to color quantization */ +#define FF_LOSS_CHROMA 0x0020 /**< loss of chroma (e.g. RGB to gray conversion) */ +#define FF_LOSS_EXCESS_RESOLUTION 0x0040 /**< loss due to unneeded extra resolution */ +#define FF_LOSS_EXCESS_DEPTH 0x0080 /**< loss due to unneeded extra color depth */ + /** * Compute what kind of losses will occur when converting from one specific diff --git a/libavutil/tests/pixfmt_best.c b/libavutil/tests/pixfmt_best.c index 0542af494f..bc0def8d83 100644 --- a/libavutil/tests/pixfmt_best.c +++ b/libavutil/tests/pixfmt_best.c @@ -39,32 +39,67 @@ static const enum AVPixelFormat pixfmt_list[] = { AV_PIX_FMT_VAAPI, }; -static enum AVPixelFormat find_best(enum AVPixelFormat pixfmt) +static const enum AVPixelFormat semiplanar_list[] = { + AV_PIX_FMT_P016, + AV_PIX_FMT_P012, + AV_PIX_FMT_P010, + AV_PIX_FMT_NV12, +}; + +static const enum AVPixelFormat packed_list[] = { + AV_PIX_FMT_XV36, + AV_PIX_FMT_XV30, + AV_PIX_FMT_VUYX, + AV_PIX_FMT_Y212, + AV_PIX_FMT_Y210, + AV_PIX_FMT_YUYV422, +}; + +static const enum AVPixelFormat subsampled_list[] = { + AV_PIX_FMT_YUV411P, + AV_PIX_FMT_YUV420P, + AV_PIX_FMT_YUV422P, + AV_PIX_FMT_YUV444P, +}; + +typedef enum AVPixelFormat (*find_best_t)(enum AVPixelFormat pixfmt); + +#define find_best_wrapper(name, list) \ +static enum AVPixelFormat find_best_ ## name (enum AVPixelFormat pixfmt) \ +{ \ + enum AVPixelFormat best = AV_PIX_FMT_NONE; \ + int i; \ + for (i = 0; i < FF_ARRAY_ELEMS(list); i++) \ + best = av_find_best_pix_fmt_of_2(best, list[i], \ + pixfmt, 0, NULL); \ + return best; \ +} + +find_best_wrapper(base, pixfmt_list) +find_best_wrapper(seminplanar, semiplanar_list) +find_best_wrapper(packed, packed_list) +find_best_wrapper(subsampled, subsampled_list) + +static void test(enum AVPixelFormat input, enum AVPixelFormat expected, + int *pass, int *fail, find_best_t find_best_fn) { - enum AVPixelFormat best = AV_PIX_FMT_NONE; - int i; - for (i = 0; i < FF_ARRAY_ELEMS(pixfmt_list); i++) - best = av_find_best_pix_fmt_of_2(best, pixfmt_list[i], - pixfmt, 0, NULL); - return best; + enum AVPixelFormat output = find_best_fn(input); + if (output != expected) { + printf("Matching %s: got %s, expected %s\n", + av_get_pix_fmt_name(input), + av_get_pix_fmt_name(output), + av_get_pix_fmt_name(expected)); + ++(*fail); + } else + ++(*pass); } int main(void) { - enum AVPixelFormat output; int i, pass = 0, fail = 0; -#define TEST(input, expected) do { \ - output = find_best(input); \ - if (output != expected) { \ - printf("Matching %s: got %s, expected %s\n", \ - av_get_pix_fmt_name(input), \ - av_get_pix_fmt_name(output), \ - av_get_pix_fmt_name(expected)); \ - ++fail; \ - } else \ - ++pass; \ - } while (0) +#define TEST(input, expected) \ + test(input, expected, &pass, &fail, find_best_base) // Same formats. for (i = 0; i < FF_ARRAY_ELEMS(pixfmt_list); i++) @@ -137,6 +172,44 @@ int main(void) // Opaque formats are least unlike each other. TEST(AV_PIX_FMT_DXVA2_VLD, AV_PIX_FMT_VDPAU); +#define TEST_SEMIPLANAR(input, expected) \ + test(input, expected, &pass, &fail, find_best_seminplanar) + + // Same formats. + for (i = 0; i < FF_ARRAY_ELEMS(semiplanar_list); i++) + TEST_SEMIPLANAR(semiplanar_list[i], semiplanar_list[i]); + + // Formats containing the same data in different layouts. + TEST_SEMIPLANAR(AV_PIX_FMT_YUV420P, AV_PIX_FMT_NV12); + TEST_SEMIPLANAR(AV_PIX_FMT_YUV420P10, AV_PIX_FMT_P010); + TEST_SEMIPLANAR(AV_PIX_FMT_YUV420P12, AV_PIX_FMT_P012); + TEST_SEMIPLANAR(AV_PIX_FMT_YUV420P16, AV_PIX_FMT_P016); + TEST_SEMIPLANAR(AV_PIX_FMT_YUV420P9, AV_PIX_FMT_P010); + +#define TEST_PACKED(input, expected) \ + test(input, expected, &pass, &fail, find_best_packed) + + // Same formats. + for (i = 0; i < FF_ARRAY_ELEMS(packed_list); i++) + TEST_PACKED(packed_list[i], packed_list[i]); + + // Formats containing the same data in different layouts. + TEST_PACKED(AV_PIX_FMT_YUV444P, AV_PIX_FMT_VUYX); + TEST_PACKED(AV_PIX_FMT_YUV444P10, AV_PIX_FMT_XV30); + TEST_PACKED(AV_PIX_FMT_YUV444P12, AV_PIX_FMT_XV36); + TEST_PACKED(AV_PIX_FMT_YUV422P, AV_PIX_FMT_YUYV422); + TEST_PACKED(AV_PIX_FMT_YUV422P10, AV_PIX_FMT_Y210); + TEST_PACKED(AV_PIX_FMT_YUV422P12, AV_PIX_FMT_Y212); + +#define TEST_SUBSAMPLED(input, expected) \ + test(input, expected, &pass, &fail, find_best_subsampled) + + // Same formats. + for (i = 0; i < FF_ARRAY_ELEMS(subsampled_list); i++) + TEST_SUBSAMPLED(subsampled_list[i], subsampled_list[i]); + + TEST_SUBSAMPLED(AV_PIX_FMT_YUV410P, AV_PIX_FMT_YUV420P); + printf("%d tests passed, %d tests failed.\n", pass, fail); return !!fail; } diff --git a/tests/ref/fate/pixfmt_best b/tests/ref/fate/pixfmt_best index 783c5fe640..f9a0fd573b 100644 --- a/tests/ref/fate/pixfmt_best +++ b/tests/ref/fate/pixfmt_best @@ -1 +1 @@ -75 tests passed, 0 tests failed. +101 tests passed, 0 tests failed.