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 <jamr...@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
_______________________________________________
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".
