[FFmpeg-devel] [PATCH 1/4] avcodec/av1dec: expose skip mode frame index
Timo Rothenpieler
timo at rothenpieler.org
Tue Nov 10 00:50:55 EET 2020
On 09.11.2020 23:30, Mark Thompson wrote:
> On 09/11/2020 16:30, Timo Rothenpieler wrote:
>> Signed-off-by: Timo Rothenpieler <timo at rothenpieler.org>
>> Co-authored-by: James Almer <jamrial at 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 (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?
The code is trying to behave the same the skip_mode_params params
function in the cbs_av1 parser does.
>> + 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 at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
More information about the ffmpeg-devel
mailing list