[FFmpeg-devel] [PATCH] avcodec/cbs_av1: avoid reset some members of loop filter

[James Almer]

On 12/13/2019 6:08 AM, Fei Wang wrote:
> According to spec 6.8.10, loop_filter_ref_deltas & loop_filter_mode_deltas
> should be keep same as its previous value if it is not presented in current
> syntax.
> 
> Signed-off-by: Fei Wang <fei.w.wang at intel.com>
> ---
>  libavcodec/cbs_av1.h                 |  3 +++
>  libavcodec/cbs_av1_syntax_template.c | 24 +++++++++++++++++++++++-
>  2 files changed, 26 insertions(+), 1 deletion(-)
> 
> diff --git a/libavcodec/cbs_av1.h b/libavcodec/cbs_av1.h
> index 643e76793f..7267baaceb 100644
> --- a/libavcodec/cbs_av1.h
> +++ b/libavcodec/cbs_av1.h
> @@ -444,6 +444,9 @@ typedef struct CodedBitstreamAV1Context {
>      AV1ReferenceFrameState *ref;
>      AV1ReferenceFrameState read_ref[AV1_NUM_REF_FRAMES];
>      AV1ReferenceFrameState write_ref[AV1_NUM_REF_FRAMES];
> +
> +    int8_t pre_loop_filter_ref_deltas[AV1_TOTAL_REFS_PER_FRAME];
> +    int8_t pre_loop_filter_mode_deltas[2];
>  } CodedBitstreamAV1Context;
>  
>  
> diff --git a/libavcodec/cbs_av1_syntax_template.c b/libavcodec/cbs_av1_syntax_template.c
> index f53955c52e..c9ac1dc600 100644
> --- a/libavcodec/cbs_av1_syntax_template.c
> +++ b/libavcodec/cbs_av1_syntax_template.c
> @@ -819,6 +819,13 @@ static int FUNC(loop_filter_params)(CodedBitstreamContext *ctx, RWContext *rw,
>      CodedBitstreamAV1Context *priv = ctx->priv_data;
>      int i, err;
>  
> +    memcpy(current->loop_filter_ref_deltas,
> +            priv->pre_loop_filter_ref_deltas,
> +            AV1_TOTAL_REFS_PER_FRAME * sizeof(int8_t));
> +    memcpy(current->loop_filter_mode_deltas,
> +            priv->pre_loop_filter_mode_deltas,
> +            2 * sizeof(int8_t));
> +
>      if (priv->coded_lossless || current->allow_intrabc) {
>          infer(loop_filter_level[0], 0);
>          infer(loop_filter_level[1], 0);
> @@ -832,7 +839,15 @@ static int FUNC(loop_filter_params)(CodedBitstreamContext *ctx, RWContext *rw,
>          infer(loop_filter_ref_deltas[AV1_REF_FRAME_ALTREF2], -1);
>          for (i = 0; i < 2; i++)
>              infer(loop_filter_mode_deltas[i], 0);
> -        return 0;
> +
> +        memcpy(priv->pre_loop_filter_ref_deltas,
> +                current->loop_filter_ref_deltas,
> +                AV1_TOTAL_REFS_PER_FRAME * sizeof(int8_t));
> +        memcpy(priv->pre_loop_filter_mode_deltas,
> +                current->loop_filter_mode_deltas,
> +                2 * sizeof(int8_t));
> +
> +	return 0;
>      }
>  
>      fb(6, loop_filter_level[0]);
> @@ -865,6 +880,13 @@ static int FUNC(loop_filter_params)(CodedBitstreamContext *ctx, RWContext *rw,
>          }
>      }
>  
> +    memcpy(priv->pre_loop_filter_ref_deltas,
> +            current->loop_filter_ref_deltas,
> +            AV1_TOTAL_REFS_PER_FRAME * sizeof(int8_t));
> +    memcpy(priv->pre_loop_filter_mode_deltas,
> +            current->loop_filter_mode_deltas,
> +            2 * sizeof(int8_t));
> +
>      return 0;
>  }

You're trying to implement the load_previous() function defined in 6.8.2
but not following the requirement of using the values from
ref_frame_idx[primary_ref_frame]. You're instead loading the values from
whatever frame was last parsed.

Also, all this is for decoding. Within CBS, if it's not needed to get
correct parsing of some explicitly coded value in the bitstream, then
there's no reason to implement it.
CBS users can, if needed and for any given frame, take these values from
priv->ref[frame->ref_frame_idx[frame->primary_ref_frame]], effectively
implementing load_previous().