Re: [FFmpeg-devel] [PATCH 1/2] lavc/cbs_av1: fill in ref_frame_sign_bias

From: Mark Thompson <sw@jkqxz.net>
To: ffmpeg-devel@ffmpeg.org
Subject: Re: [FFmpeg-devel] [PATCH 1/2] lavc/cbs_av1: fill in ref_frame_sign_bias
Date: Sun, 4 Feb 2024 16:06:26 +0000
Message-ID: <bfa8a190-7f53-4a9c-9fb5-4d6e30866199@jkqxz.net> (raw)
In-Reply-To: <Npbk3Pz--3-9@lynne.ee>

On 02/02/2024 02:57, Lynne wrote:
> Needed for AV1.
> 
 > From 81be215060a718fdc3d043847e8155ba56fcb431 Mon Sep 17 00:00:00 2001
 > From: Lynne <dev@lynne.ee>
 > Date: Fri, 2 Feb 2024 03:54:06 +0100
 > Subject: [PATCH 1/2] lavc/cbs_av1: fill in ref_frame_sign_bias
 >
 > Needed for AV1.
 > ---
 >  libavcodec/cbs_av1.h                 |  1 +
 >  libavcodec/cbs_av1_syntax_template.c | 10 ++++++++++
 >  2 files changed, 11 insertions(+)
 >
 > diff --git a/libavcodec/cbs_av1.h b/libavcodec/cbs_av1.h
 > index a5402f069d..cbb43ac810 100644
 > --- a/libavcodec/cbs_av1.h
 > +++ b/libavcodec/cbs_av1.h
 > @@ -198,6 +198,7 @@ typedef struct AV1RawFrameHeader {
 >      uint8_t refresh_frame_flags;
 >      uint8_t allow_intrabc;
 >      uint8_t ref_order_hint[AV1_NUM_REF_FRAMES];
 > +    uint8_t ref_frame_sign_bias[AV1_NUM_REF_FRAMES];

This isn't a syntax element so it shouldn't go in the syntax structure.  Put it in the context structure with other derived fields (the pointer is already available as priv where you want it).

 >      uint8_t frame_refs_short_signaling;
 >      uint8_t last_frame_idx;
 >      uint8_t golden_frame_idx;
 > diff --git a/libavcodec/cbs_av1_syntax_template.c b/libavcodec/cbs_av1_syntax_template.c
 > index 3be1f2d30f..00e9a6d030 100644
 > --- a/libavcodec/cbs_av1_syntax_template.c
 > +++ b/libavcodec/cbs_av1_syntax_template.c
 > @@ -1572,6 +1572,16 @@ static int FUNC(uncompressed_header)(CodedBitstreamContext *ctx, RWContext *rw,
 >      }
 >
 >      if (!frame_is_intra) {
 > +        for (i = 0; i < AV1_REFS_PER_FRAME; i++) {
 > +            if (seq->enable_order_hint) {
 > +                int idx = current->ref_frame_idx[i];
 > +                int hint = current->ref_order_hint[idx];
 > +                current->ref_frame_sign_bias[i] = cbs_av1_get_relative_dist(seq, hint,
 > +                                                                            priv->order_hint) > 0;
 > +            } else {
 > +                infer(ref_frame_sign_bias[i], 0);
 > +            }
 > +        }
 >          // Derive reference frame sign biases.
 >      }
 >
 > --
 > 2.43.0.381.gb435a96ce8

Please exactly follow the layout of the specification as far as possible, since that makes it much easier to assess whether it is correct.  It looks like the indexing is wrong?  (Note that LAST_FRAME == 1.)

Thanks,

- Mark
_______________________________________________
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".