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

[Michael Niedermayer]

On Wed, Sep 14, 2022 at 04:20:02PM -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).
> 
> Signed-off-by: Philip Langdale <philipl at overt.org>
> ---
>  libavutil/pixdesc.c           |  31 +++++++++-
>  libavutil/pixdesc.h           |  15 +++--
>  libavutil/tests/pixfmt_best.c | 111 ++++++++++++++++++++++++++++------
>  tests/ref/fate/pixfmt_best    |   2 +-
>  4 files changed, 132 insertions(+), 27 deletions(-)
> 
> diff --git a/libavutil/pixdesc.c b/libavutil/pixdesc.c
> index d7c6ebfdc4..6377224c64 100644
> --- a/libavutil/pixdesc.c
> +++ b/libavutil/pixdesc.c
> @@ -3013,9 +3013,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 += depth_delta;
>          }
>      }
>  
> @@ -3035,6 +3042,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);
> +        }

with 16bit 4:2:0 
to
14bit 4:2:0
vs.
16bit 4:4:4

more data is preserved in the later but the 420->444 would have a loss of 64
i think and 16->14 i think 8. I didnt try this just reading the code so i may
be missing something but i think the code would favor the lower bitdepth
That may be unexpected

thx


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

Complexity theory is the science of finding the exact solution to an
approximation. Benchmarking OTOH is finding an approximation of the exact
-------------- 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/20220915/dcc2f3f0/attachment.sig>