Remove some side-effects from av1_pack_bitstream
Various actions take place in av1_pack_bitstream which are not actually
part of the process of writing the bitstream. In this patch I remove
some of these with the goal that in future av1_pack_bitstream will only
produce a bitstream and not mutate state in any other way.
* There is no need to reset cm->cdef_info here if CDEF is disabled
* Replace unnecessary modification of cm->tx_mode with assertion
* Make cm->frame_refs_short_signaling a local variable
* Remove cm->is_reference_frame flag (it is never read)
* Remove unnecessary write to allow_high_precision_mv
* Remove unnecessary write to cm->refresh_frame_context
* Remove invalid_delta_frame_id_minus_1 flag
* Replace the error flag with an immediate aom_internal_error at the
point where the error is detected.
* Remove CurrentFrame.intra_only
* In the encoder it was always 0.
* In the decoder it was set to CurrentFrame.frame_type == INTRA_ONLY_FRAME
* In each case replace reads of the variable with its assigned value.
* Remove RefCntBuffer.intra_only as it was always equal to
(buf.frame_type == KEY_FRAME || buf.frame_type == INTRA_ONLY_FRAME)
This forms part of wider restructuring and refactoring in order to
achieve a clean API separation at the entry to the low-level encoder.
BUG=aomedia:2244
Change-Id: Ifcfdf6ec089a0f211facfeeab5a4ac6e4796989e
diff --git a/av1/av1_cx_iface.c b/av1/av1_cx_iface.c
index ab5f26a..6c8e7c8 100644
--- a/av1/av1_cx_iface.c
+++ b/av1/av1_cx_iface.c
@@ -1355,12 +1355,6 @@
-1 != av1_get_compressed_data(cpi, &lib_flags, &frame_size, cx_data,
&dst_time_stamp, &dst_end_time_stamp,
!img, timebase)) {
- if (cpi->common.seq_params.frame_id_numbers_present_flag) {
- if (cpi->common.invalid_delta_frame_id_minus_1) {
- aom_internal_error(&cpi->common.error, AOM_CODEC_ERROR,
- "Invalid delta_frame_id_minus_1");
- }
- }
cpi->seq_params_locked = 1;
if (frame_size) {
if (ctx->pending_cx_data == 0) ctx->pending_cx_data = cx_data;
diff --git a/av1/av1_dx_iface.c b/av1/av1_dx_iface.c
index 44738b1..60a8a5c 100644
--- a/av1/av1_dx_iface.c
+++ b/av1/av1_dx_iface.c
@@ -449,8 +449,7 @@
const AV1Decoder *const pbi) {
// Clear resync flag if worker got a key frame or intra only frame.
if (ctx->need_resync == 1 && pbi->need_resync == 0 &&
- (pbi->common.current_frame.intra_only ||
- pbi->common.current_frame.frame_type == KEY_FRAME))
+ frame_is_intra_only(&pbi->common))
ctx->need_resync = 0;
}
diff --git a/av1/common/mvref_common.c b/av1/common/mvref_common.c
index b3d9c2f..baabbcc 100644
--- a/av1/common/mvref_common.c
+++ b/av1/common/mvref_common.c
@@ -948,7 +948,9 @@
cm->current_frame.frame_refs[FWD_RF_OFFSET(start_frame)].buf;
if (start_frame_buf == NULL) return 0;
- if (start_frame_buf->intra_only) return 0;
+ if (start_frame_buf->frame_type == KEY_FRAME ||
+ start_frame_buf->frame_type == INTRA_ONLY_FRAME)
+ return 0;
if (start_frame_buf->mi_rows != cm->mi_rows ||
start_frame_buf->mi_cols != cm->mi_cols)
diff --git a/av1/common/onyxc_int.h b/av1/common/onyxc_int.h
index d6f7e66..7f3a2b6 100644
--- a/av1/common/onyxc_int.h
+++ b/av1/common/onyxc_int.h
@@ -159,7 +159,6 @@
aom_codec_frame_buffer_t raw_frame_buffer;
YV12_BUFFER_CONFIG buf;
hash_table hash_table;
- uint8_t intra_only;
FRAME_TYPE frame_type;
// Inter frame reference frame delta for loop filter
@@ -311,8 +310,6 @@
typedef struct {
FRAME_TYPE frame_type;
- // Flag signaling that the frame is encoded using only INTRA modes.
- uint8_t intra_only;
REFERENCE_MODE reference_mode;
unsigned int order_hint;
@@ -368,8 +365,6 @@
int show_frame;
int showable_frame; // frame can be used as show existing frame in future
int show_existing_frame;
- // Flag for a frame used as a reference - not written to the bitstream
- int is_reference_frame;
int reset_decoder_state;
uint8_t disable_cdf_update;
@@ -537,7 +532,6 @@
int current_frame_id;
int ref_frame_id[REF_FRAMES];
int valid_for_referencing[REF_FRAMES];
- int invalid_delta_frame_id_minus_1;
TPL_MV_REF *tpl_mvs;
int tpl_mvs_mem_size;
// TODO(jingning): This can be combined with sign_bias later.
@@ -545,7 +539,6 @@
int is_annexb;
- int frame_refs_short_signaling;
int temporal_layer_id;
int spatial_layer_id;
unsigned int number_temporal_layers;
@@ -645,7 +638,7 @@
static INLINE int frame_is_intra_only(const AV1_COMMON *const cm) {
return cm->current_frame.frame_type == KEY_FRAME ||
- cm->current_frame.intra_only;
+ cm->current_frame.frame_type == INTRA_ONLY_FRAME;
}
static INLINE int frame_is_sframe(const AV1_COMMON *cm) {
diff --git a/av1/decoder/decodeframe.c b/av1/decoder/decodeframe.c
index 9cf967d..7f32200 100644
--- a/av1/decoder/decodeframe.c
+++ b/av1/decoder/decodeframe.c
@@ -4726,8 +4726,6 @@
pbi->need_resync = 0;
}
- cm->cur_frame->intra_only = 1;
-
if (cm->seq_params.frame_id_numbers_present_flag) {
/* If bitmask is set, update reference frame id values and
mark frames as valid for reference.
@@ -4793,9 +4791,6 @@
"No sequence header");
}
- // NOTE: By default all coded frames to be used as a reference
- cm->is_reference_frame = 1;
-
if (seq_params->reduced_still_picture_hdr) {
cm->show_existing_frame = 0;
cm->show_frame = 1;
@@ -4905,7 +4900,6 @@
cm->showable_frame = aom_rb_read_bit(rb);
}
cm->cur_frame->showable_frame = cm->showable_frame;
- current_frame->intra_only = current_frame->frame_type == INTRA_ONLY_FRAME;
cm->error_resilient_mode =
frame_is_sframe(cm) ||
(current_frame->frame_type == KEY_FRAME && cm->show_frame)
@@ -4930,7 +4924,6 @@
cm->cur_frame_force_integer_mv = 0;
}
- cm->frame_refs_short_signaling = 0;
int frame_size_override_flag = 0;
cm->allow_intrabc = 0;
cm->primary_ref_frame = PRIMARY_REF_NONE;
@@ -5030,7 +5023,7 @@
pbi->need_resync = 0;
}
} else {
- if (current_frame->intra_only) {
+ if (current_frame->frame_type == INTRA_ONLY_FRAME) {
pbi->refresh_frame_flags = aom_rb_read_literal(rb, REF_FRAMES);
if (pbi->refresh_frame_flags == 0xFF) {
aom_internal_error(&cm->error, AOM_CODEC_UNSUP_BITSTREAM,
@@ -5043,11 +5036,6 @@
} else if (pbi->need_resync != 1) { /* Skip if need resync */
pbi->refresh_frame_flags =
frame_is_sframe(cm) ? 0xFF : aom_rb_read_literal(rb, REF_FRAMES);
- if (!pbi->refresh_frame_flags) {
- // NOTE: "pbi->refresh_frame_flags == 0" indicates that the coded frame
- // will not be used as a reference
- cm->is_reference_frame = 0;
- }
}
}
@@ -5108,7 +5096,7 @@
} else {
cm->allow_ref_frame_mvs = 0;
- if (current_frame->intra_only) {
+ if (current_frame->frame_type == INTRA_ONLY_FRAME) {
cm->cur_frame->film_grain_params_present =
seq_params->film_grain_params_present;
setup_frame_size(cm, frame_size_override_flag, rb);
@@ -5116,12 +5104,12 @@
cm->allow_intrabc = aom_rb_read_bit(rb);
} else if (pbi->need_resync != 1) { /* Skip if need resync */
-
+ int frame_refs_short_signaling = 0;
// Frame refs short signaling is off when error resilient mode is on.
if (seq_params->order_hint_info.enable_order_hint)
- cm->frame_refs_short_signaling = aom_rb_read_bit(rb);
+ frame_refs_short_signaling = aom_rb_read_bit(rb);
- if (cm->frame_refs_short_signaling) {
+ if (frame_refs_short_signaling) {
// == LAST_FRAME ==
const int lst_ref = aom_rb_read_literal(rb, REF_FRAMES_LOG2);
const int lst_idx = cm->ref_frame_map[lst_ref];
@@ -5148,7 +5136,7 @@
for (int i = 0; i < INTER_REFS_PER_FRAME; ++i) {
int ref = 0;
- if (!cm->frame_refs_short_signaling) {
+ if (!frame_refs_short_signaling) {
ref = aom_rb_read_literal(rb, REF_FRAMES_LOG2);
const int idx = cm->ref_frame_map[ref];
@@ -5211,7 +5199,8 @@
"frame context is unavailable.");
}
- if (!current_frame->intra_only && pbi->need_resync != 1) {
+ if (!(current_frame->frame_type == INTRA_ONLY_FRAME) &&
+ pbi->need_resync != 1) {
if (frame_might_allow_ref_frame_mvs(cm))
cm->allow_ref_frame_mvs = aom_rb_read_bit(rb);
else
@@ -5233,8 +5222,6 @@
av1_setup_frame_sign_bias(cm);
- cm->cur_frame->intra_only =
- current_frame->frame_type == KEY_FRAME || current_frame->intra_only;
cm->cur_frame->frame_type = current_frame->frame_type;
if (seq_params->frame_id_numbers_present_flag) {
diff --git a/av1/encoder/bitstream.c b/av1/encoder/bitstream.c
index cf5bba7..8e0cdfc 100644
--- a/av1/encoder/bitstream.c
+++ b/av1/encoder/bitstream.c
@@ -867,14 +867,7 @@
static void write_cdef(AV1_COMMON *cm, MACROBLOCKD *const xd, aom_writer *w,
int skip, int mi_col, int mi_row) {
- if (cm->coded_lossless || cm->allow_intrabc) {
- // Initialize to indicate no CDEF for safety.
- cm->cdef_info.cdef_bits = 0;
- cm->cdef_info.cdef_strengths[0] = 0;
- cm->cdef_info.nb_cdef_strengths = 1;
- cm->cdef_info.cdef_uv_strengths[0] = 0;
- return;
- }
+ if (cm->coded_lossless || cm->allow_intrabc) return;
const int m = ~((1 << (6 - MI_SIZE_LOG2)) - 1);
const MB_MODE_INFO *mbmi =
@@ -2076,15 +2069,6 @@
}
}
-static void write_tx_mode(AV1_COMMON *cm, TX_MODE *mode,
- struct aom_write_bit_buffer *wb) {
- if (cm->coded_lossless) {
- *mode = ONLY_4X4;
- return;
- }
- aom_wb_write_bit(wb, *mode == TX_MODE_SELECT);
-}
-
static void write_frame_interp_filter(InterpFilter filter,
struct aom_write_bit_buffer *wb) {
aom_wb_write_bit(wb, filter == SWITCHABLE);
@@ -2842,9 +2826,8 @@
}
}
-static void check_frame_refs_short_signaling(AV1_COMP *const cpi) {
+static int check_frame_refs_short_signaling(AV1_COMP *const cpi) {
AV1_COMMON *const cm = &cpi->common;
- if (!cm->frame_refs_short_signaling) return;
// Check whether all references are distinct frames.
int buf_markers[FRAME_BUFFERS] = { 0 };
@@ -2866,8 +2849,7 @@
if (num_refs < INTER_REFS_PER_FRAME) {
// It indicates that there exist more than one reference frame pointing to
// the same reference buffer, i.e. two or more references are duplicate.
- cm->frame_refs_short_signaling = 0;
- return;
+ return 0;
}
// Check whether the encoder side ref frame choices are aligned with that to
@@ -2887,19 +2869,20 @@
// We only turn on frame_refs_short_signaling when the encoder side decision
// on ref frames is identical to that at the decoder side.
+ int frame_refs_short_signaling = 1;
for (int ref_idx = 0; ref_idx < INTER_REFS_PER_FRAME; ++ref_idx) {
// Compare the buffer index between two reference frames indexed
// respectively by the encoder and the decoder side decisions.
if (cm->current_frame.frame_refs[ref_idx].buf !=
frame_refs_copy[ref_idx].buf) {
- cm->frame_refs_short_signaling = 0;
+ frame_refs_short_signaling = 0;
break;
}
}
#if 0 // For debug
printf("\nFrame=%d: \n", cm->current_frame.frame_number);
- printf("***frame_refs_short_signaling=%d\n", cm->frame_refs_short_signaling);
+ printf("***frame_refs_short_signaling=%d\n", frame_refs_short_signaling);
for (int ref_frame = LAST_FRAME; ref_frame <= ALTREF_FRAME; ++ref_frame) {
printf("enc_ref(map_idx=%d, buf_idx=%d)=%d, vs. "
"dec_ref(map_idx=%d)=%d\n",
@@ -2911,9 +2894,11 @@
#endif // 0
// Restore the frame refs info if frame_refs_short_signaling is off.
- if (!cm->frame_refs_short_signaling)
+ if (!frame_refs_short_signaling) {
memcpy(cm->current_frame.frame_refs, frame_refs_copy,
INTER_REFS_PER_FRAME * sizeof(RefBuffer));
+ }
+ return frame_refs_short_signaling;
}
// New function based on HLS R18
@@ -2925,11 +2910,6 @@
MACROBLOCKD *const xd = &cpi->td.mb.e_mbd;
CurrentFrame *const current_frame = &cm->current_frame;
- // NOTE: By default all coded frames to be used as a reference
- cm->is_reference_frame = 1;
- current_frame->frame_type =
- current_frame->intra_only ? INTRA_ONLY_FRAME : current_frame->frame_type;
-
if (seq_params->still_picture) {
assert(cm->show_existing_frame == 0);
assert(cm->show_frame == 1);
@@ -3008,9 +2988,7 @@
assert(cm->cur_frame_force_integer_mv == 0);
}
- cm->invalid_delta_frame_id_minus_1 = 0;
int frame_size_override_flag = 0;
- cm->frame_refs_short_signaling = 0;
if (seq_params->reduced_still_picture_hdr) {
assert(cm->width == seq_params->max_frame_width &&
@@ -3111,12 +3089,6 @@
if (updated_fb >= 0) {
cm->fb_of_context_type[cm->frame_context_idx] = updated_fb;
}
-
- if (!cpi->refresh_frame_mask) {
- // NOTE: "cpi->refresh_frame_mask == 0" indicates that the coded frame
- // will not be used as a reference
- cm->is_reference_frame = 0;
- }
}
}
@@ -3157,24 +3129,25 @@
// NOTE: Error resilient mode turns off frame_refs_short_signaling
// automatically.
+ int frame_refs_short_signaling = 0;
#define FRAME_REFS_SHORT_SIGNALING 0
#if FRAME_REFS_SHORT_SIGNALING
- cm->frame_refs_short_signaling =
+ frame_refs_short_signaling =
seq_params->order_hint_info.enable_order_hint;
#endif // FRAME_REFS_SHORT_SIGNALING
- if (cm->frame_refs_short_signaling) {
+ if (frame_refs_short_signaling) {
// NOTE(zoeliu@google.com):
// An example solution for encoder-side implementation on frame refs
// short signaling, which is only turned on when the encoder side
// decision on ref frames is identical to that at the decoder side.
- check_frame_refs_short_signaling(cpi);
+ frame_refs_short_signaling = check_frame_refs_short_signaling(cpi);
}
if (seq_params->order_hint_info.enable_order_hint)
- aom_wb_write_bit(wb, cm->frame_refs_short_signaling);
+ aom_wb_write_bit(wb, frame_refs_short_signaling);
- if (cm->frame_refs_short_signaling) {
+ if (frame_refs_short_signaling) {
const int lst_ref = get_ref_frame_map_idx(cpi, LAST_FRAME);
aom_wb_write_literal(wb, lst_ref, REF_FRAMES_LOG2);
@@ -3184,7 +3157,7 @@
for (ref_frame = LAST_FRAME; ref_frame <= ALTREF_FRAME; ++ref_frame) {
assert(get_ref_frame_map_idx(cpi, ref_frame) != INVALID_IDX);
- if (!cm->frame_refs_short_signaling)
+ if (!frame_refs_short_signaling)
aom_wb_write_literal(wb, get_ref_frame_map_idx(cpi, ref_frame),
REF_FRAMES_LOG2);
if (seq_params->frame_id_numbers_present_flag) {
@@ -3197,8 +3170,10 @@
(1 << frame_id_len)) -
1;
if (delta_frame_id_minus_1 < 0 ||
- delta_frame_id_minus_1 >= (1 << diff_len))
- cm->invalid_delta_frame_id_minus_1 = 1;
+ delta_frame_id_minus_1 >= (1 << diff_len)) {
+ aom_internal_error(&cpi->common.error, AOM_CODEC_ERROR,
+ "Invalid delta_frame_id_minus_1");
+ }
aom_wb_write_literal(wb, delta_frame_id_minus_1, diff_len);
}
}
@@ -3209,11 +3184,8 @@
write_frame_size(cm, frame_size_override_flag, wb);
}
- if (cm->cur_frame_force_integer_mv) {
- cm->allow_high_precision_mv = 0;
- } else {
+ if (!cm->cur_frame_force_integer_mv)
aom_wb_write_bit(wb, cm->allow_high_precision_mv);
- }
fix_interp_filter(cm, cpi->td.counts);
write_frame_interp_filter(cm->interp_filter, wb);
aom_wb_write_bit(wb, cm->switchable_motion_mode);
@@ -3228,7 +3200,7 @@
const int might_bwd_adapt =
!(seq_params->reduced_still_picture_hdr) && !(cm->disable_cdf_update);
if (cm->large_scale_tile)
- cm->refresh_frame_context = REFRESH_FRAME_CONTEXT_DISABLED;
+ assert(cm->refresh_frame_context == REFRESH_FRAME_CONTEXT_DISABLED);
if (might_bwd_adapt) {
aom_wb_write_bit(
@@ -3268,7 +3240,11 @@
encode_restoration_mode(cm, wb);
}
- write_tx_mode(cm, &cm->tx_mode, wb);
+ // Write TX mode
+ if (cm->coded_lossless)
+ assert(cm->tx_mode == ONLY_4X4);
+ else
+ aom_wb_write_bit(wb, cm->tx_mode == TX_MODE_SELECT);
if (!frame_is_intra_only(cm)) {
const int use_hybrid_pred =
diff --git a/av1/encoder/encoder.c b/av1/encoder/encoder.c
index 312d836..f03de63 100644
--- a/av1/encoder/encoder.c
+++ b/av1/encoder/encoder.c
@@ -5038,7 +5038,6 @@
aom_clear_system_state();
// frame type has been decided outside of this function call
- cm->cur_frame->intra_only = frame_is_intra_only(cm);
cm->cur_frame->frame_type = current_frame->frame_type;
// S_FRAMEs are always error resilient
@@ -6671,7 +6670,6 @@
}
}
cm->show_frame = 0;
- current_frame->intra_only = 0;
if (oxcf->pass < 2) {
// In second pass, the buffer updates configure will be set
@@ -6713,7 +6711,6 @@
}
cm->show_frame = 0;
- current_frame->intra_only = 0;
if (oxcf->pass < 2) {
// In second pass, the buffer updates configure will be set
@@ -6731,7 +6728,6 @@
if ((source = av1_lookahead_peek(cpi->lookahead, brf_src_index)) != NULL) {
cm->showable_frame = 1;
cm->show_frame = 0;
- current_frame->intra_only = 0;
if (oxcf->pass < 2) {
// In second pass, the buffer updates configure will be set
@@ -6753,7 +6749,6 @@
if (source != NULL) {
cm->show_frame = 1;
- current_frame->intra_only = 0;
// Check to see if the frame should be encoded as an arf overlay.
check_src_altref(cpi, source);
