| Message ID | 20201109163057.18005-2-timo@rothenpieler.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | Add NVDEC AV1 hwaccel | expand |
| Context | Check | Description |
|---|---|---|
| andriy/x86_make | success | Make finished |
| andriy/x86_make_fate | success | Make fate finished |
On 09/11/2020 16:30, Timo Rothenpieler wrote: > Signed-off-by: Timo Rothenpieler <timo@rothenpieler.org> > Co-authored-by: James Almer <jamrial@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? > + > + 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. > + 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
On 09.11.2020 23:30, Mark Thompson wrote: > On 09/11/2020 16:30, Timo Rothenpieler wrote: >> Signed-off-by: Timo Rothenpieler <timo@rothenpieler.org> >> Co-authored-by: James Almer <jamrial@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@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
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 <timo@rothenpieler.org> >>> Co-authored-by: James Almer <jamrial@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. >>> + >>> + 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
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 <timo@rothenpieler.org> >>>> Co-authored-by: James Almer <jamrial@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-request@ffmpeg.org with subject "unsubscribe".
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; + 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; + + if (header->frame_type == AV1_FRAME_KEY || + header->frame_type == AV1_FRAME_INTRA_ONLY || + !header->reference_select || !seq->enable_order_hint) + 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)); 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 {
Signed-off-by: Timo Rothenpieler <timo@rothenpieler.org> Co-authored-by: James Almer <jamrial@gmail.com> --- libavcodec/av1dec.c | 89 +++++++++++++++++++++++++++++++++++++++++++++ libavcodec/av1dec.h | 3 ++ 2 files changed, 92 insertions(+)