[FFmpeg-devel] [PATCH 1/3] avcodec/cbs_av1: infer loop filter delta parameters from reference frames

[Mark Thompson]

On 21/10/2020 01:11, James Almer wrote:
> Partially implements of setup_past_independence() and load_previous().
> These ensures they are always set, even if the values were not coded
> in the input bitstream and will not be coded in the output bitstream.
> 
> Signed-off-by: James Almer <jamrial at gmail.com>
> ---
>   libavcodec/cbs_av1.h                 |  3 +++
>   libavcodec/cbs_av1_syntax_template.c | 40 +++++++++++++++++++++++++---
>   2 files changed, 39 insertions(+), 4 deletions(-)
> 
> diff --git a/libavcodec/cbs_av1.h b/libavcodec/cbs_av1.h
> index 7a0c08c596..97aeee9795 100644
> --- a/libavcodec/cbs_av1.h
> +++ b/libavcodec/cbs_av1.h
> @@ -413,6 +413,9 @@ typedef struct AV1ReferenceFrameState {
>       int subsampling_y;  // RefSubsamplingY
>       int bit_depth;      // RefBitDepth
>       int order_hint;     // RefOrderHint
> +
> +    int8_t  loop_filter_ref_deltas[AV1_TOTAL_REFS_PER_FRAME];
> +    int8_t  loop_filter_mode_deltas[2];
>   } AV1ReferenceFrameState;
>   
>   typedef struct CodedBitstreamAV1Context {
> diff --git a/libavcodec/cbs_av1_syntax_template.c b/libavcodec/cbs_av1_syntax_template.c
> index bcaa334134..4edf4fd47c 100644
> --- a/libavcodec/cbs_av1_syntax_template.c
> +++ b/libavcodec/cbs_av1_syntax_template.c
> @@ -837,6 +837,9 @@ static int FUNC(loop_filter_params)(CodedBitstreamContext *ctx, RWContext *rw,
>                                       AV1RawFrameHeader *current)
>   {
>       CodedBitstreamAV1Context *priv = ctx->priv_data;
> +    static const int8_t default_loop_filter_ref_deltas[AV1_TOTAL_REFS_PER_FRAME] =
> +        { 1, 0, 0, 0, -1, 0, -1, -1 };
> +    static const int8_t default_loop_filter_mode_deltas[2] = { 0 };

I realise it's the same, but the single zero there looks like an error so I think put two of them.

>       int i, err;
>   
>       if (priv->coded_lossless || current->allow_intrabc) {
> @@ -870,19 +873,44 @@ static int FUNC(loop_filter_params)(CodedBitstreamContext *ctx, RWContext *rw,
>   
>       flag(loop_filter_delta_enabled);
>       if (current->loop_filter_delta_enabled) {
> +        const int8_t *loop_filter_ref_deltas, *loop_filter_mode_deltas;

Maybe call these ref_* to make the below a little clearer?  (foo[n] is inferred from ref_foo[n].)

> +
> +        if (current->primary_ref_frame == AV1_PRIMARY_REF_NONE) {
> +            loop_filter_ref_deltas = default_loop_filter_ref_deltas;
> +            loop_filter_mode_deltas = default_loop_filter_mode_deltas;
> +        } else {
> +            loop_filter_ref_deltas =
> +                priv->ref[current->ref_frame_idx[current->primary_ref_frame]].loop_filter_ref_deltas;
> +            loop_filter_mode_deltas =
> +                priv->ref[current->ref_frame_idx[current->primary_ref_frame]].loop_filter_mode_deltas;
> +        }
> +
>           flag(loop_filter_delta_update);
> -        if (current->loop_filter_delta_update) {
>               for (i = 0; i < AV1_TOTAL_REFS_PER_FRAME; i++) {
> -                flags(update_ref_delta[i], 1, i);
> +                if (current->loop_filter_delta_update)
> +                    flags(update_ref_delta[i], 1, i);
> +                else
> +                    infer(update_ref_delta[i], 0);
>                   if (current->update_ref_delta[i])
>                       sus(1 + 6, loop_filter_ref_deltas[i], 1, i);
> +                else
> +                    infer(loop_filter_ref_deltas[i], loop_filter_ref_deltas[i]);
>               }
>               for (i = 0; i < 2; i++) {
> -                flags(update_mode_delta[i], 1, i);
> +                if (current->loop_filter_delta_update)
> +                    flags(update_mode_delta[i], 1, i);
> +                else
> +                    infer(update_mode_delta[i], 0);
>                   if (current->update_mode_delta[i])
>                       sus(1 + 6, loop_filter_mode_deltas[i], 1, i);
> +                else
> +                    infer(loop_filter_mode_deltas[i], loop_filter_mode_deltas[i]);
>               }
> -        }
> +    } else {
> +        for (i = 0; i < AV1_TOTAL_REFS_PER_FRAME; i++)
> +            infer(loop_filter_ref_deltas[i], default_loop_filter_ref_deltas[i]);
> +        for (i = 0; i < 2; i++)
> +            infer(loop_filter_mode_deltas[i], default_loop_filter_mode_deltas[i]);
>       }
>   
>       return 0;
> @@ -1613,6 +1641,10 @@ update_refs:
>                   .bit_depth      = priv->bit_depth,
>                   .order_hint     = priv->order_hint,
>               };
> +            memcpy(priv->ref[i].loop_filter_ref_deltas, current->loop_filter_ref_deltas,
> +                   sizeof(current->loop_filter_ref_deltas));
> +            memcpy(priv->ref[i].loop_filter_mode_deltas, current->loop_filter_mode_deltas,
> +                   sizeof(current->loop_filter_mode_deltas));
>           }
>       }
>   
> 

Looks sensible.

Thanks,

- Mark