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 85F3043DC0 for ; Sat, 10 Sep 2022 15:04:43 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 4535068BB15; Sat, 10 Sep 2022 18:04:41 +0300 (EEST) Received: from relay10.mail.gandi.net (relay10.mail.gandi.net [217.70.178.230]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id C7D3F68BAD6 for ; Sat, 10 Sep 2022 18:04:34 +0300 (EEST) Received: (Authenticated sender: michael@niedermayer.cc) by mail.gandi.net (Postfix) with ESMTPSA id D6A6E240003 for ; Sat, 10 Sep 2022 15:04:33 +0000 (UTC) Date: Sat, 10 Sep 2022 17:04:32 +0200 From: Michael Niedermayer To: FFmpeg development discussions and patches Message-ID: <20220910150432.GH2088045@pb2> References: <20220909034611.390710-1-philipl@overt.org> MIME-Version: 1.0 In-Reply-To: <20220909034611.390710-1-philipl@overt.org> Subject: Re: [FFmpeg-devel] [PATCH] lavu/pixdesc: favour formats where bit depth exactly matches 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: multipart/mixed; boundary="===============1865756730055545458==" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: --===============1865756730055545458== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="Xzwv1pm7hH1NAiSK" Content-Disposition: inline --Xzwv1pm7hH1NAiSK Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Sep 08, 2022 at 08:46:11PM -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. >=20 > 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. >=20 > 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. >=20 > 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. >=20 > 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. >=20 > 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. >=20 > I have added a set of tests around these formats which will fail > without this fix. >=20 > Signed-off-by: Philip Langdale > --- > libavutil/pixdesc.c | 16 +++++- > libavutil/tests/pixfmt_best.c | 93 ++++++++++++++++++++++++++++------- > tests/ref/fate/pixfmt_best | 2 +- > 3 files changed, 89 insertions(+), 22 deletions(-) >=20 > diff --git a/libavutil/pixdesc.c b/libavutil/pixdesc.c > index d7c6ebfdc4..412e257a30 100644 > --- a/libavutil/pixdesc.c > +++ b/libavutil/pixdesc.c > @@ -3004,6 +3004,11 @@ static int get_pix_fmt_score(enum AVPixelFormat ds= t_pix_fmt, > if ((ret =3D get_pix_fmt_depth(&dst_min_depth, &dst_max_depth, dst_p= ix_fmt)) < 0) > return -3; > =20 > + // Favour formats where bit depth exactly matches. If all other scor= ing is > + // equal, we'd rather use a lower bit depth that matches the source. > + if (src_min_depth !=3D dst_min_depth || src_max_depth !=3D dst_max_d= epth) > + score -=3D 64; This adds a penalty for decreasing the depth, that is under FF_LOSS_DEPTH more so if we decreas depth from 16 to 15 bit, that gives a penalty of 2 but increasing from 15 to 16 a penalty of 64. This needs an adjustment also are we intentionally treating a increase from 14 to 15 like 14 to 16 ? or do we want to prefer the closer ? a FF_LOSS_EQUAL_DEPTH / DIFFERENT_DEPTH could be added to keep this more in line with existing code > + > src_color =3D get_color_type(src_desc); > dst_color =3D get_color_type(dst_desc); > if (dst_pix_fmt =3D=3D AV_PIX_FMT_PAL8) > @@ -3020,14 +3025,21 @@ static int get_pix_fmt_score(enum AVPixelFormat d= st_pix_fmt, > } > =20 > if (consider & FF_LOSS_RESOLUTION) { > + // Apply a large penalty if there's a loss of resolution, but al= so apply > + // a small penalty of the dst resolution is higher so that we fa= vour > + // exactly matching formats. > if (dst_desc->log2_chroma_w > src_desc->log2_chroma_w) { > loss |=3D FF_LOSS_RESOLUTION; > score -=3D 256 << dst_desc->log2_chroma_w; > - } > + } else if (dst_desc->log2_chroma_w < src_desc->log2_chroma_w) > + score -=3D 32; > + > if (dst_desc->log2_chroma_h > src_desc->log2_chroma_h) { > loss |=3D FF_LOSS_RESOLUTION; > score -=3D 256 << dst_desc->log2_chroma_h; > - } > + } else if (dst_desc->log2_chroma_h < src_desc->log2_chroma_h) > + score -=3D 32; this would favor 410 -> 411 over 410 -> 420, i dont think that choice is correct in that specific case i do agree though with the basic idea behind this patch thx [...] --=20 Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB I do not agree with what you have to say, but I'll defend to the death your right to say it. -- Voltaire --Xzwv1pm7hH1NAiSK Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iF0EABEIAB0WIQSf8hKLFH72cwut8TNhHseHBAsPqwUCYxynfAAKCRBhHseHBAsP q8vDAJ9OVh8glXBdnc+w9WchkMiBw8tMMgCeKx3QHy3Ek1MFLlRkHPoAD+6SpDg= =/muk -----END PGP SIGNATURE----- --Xzwv1pm7hH1NAiSK-- --===============1865756730055545458== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ 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". --===============1865756730055545458==--