On 11/9/2020 8:02 PM, Mark Thompson wrote:
On 09/11/2020 22:50, Timo Rothenpieler wrote:
On 09.11.2020 23:30, Mark Thompson wrote:
On 09/11/2020 16:30, Timo Rothenpieler wrote:
Signed-off-by: Timo Rothenpieler <t...@rothenpieler.org>
Co-authored-by: James Almer <jamr...@gmail.com>
---
  libavcodec/av1dec.c | 89 +++++++++++++++++++++++++++++++++++++++++++++
  libavcodec/av1dec.h |  3 ++
  2 files changed, 92 insertions(+)

diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
index 56712279aa..83295699e1 100644
--- a/libavcodec/av1dec.c
+++ b/libavcodec/av1dec.c
@@ -145,6 +145,87 @@ static void global_motion_params(AV1DecContext *s)
      }
  }
+static int get_relative_dist(const AV1RawSequenceHeader *seq,
+                             unsigned int a, unsigned int b)
+{
+    unsigned int diff, m;
+    if (!seq->enable_order_hint)
+        return 0;

This never gets called if !enable_order_hint, so the argument isn't needed.

+    diff = a - b;
+    m = 1 << seq->order_hint_bits_minus_1;
+    diff = (diff & (m - 1)) - (diff & m);
+    return diff;
+}
+
+static void skip_mode_params(AV1DecContext *s)
+{
+    const AV1RawFrameHeader *header = s->raw_frame_header;
+    const AV1RawSequenceHeader *seq = s->raw_seq;
+
+    int forward_idx,  backward_idx;
+    int forward_hint, backward_hint;
+    int second_forward_idx, second_forward_hint;
+    int ref_hint, dist, i;
+
+    s->cur_frame.skip_mode_frame_idx[0] = 0;
+    s->cur_frame.skip_mode_frame_idx[1] = 0;

0 is AV1_REF_FRAME_INTRA, which doesn't make sense for skip.  Does this value actually get used?

What else would you set it to in that case?
It's set to 0 to indicate an invalid value.

If it isn't used then don't set it at all?  If it has to be set then I guess use a value outside the 0..7 valid range of ref frames.

He should do whatever cuviddec does, since one would assume is what the hardware expects. primary_ref_frame in CUVIDAV1PICPARAMS for example is set to -1 (255) to signal AV1_PRIMARY_REF_NONE.

Also, both AV1Frame and CUVIDAV1PICPARAMS are zero initialized, so not setting it to anything is the same as setting it to 0.


+
+    if (header->frame_type == AV1_FRAME_KEY ||
+        header->frame_type == AV1_FRAME_INTRA_ONLY ||
+        !header->reference_select || !seq->enable_order_hint)

if (!header->skip_mode_present)

would be much simpler and short-cut some empty cases that this condition misses.

But that's not equivalent to the CBS code, is it?
Cause skip_mode_allowed can be 1 and the flag it reads into skip_mode_present can still be 0?

skip_mode_present is set iff skip modes are present.  If they aren't then calculating what frames could have been used for skip references had they been present is meaningless.

The code is trying to behave the same the skip_mode_params params function in the cbs_av1 parser does.

Which isn't what you want, because here we know more than the parser did.  (Notably: whether frames you have headers for were actually complete, which you are doing.)

+        return;
+
+    forward_idx  = -1;
+    backward_idx = -1;
+    for (i = 0; i < AV1_REFS_PER_FRAME; i++) {
+        ref_hint = s->ref[header->ref_frame_idx[i]].order_hint;
+        dist = get_relative_dist(seq, ref_hint, header->order_hint);
+        if (dist < 0) {
+            if (forward_idx < 0 ||
+                get_relative_dist(seq, ref_hint, forward_hint) > 0) {
+                forward_idx  = i;
+                forward_hint = ref_hint;
+            }
+        } else if (dist > 0) {
+            if (backward_idx < 0 ||
+                get_relative_dist(seq, ref_hint, backward_hint) < 0) {
+                backward_idx  = i;
+                backward_hint = ref_hint;
+            }
+        }
+    }
+
+    if (forward_idx < 0) {
+        return;
+    } else if (backward_idx >= 0) {
+        s->cur_frame.skip_mode_frame_idx[0] =
+            AV1_REF_FRAME_LAST + FFMIN(forward_idx, backward_idx);
+        s->cur_frame.skip_mode_frame_idx[1] =
+            AV1_REF_FRAME_LAST + FFMAX(forward_idx, backward_idx);
+        return;
+    }
+
+    second_forward_idx = -1;
+    for (i = 0; i < AV1_REFS_PER_FRAME; i++) {
+        ref_hint = s->ref[header->ref_frame_idx[i]].order_hint;
+        if (get_relative_dist(seq, ref_hint, forward_hint) < 0) {
+            if (second_forward_idx < 0 ||
+                get_relative_dist(seq, ref_hint, second_forward_hint) > 0) {
+                second_forward_idx  = i;
+                second_forward_hint = ref_hint;
+            }
+        }
+    }
+
+    if (second_forward_idx < 0)
+        return;
+
+    s->cur_frame.skip_mode_frame_idx[0] =
+        AV1_REF_FRAME_LAST + FFMIN(forward_idx, second_forward_idx);
+    s->cur_frame.skip_mode_frame_idx[1] =
+        AV1_REF_FRAME_LAST + FFMAX(forward_idx, second_forward_idx);
+}
+
  static int init_tile_data(AV1DecContext *s)
  {
@@ -318,6 +399,7 @@ static void av1_frame_unref(AVCodecContext *avctx, AV1Frame *f)
      av_buffer_unref(&f->hwaccel_priv_buf);
      f->hwaccel_picture_private = NULL;
      f->spatial_id = f->temporal_id = 0;
+    f->order_hint = 0;
  }
  static int av1_frame_ref(AVCodecContext *avctx, AV1Frame *dst, const AV1Frame *src) @@ -337,12 +419,16 @@ static int av1_frame_ref(AVCodecContext *avctx, AV1Frame *dst, const AV1Frame *s
      dst->spatial_id = src->spatial_id;
      dst->temporal_id = src->temporal_id;
+    memcpy(dst->skip_mode_frame_idx,
+           src->skip_mode_frame_idx,
+           2 * sizeof(uint8_t));

Keeping this in the same order as the structure would make it easier to check that all fields are copied.

      memcpy(dst->gm_type,
             src->gm_type,
             AV1_NUM_REF_FRAMES * sizeof(uint8_t));
      memcpy(dst->gm_params,
             src->gm_params,
             AV1_NUM_REF_FRAMES * 6 * sizeof(int32_t));
+    dst->order_hint = src->order_hint;
      return 0;
@@ -613,6 +699,7 @@ static int get_current_frame(AVCodecContext *avctx)
      }
      global_motion_params(s);
+    skip_mode_params(s);
      return ret;
  }
@@ -740,6 +827,8 @@ static int av1_decode_frame(AVCodecContext *avctx, void *frame,
              s->cur_frame.spatial_id  = header->spatial_id;
              s->cur_frame.temporal_id = header->temporal_id;
+            s->cur_frame.order_hint = s->raw_frame_header->order_hint;
+
              if (avctx->hwaccel) {
                  ret = avctx->hwaccel->start_frame(avctx, unit->data,
                                                    unit->data_size);
diff --git a/libavcodec/av1dec.h b/libavcodec/av1dec.h
index b58bc53961..8cf50f0d59 100644
--- a/libavcodec/av1dec.h
+++ b/libavcodec/av1dec.h
@@ -41,6 +41,9 @@ typedef struct AV1Frame {
      uint8_t gm_type[AV1_NUM_REF_FRAMES];
      int32_t gm_params[AV1_NUM_REF_FRAMES][6];
+
+    uint8_t order_hint;
+    uint8_t skip_mode_frame_idx[2];
  } AV1Frame;
  typedef struct TileGroupInfo {


- 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-requ...@ffmpeg.org with subject "unsubscribe".

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Reply via email to