[FFmpeg-devel] [PATCH v4] lavu/pixdesc: favour formats where depth and subsampling exactly match

[Michael Niedermayer]

On Fri, Sep 16, 2022 at 06:44:06PM -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.
> 
>  v3: Remove leftover bits of v1.
>      Remove bit depth penalty scaling to avoid the value being too large
>      in extreme cases (1 bit vs 16 bit).
> 
>  v4: Reduce subsampling penalty to avoid situations where a loss of
>      bit dpeth will have a smaller penalty than having greater than
>      needed chroma resolution.
> 
> Signed-off-by: Philip Langdale <philipl at overt.org>
> ---
>  libavutil/pixdesc.c           |  31 +++++++-
>  libavutil/pixdesc.h           |  15 ++--
>  libavutil/tests/pixfmt_best.c | 129 +++++++++++++++++++++++++++++-----
>  tests/ref/fate/pixfmt_best    |   2 +-
>  4 files changed, 150 insertions(+), 27 deletions(-)

should be ok

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If a bugfix only changes things apparently unrelated to the bug with no
further explanation, that is a good sign that the bugfix is wrong.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20220917/39c7bf83/attachment.sig>