| Message ID | 20201021001126.14044-1-jamrial@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [FFmpeg-devel,1/3] avcodec/cbs_av1: infer loop filter delta parameters from reference frames | expand |
| Context | Check | Description |
|---|---|---|
| andriy/x86_make | success | Make finished |
| andriy/x86_make_fate | success | Make fate finished |
On 21/10/2020 01:11, James Almer wrote: > Partially implements of setup_past_independence() and load_previous(). > These ensures they are always set, even if the values were not coded > in the input bitstream and will not be coded in the output bitstream. > > Signed-off-by: James Almer <jamrial@gmail.com> > --- > libavcodec/cbs_av1.h | 3 +++ > libavcodec/cbs_av1_syntax_template.c | 40 +++++++++++++++++++++++++--- > 2 files changed, 39 insertions(+), 4 deletions(-) > > diff --git a/libavcodec/cbs_av1.h b/libavcodec/cbs_av1.h > index 7a0c08c596..97aeee9795 100644 > --- a/libavcodec/cbs_av1.h > +++ b/libavcodec/cbs_av1.h > @@ -413,6 +413,9 @@ typedef struct AV1ReferenceFrameState { > int subsampling_y; // RefSubsamplingY > int bit_depth; // RefBitDepth > int order_hint; // RefOrderHint > + > + int8_t loop_filter_ref_deltas[AV1_TOTAL_REFS_PER_FRAME]; > + int8_t loop_filter_mode_deltas[2]; > } AV1ReferenceFrameState; > > typedef struct CodedBitstreamAV1Context { > diff --git a/libavcodec/cbs_av1_syntax_template.c b/libavcodec/cbs_av1_syntax_template.c > index bcaa334134..4edf4fd47c 100644 > --- a/libavcodec/cbs_av1_syntax_template.c > +++ b/libavcodec/cbs_av1_syntax_template.c > @@ -837,6 +837,9 @@ static int FUNC(loop_filter_params)(CodedBitstreamContext *ctx, RWContext *rw, > AV1RawFrameHeader *current) > { > CodedBitstreamAV1Context *priv = ctx->priv_data; > + static const int8_t default_loop_filter_ref_deltas[AV1_TOTAL_REFS_PER_FRAME] = > + { 1, 0, 0, 0, -1, 0, -1, -1 }; > + static const int8_t default_loop_filter_mode_deltas[2] = { 0 }; I realise it's the same, but the single zero there looks like an error so I think put two of them. > int i, err; > > if (priv->coded_lossless || current->allow_intrabc) { > @@ -870,19 +873,44 @@ static int FUNC(loop_filter_params)(CodedBitstreamContext *ctx, RWContext *rw, > > flag(loop_filter_delta_enabled); > if (current->loop_filter_delta_enabled) { > + const int8_t *loop_filter_ref_deltas, *loop_filter_mode_deltas; Maybe call these ref_* to make the below a little clearer? (foo[n] is inferred from ref_foo[n].) > + > + if (current->primary_ref_frame == AV1_PRIMARY_REF_NONE) { > + loop_filter_ref_deltas = default_loop_filter_ref_deltas; > + loop_filter_mode_deltas = default_loop_filter_mode_deltas; > + } else { > + loop_filter_ref_deltas = > + priv->ref[current->ref_frame_idx[current->primary_ref_frame]].loop_filter_ref_deltas; > + loop_filter_mode_deltas = > + priv->ref[current->ref_frame_idx[current->primary_ref_frame]].loop_filter_mode_deltas; > + } > + > flag(loop_filter_delta_update); > - if (current->loop_filter_delta_update) { > for (i = 0; i < AV1_TOTAL_REFS_PER_FRAME; i++) { > - flags(update_ref_delta[i], 1, i); > + if (current->loop_filter_delta_update) > + flags(update_ref_delta[i], 1, i); > + else > + infer(update_ref_delta[i], 0); > if (current->update_ref_delta[i]) > sus(1 + 6, loop_filter_ref_deltas[i], 1, i); > + else > + infer(loop_filter_ref_deltas[i], loop_filter_ref_deltas[i]); > } > for (i = 0; i < 2; i++) { > - flags(update_mode_delta[i], 1, i); > + if (current->loop_filter_delta_update) > + flags(update_mode_delta[i], 1, i); > + else > + infer(update_mode_delta[i], 0); > if (current->update_mode_delta[i]) > sus(1 + 6, loop_filter_mode_deltas[i], 1, i); > + else > + infer(loop_filter_mode_deltas[i], loop_filter_mode_deltas[i]); > } > - } > + } else { > + for (i = 0; i < AV1_TOTAL_REFS_PER_FRAME; i++) > + infer(loop_filter_ref_deltas[i], default_loop_filter_ref_deltas[i]); > + for (i = 0; i < 2; i++) > + infer(loop_filter_mode_deltas[i], default_loop_filter_mode_deltas[i]); > } > > return 0; > @@ -1613,6 +1641,10 @@ update_refs: > .bit_depth = priv->bit_depth, > .order_hint = priv->order_hint, > }; > + memcpy(priv->ref[i].loop_filter_ref_deltas, current->loop_filter_ref_deltas, > + sizeof(current->loop_filter_ref_deltas)); > + memcpy(priv->ref[i].loop_filter_mode_deltas, current->loop_filter_mode_deltas, > + sizeof(current->loop_filter_mode_deltas)); > } > } > > Looks sensible. Thanks, - Mark
On 10/27/2020 5:38 PM, Mark Thompson wrote: > On 21/10/2020 01:11, James Almer wrote: >> Partially implements of setup_past_independence() and load_previous(). >> These ensures they are always set, even if the values were not coded >> in the input bitstream and will not be coded in the output bitstream. >> >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> libavcodec/cbs_av1.h | 3 +++ >> libavcodec/cbs_av1_syntax_template.c | 40 +++++++++++++++++++++++++--- >> 2 files changed, 39 insertions(+), 4 deletions(-) >> >> diff --git a/libavcodec/cbs_av1.h b/libavcodec/cbs_av1.h >> index 7a0c08c596..97aeee9795 100644 >> --- a/libavcodec/cbs_av1.h >> +++ b/libavcodec/cbs_av1.h >> @@ -413,6 +413,9 @@ typedef struct AV1ReferenceFrameState { >> int subsampling_y; // RefSubsamplingY >> int bit_depth; // RefBitDepth >> int order_hint; // RefOrderHint >> + >> + int8_t loop_filter_ref_deltas[AV1_TOTAL_REFS_PER_FRAME]; >> + int8_t loop_filter_mode_deltas[2]; >> } AV1ReferenceFrameState; >> typedef struct CodedBitstreamAV1Context { >> diff --git a/libavcodec/cbs_av1_syntax_template.c >> b/libavcodec/cbs_av1_syntax_template.c >> index bcaa334134..4edf4fd47c 100644 >> --- a/libavcodec/cbs_av1_syntax_template.c >> +++ b/libavcodec/cbs_av1_syntax_template.c >> @@ -837,6 +837,9 @@ static int >> FUNC(loop_filter_params)(CodedBitstreamContext *ctx, RWContext *rw, >> AV1RawFrameHeader *current) >> { >> CodedBitstreamAV1Context *priv = ctx->priv_data; >> + static const int8_t >> default_loop_filter_ref_deltas[AV1_TOTAL_REFS_PER_FRAME] = >> + { 1, 0, 0, 0, -1, 0, -1, -1 }; >> + static const int8_t default_loop_filter_mode_deltas[2] = { 0 }; > > I realise it's the same, but the single zero there looks like an error > so I think put two of them. > >> int i, err; >> if (priv->coded_lossless || current->allow_intrabc) { >> @@ -870,19 +873,44 @@ static int >> FUNC(loop_filter_params)(CodedBitstreamContext *ctx, RWContext *rw, >> flag(loop_filter_delta_enabled); >> if (current->loop_filter_delta_enabled) { >> + const int8_t *loop_filter_ref_deltas, *loop_filter_mode_deltas; > > Maybe call these ref_* to make the below a little clearer? (foo[n] is > inferred from ref_foo[n].) > >> + >> + if (current->primary_ref_frame == AV1_PRIMARY_REF_NONE) { >> + loop_filter_ref_deltas = default_loop_filter_ref_deltas; >> + loop_filter_mode_deltas = default_loop_filter_mode_deltas; >> + } else { >> + loop_filter_ref_deltas = >> + >> priv->ref[current->ref_frame_idx[current->primary_ref_frame]].loop_filter_ref_deltas; >> >> + loop_filter_mode_deltas = >> + >> priv->ref[current->ref_frame_idx[current->primary_ref_frame]].loop_filter_mode_deltas; >> >> + } >> + >> flag(loop_filter_delta_update); >> - if (current->loop_filter_delta_update) { >> for (i = 0; i < AV1_TOTAL_REFS_PER_FRAME; i++) { >> - flags(update_ref_delta[i], 1, i); >> + if (current->loop_filter_delta_update) >> + flags(update_ref_delta[i], 1, i); >> + else >> + infer(update_ref_delta[i], 0); >> if (current->update_ref_delta[i]) >> sus(1 + 6, loop_filter_ref_deltas[i], 1, i); >> + else >> + infer(loop_filter_ref_deltas[i], >> loop_filter_ref_deltas[i]); >> } >> for (i = 0; i < 2; i++) { >> - flags(update_mode_delta[i], 1, i); >> + if (current->loop_filter_delta_update) >> + flags(update_mode_delta[i], 1, i); >> + else >> + infer(update_mode_delta[i], 0); >> if (current->update_mode_delta[i]) >> sus(1 + 6, loop_filter_mode_deltas[i], 1, i); >> + else >> + infer(loop_filter_mode_deltas[i], >> loop_filter_mode_deltas[i]); >> } >> - } >> + } else { >> + for (i = 0; i < AV1_TOTAL_REFS_PER_FRAME; i++) >> + infer(loop_filter_ref_deltas[i], >> default_loop_filter_ref_deltas[i]); >> + for (i = 0; i < 2; i++) >> + infer(loop_filter_mode_deltas[i], >> default_loop_filter_mode_deltas[i]); >> } >> return 0; >> @@ -1613,6 +1641,10 @@ update_refs: >> .bit_depth = priv->bit_depth, >> .order_hint = priv->order_hint, >> }; >> + memcpy(priv->ref[i].loop_filter_ref_deltas, >> current->loop_filter_ref_deltas, >> + sizeof(current->loop_filter_ref_deltas)); >> + memcpy(priv->ref[i].loop_filter_mode_deltas, >> current->loop_filter_mode_deltas, >> + sizeof(current->loop_filter_mode_deltas)); >> } >> } >> > > Looks sensible. If you'd rather let the caller derive these values (because atm, none of the bsfs care, only av1dec), then that's fine by me too. See https://github.com/jamrial/FFmpeg/commit/0803491e2e794a8d6cf409432b4d970da68a717b for how it would be done there (replacing patch 3 in this set). I personally prefer the approach in this patch since it works for all cases and prevents code duplication in callers. Patch 2 in this set however is probably needed because feature_enabled[] and feature_value[] are both taken into account when setting coded_lossless after segmentation_params() was called. > > 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".
On 27/10/2020 20:53, James Almer wrote: > On 10/27/2020 5:38 PM, Mark Thompson wrote: >> On 21/10/2020 01:11, James Almer wrote: >>> Partially implements of setup_past_independence() and load_previous(). >>> These ensures they are always set, even if the values were not coded >>> in the input bitstream and will not be coded in the output bitstream. >>> >>> Signed-off-by: James Almer <jamrial@gmail.com> >>> --- >>> libavcodec/cbs_av1.h | 3 +++ >>> libavcodec/cbs_av1_syntax_template.c | 40 +++++++++++++++++++++++++--- >>> 2 files changed, 39 insertions(+), 4 deletions(-) >>> >>> diff --git a/libavcodec/cbs_av1.h b/libavcodec/cbs_av1.h >>> index 7a0c08c596..97aeee9795 100644 >>> --- a/libavcodec/cbs_av1.h >>> +++ b/libavcodec/cbs_av1.h >>> @@ -413,6 +413,9 @@ typedef struct AV1ReferenceFrameState { >>> int subsampling_y; // RefSubsamplingY >>> int bit_depth; // RefBitDepth >>> int order_hint; // RefOrderHint >>> + >>> + int8_t loop_filter_ref_deltas[AV1_TOTAL_REFS_PER_FRAME]; >>> + int8_t loop_filter_mode_deltas[2]; >>> } AV1ReferenceFrameState; >>> typedef struct CodedBitstreamAV1Context { >>> diff --git a/libavcodec/cbs_av1_syntax_template.c >>> b/libavcodec/cbs_av1_syntax_template.c >>> index bcaa334134..4edf4fd47c 100644 >>> --- a/libavcodec/cbs_av1_syntax_template.c >>> +++ b/libavcodec/cbs_av1_syntax_template.c >>> @@ -837,6 +837,9 @@ static int >>> FUNC(loop_filter_params)(CodedBitstreamContext *ctx, RWContext *rw, >>> AV1RawFrameHeader *current) >>> { >>> CodedBitstreamAV1Context *priv = ctx->priv_data; >>> + static const int8_t >>> default_loop_filter_ref_deltas[AV1_TOTAL_REFS_PER_FRAME] = >>> + { 1, 0, 0, 0, -1, 0, -1, -1 }; >>> + static const int8_t default_loop_filter_mode_deltas[2] = { 0 }; >> >> I realise it's the same, but the single zero there looks like an error >> so I think put two of them. >> >>> int i, err; >>> if (priv->coded_lossless || current->allow_intrabc) { >>> @@ -870,19 +873,44 @@ static int >>> FUNC(loop_filter_params)(CodedBitstreamContext *ctx, RWContext *rw, >>> flag(loop_filter_delta_enabled); >>> if (current->loop_filter_delta_enabled) { >>> + const int8_t *loop_filter_ref_deltas, *loop_filter_mode_deltas; >> >> Maybe call these ref_* to make the below a little clearer? (foo[n] is >> inferred from ref_foo[n].) >> >>> + >>> + if (current->primary_ref_frame == AV1_PRIMARY_REF_NONE) { >>> + loop_filter_ref_deltas = default_loop_filter_ref_deltas; >>> + loop_filter_mode_deltas = default_loop_filter_mode_deltas; >>> + } else { >>> + loop_filter_ref_deltas = >>> + >>> priv->ref[current->ref_frame_idx[current->primary_ref_frame]].loop_filter_ref_deltas; >>> >>> + loop_filter_mode_deltas = >>> + >>> priv->ref[current->ref_frame_idx[current->primary_ref_frame]].loop_filter_mode_deltas; >>> >>> + } >>> + >>> flag(loop_filter_delta_update); >>> - if (current->loop_filter_delta_update) { >>> for (i = 0; i < AV1_TOTAL_REFS_PER_FRAME; i++) { >>> - flags(update_ref_delta[i], 1, i); >>> + if (current->loop_filter_delta_update) >>> + flags(update_ref_delta[i], 1, i); >>> + else >>> + infer(update_ref_delta[i], 0); >>> if (current->update_ref_delta[i]) >>> sus(1 + 6, loop_filter_ref_deltas[i], 1, i); >>> + else >>> + infer(loop_filter_ref_deltas[i], >>> loop_filter_ref_deltas[i]); >>> } >>> for (i = 0; i < 2; i++) { >>> - flags(update_mode_delta[i], 1, i); >>> + if (current->loop_filter_delta_update) >>> + flags(update_mode_delta[i], 1, i); >>> + else >>> + infer(update_mode_delta[i], 0); >>> if (current->update_mode_delta[i]) >>> sus(1 + 6, loop_filter_mode_deltas[i], 1, i); >>> + else >>> + infer(loop_filter_mode_deltas[i], >>> loop_filter_mode_deltas[i]); >>> } >>> - } >>> + } else { >>> + for (i = 0; i < AV1_TOTAL_REFS_PER_FRAME; i++) >>> + infer(loop_filter_ref_deltas[i], >>> default_loop_filter_ref_deltas[i]); >>> + for (i = 0; i < 2; i++) >>> + infer(loop_filter_mode_deltas[i], >>> default_loop_filter_mode_deltas[i]); >>> } >>> return 0; >>> @@ -1613,6 +1641,10 @@ update_refs: >>> .bit_depth = priv->bit_depth, >>> .order_hint = priv->order_hint, >>> }; >>> + memcpy(priv->ref[i].loop_filter_ref_deltas, >>> current->loop_filter_ref_deltas, >>> + sizeof(current->loop_filter_ref_deltas)); >>> + memcpy(priv->ref[i].loop_filter_mode_deltas, >>> current->loop_filter_mode_deltas, >>> + sizeof(current->loop_filter_mode_deltas)); >>> } >>> } >>> >> >> Looks sensible. > > If you'd rather let the caller derive these values (because atm, none of > the bsfs care, only av1dec), then that's fine by me too. > See > https://github.com/jamrial/FFmpeg/commit/0803491e2e794a8d6cf409432b4d970da68a717b > for how it would be done there (replacing patch 3 in this set). > I personally prefer the approach in this patch since it works for all > cases and prevents code duplication in callers. > > Patch 2 in this set however is probably needed because feature_enabled[] > and feature_value[] are both taken into account when setting > coded_lossless after segmentation_params() was called. Yes, I agree with the preference for this one. It's a bitstream value so inferring the cases where it isn't actually present is reasonable and has little overhead (when in H.26n standards you get a nice "when not present, the value of foo is inferred to be X" line); it's only a problem when we start trying to carry a lot of derived values around. (I'm still rather dubious on gm_params, though I guess I could be convinced.) - Mark
On 10/27/2020 6:12 PM, Mark Thompson wrote: > On 27/10/2020 20:53, James Almer wrote: >> On 10/27/2020 5:38 PM, Mark Thompson wrote: >>> On 21/10/2020 01:11, James Almer wrote: >>>> Partially implements of setup_past_independence() and load_previous(). >>>> These ensures they are always set, even if the values were not coded >>>> in the input bitstream and will not be coded in the output bitstream. >>>> >>>> Signed-off-by: James Almer <jamrial@gmail.com> >>>> --- >>>> libavcodec/cbs_av1.h | 3 +++ >>>> libavcodec/cbs_av1_syntax_template.c | 40 >>>> +++++++++++++++++++++++++--- >>>> 2 files changed, 39 insertions(+), 4 deletions(-) >>>> >>>> diff --git a/libavcodec/cbs_av1.h b/libavcodec/cbs_av1.h >>>> index 7a0c08c596..97aeee9795 100644 >>>> --- a/libavcodec/cbs_av1.h >>>> +++ b/libavcodec/cbs_av1.h >>>> @@ -413,6 +413,9 @@ typedef struct AV1ReferenceFrameState { >>>> int subsampling_y; // RefSubsamplingY >>>> int bit_depth; // RefBitDepth >>>> int order_hint; // RefOrderHint >>>> + >>>> + int8_t loop_filter_ref_deltas[AV1_TOTAL_REFS_PER_FRAME]; >>>> + int8_t loop_filter_mode_deltas[2]; >>>> } AV1ReferenceFrameState; >>>> typedef struct CodedBitstreamAV1Context { >>>> diff --git a/libavcodec/cbs_av1_syntax_template.c >>>> b/libavcodec/cbs_av1_syntax_template.c >>>> index bcaa334134..4edf4fd47c 100644 >>>> --- a/libavcodec/cbs_av1_syntax_template.c >>>> +++ b/libavcodec/cbs_av1_syntax_template.c >>>> @@ -837,6 +837,9 @@ static int >>>> FUNC(loop_filter_params)(CodedBitstreamContext *ctx, RWContext *rw, >>>> AV1RawFrameHeader *current) >>>> { >>>> CodedBitstreamAV1Context *priv = ctx->priv_data; >>>> + static const int8_t >>>> default_loop_filter_ref_deltas[AV1_TOTAL_REFS_PER_FRAME] = >>>> + { 1, 0, 0, 0, -1, 0, -1, -1 }; >>>> + static const int8_t default_loop_filter_mode_deltas[2] = { 0 }; >>> >>> I realise it's the same, but the single zero there looks like an error >>> so I think put two of them. >>> >>>> int i, err; >>>> if (priv->coded_lossless || current->allow_intrabc) { >>>> @@ -870,19 +873,44 @@ static int >>>> FUNC(loop_filter_params)(CodedBitstreamContext *ctx, RWContext *rw, >>>> flag(loop_filter_delta_enabled); >>>> if (current->loop_filter_delta_enabled) { >>>> + const int8_t *loop_filter_ref_deltas, >>>> *loop_filter_mode_deltas; >>> >>> Maybe call these ref_* to make the below a little clearer? (foo[n] is >>> inferred from ref_foo[n].) >>> >>>> + >>>> + if (current->primary_ref_frame == AV1_PRIMARY_REF_NONE) { >>>> + loop_filter_ref_deltas = default_loop_filter_ref_deltas; >>>> + loop_filter_mode_deltas = default_loop_filter_mode_deltas; >>>> + } else { >>>> + loop_filter_ref_deltas = >>>> + >>>> priv->ref[current->ref_frame_idx[current->primary_ref_frame]].loop_filter_ref_deltas; >>>> >>>> >>>> + loop_filter_mode_deltas = >>>> + >>>> priv->ref[current->ref_frame_idx[current->primary_ref_frame]].loop_filter_mode_deltas; >>>> >>>> >>>> + } >>>> + >>>> flag(loop_filter_delta_update); >>>> - if (current->loop_filter_delta_update) { >>>> for (i = 0; i < AV1_TOTAL_REFS_PER_FRAME; i++) { >>>> - flags(update_ref_delta[i], 1, i); >>>> + if (current->loop_filter_delta_update) >>>> + flags(update_ref_delta[i], 1, i); >>>> + else >>>> + infer(update_ref_delta[i], 0); >>>> if (current->update_ref_delta[i]) >>>> sus(1 + 6, loop_filter_ref_deltas[i], 1, i); >>>> + else >>>> + infer(loop_filter_ref_deltas[i], >>>> loop_filter_ref_deltas[i]); >>>> } >>>> for (i = 0; i < 2; i++) { >>>> - flags(update_mode_delta[i], 1, i); >>>> + if (current->loop_filter_delta_update) >>>> + flags(update_mode_delta[i], 1, i); >>>> + else >>>> + infer(update_mode_delta[i], 0); >>>> if (current->update_mode_delta[i]) >>>> sus(1 + 6, loop_filter_mode_deltas[i], 1, i); >>>> + else >>>> + infer(loop_filter_mode_deltas[i], >>>> loop_filter_mode_deltas[i]); >>>> } >>>> - } >>>> + } else { >>>> + for (i = 0; i < AV1_TOTAL_REFS_PER_FRAME; i++) >>>> + infer(loop_filter_ref_deltas[i], >>>> default_loop_filter_ref_deltas[i]); >>>> + for (i = 0; i < 2; i++) >>>> + infer(loop_filter_mode_deltas[i], >>>> default_loop_filter_mode_deltas[i]); >>>> } >>>> return 0; >>>> @@ -1613,6 +1641,10 @@ update_refs: >>>> .bit_depth = priv->bit_depth, >>>> .order_hint = priv->order_hint, >>>> }; >>>> + memcpy(priv->ref[i].loop_filter_ref_deltas, >>>> current->loop_filter_ref_deltas, >>>> + sizeof(current->loop_filter_ref_deltas)); >>>> + memcpy(priv->ref[i].loop_filter_mode_deltas, >>>> current->loop_filter_mode_deltas, >>>> + sizeof(current->loop_filter_mode_deltas)); >>>> } >>>> } >>>> >>> >>> Looks sensible. >> >> If you'd rather let the caller derive these values (because atm, none of >> the bsfs care, only av1dec), then that's fine by me too. >> See >> https://github.com/jamrial/FFmpeg/commit/0803491e2e794a8d6cf409432b4d970da68a717b >> >> for how it would be done there (replacing patch 3 in this set). >> I personally prefer the approach in this patch since it works for all >> cases and prevents code duplication in callers. >> >> Patch 2 in this set however is probably needed because feature_enabled[] >> and feature_value[] are both taken into account when setting >> coded_lossless after segmentation_params() was called. > > Yes, I agree with the preference for this one. It's a bitstream value > so inferring the cases where it isn't actually present is reasonable and > has little overhead (when in H.26n standards you get a nice "when not > present, the value of foo is inferred to be X" line); it's only a > problem when we start trying to carry a lot of derived values around. > > (I'm still rather dubious on gm_params, though I guess I could be > convinced.) > > - Mark Made the changes you requested and applied, thanks.
diff --git a/libavcodec/cbs_av1.h b/libavcodec/cbs_av1.h index 7a0c08c596..97aeee9795 100644 --- a/libavcodec/cbs_av1.h +++ b/libavcodec/cbs_av1.h @@ -413,6 +413,9 @@ typedef struct AV1ReferenceFrameState { int subsampling_y; // RefSubsamplingY int bit_depth; // RefBitDepth int order_hint; // RefOrderHint + + int8_t loop_filter_ref_deltas[AV1_TOTAL_REFS_PER_FRAME]; + int8_t loop_filter_mode_deltas[2]; } AV1ReferenceFrameState; typedef struct CodedBitstreamAV1Context { diff --git a/libavcodec/cbs_av1_syntax_template.c b/libavcodec/cbs_av1_syntax_template.c index bcaa334134..4edf4fd47c 100644 --- a/libavcodec/cbs_av1_syntax_template.c +++ b/libavcodec/cbs_av1_syntax_template.c @@ -837,6 +837,9 @@ static int FUNC(loop_filter_params)(CodedBitstreamContext *ctx, RWContext *rw, AV1RawFrameHeader *current) { CodedBitstreamAV1Context *priv = ctx->priv_data; + static const int8_t default_loop_filter_ref_deltas[AV1_TOTAL_REFS_PER_FRAME] = + { 1, 0, 0, 0, -1, 0, -1, -1 }; + static const int8_t default_loop_filter_mode_deltas[2] = { 0 }; int i, err; if (priv->coded_lossless || current->allow_intrabc) { @@ -870,19 +873,44 @@ static int FUNC(loop_filter_params)(CodedBitstreamContext *ctx, RWContext *rw, flag(loop_filter_delta_enabled); if (current->loop_filter_delta_enabled) { + const int8_t *loop_filter_ref_deltas, *loop_filter_mode_deltas; + + if (current->primary_ref_frame == AV1_PRIMARY_REF_NONE) { + loop_filter_ref_deltas = default_loop_filter_ref_deltas; + loop_filter_mode_deltas = default_loop_filter_mode_deltas; + } else { + loop_filter_ref_deltas = + priv->ref[current->ref_frame_idx[current->primary_ref_frame]].loop_filter_ref_deltas; + loop_filter_mode_deltas = + priv->ref[current->ref_frame_idx[current->primary_ref_frame]].loop_filter_mode_deltas; + } + flag(loop_filter_delta_update); - if (current->loop_filter_delta_update) { for (i = 0; i < AV1_TOTAL_REFS_PER_FRAME; i++) { - flags(update_ref_delta[i], 1, i); + if (current->loop_filter_delta_update) + flags(update_ref_delta[i], 1, i); + else + infer(update_ref_delta[i], 0); if (current->update_ref_delta[i]) sus(1 + 6, loop_filter_ref_deltas[i], 1, i); + else + infer(loop_filter_ref_deltas[i], loop_filter_ref_deltas[i]); } for (i = 0; i < 2; i++) { - flags(update_mode_delta[i], 1, i); + if (current->loop_filter_delta_update) + flags(update_mode_delta[i], 1, i); + else + infer(update_mode_delta[i], 0); if (current->update_mode_delta[i]) sus(1 + 6, loop_filter_mode_deltas[i], 1, i); + else + infer(loop_filter_mode_deltas[i], loop_filter_mode_deltas[i]); } - } + } else { + for (i = 0; i < AV1_TOTAL_REFS_PER_FRAME; i++) + infer(loop_filter_ref_deltas[i], default_loop_filter_ref_deltas[i]); + for (i = 0; i < 2; i++) + infer(loop_filter_mode_deltas[i], default_loop_filter_mode_deltas[i]); } return 0; @@ -1613,6 +1641,10 @@ update_refs: .bit_depth = priv->bit_depth, .order_hint = priv->order_hint, }; + memcpy(priv->ref[i].loop_filter_ref_deltas, current->loop_filter_ref_deltas, + sizeof(current->loop_filter_ref_deltas)); + memcpy(priv->ref[i].loop_filter_mode_deltas, current->loop_filter_mode_deltas, + sizeof(current->loop_filter_mode_deltas)); } }
Partially implements of setup_past_independence() and load_previous(). These ensures they are always set, even if the values were not coded in the input bitstream and will not be coded in the output bitstream. Signed-off-by: James Almer <jamrial@gmail.com> --- libavcodec/cbs_av1.h | 3 +++ libavcodec/cbs_av1_syntax_template.c | 40 +++++++++++++++++++++++++--- 2 files changed, 39 insertions(+), 4 deletions(-)