From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id 3412344554 for ; Wed, 14 Sep 2022 22:43:18 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 65C4768BAAE; Thu, 15 Sep 2022 01:43:16 +0300 (EEST) Received: from mail.overt.org (mail.overt.org [157.230.92.47]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 5D53468B99B for ; Thu, 15 Sep 2022 01:43:09 +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 3CDEB3F92A for ; Wed, 14 Sep 2022 17:43:07 -0500 (CDT) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=overt.org; s=mail; t=1663195387; bh=thyD6ExsVTUCG836fSD83msWI9i0gn9sild1Reo2SZg=; h=Date:From:To:Subject:In-Reply-To:References:From; b=OdtqqtN/tcdV5Qg21q6TZ92NKWlMJ7hcsPJuZFwZCyU7xNa8JQSJtV+a6Z4Y6EWe9 PulRGjNWz5AtBtn7i9ek0jGooPqDKcDUtFYWE27/ublauluOW1HRwtXQ8Lq2Y49hub ArXmwU/B4MxIVNEl9clyvvuzt6gRbgynZlSZHrOoNZGJWeUIHF7aVit6sXCcnh+XtA gmtdPTqVwVts1p4DLvQIs8+0eJbCfmIW7SjKkHVE8KtM7idpN9eENCMBLnLZrHa53x 6E7JR5v/XhrhKRSVEATVJT0avYDhYgPxPnYwFWIl2SncbcUwVUb5mFVo/Zzvo90Wit zXEo8FZEAkCnw== Date: Wed, 14 Sep 2022 15:43:04 -0700 From: Philip Langdale To: ffmpeg-devel@ffmpeg.org Message-ID: <20220914154304.64a99103@fido7> In-Reply-To: <20220914210816.GP2088045@pb2> References: <20220910192330.30872-1-philipl@overt.org> <20220914210816.GP2088045@pb2> MIME-Version: 1.0 Subject: Re: [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 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: On Wed, 14 Sep 2022 23:08:16 +0200 Michael Niedermayer wrote: > On Sat, Sep 10, 2022 at 12:23:30PM -0700, Philip Langdale wrote: > > 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; > > why 64 ? Argh. I was supposed to remove this. This is from v1 and was replaced by the part below. > > + // 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. > > ok > > > > + loss |= FF_LOSS_EXCESS_DEPTH; > > + score -= 1 << -depth_delta; > > but does that do that ? > a 1bpp -> 16bpp has a considerable -depth_delta > > do we need the << at all ? The idea here is to have the scoring reflect the gap. Are you saying you'd just apply the depth_delta as-is (just a small number 1 <= n <= 15)? > > I was thinking more generically that more symmetric resolution > (difference) may be preferrable > > but this is fine too > theres an infinite number of ways to do this > its probably best to move forward and wait for some bug report to > provide real key points the heuristic can be tuned toward than > inventing cases > > also keeping the heuristic simple should be a goal Yes. I've tried to follow the existing structure to keep it comprehensible and avoid altering any existing behaviour beyond what we are intentionally changing. I agree that we can see how it does in the real world and tune. Thanks, --phil _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".