Move interp_filter_selected to RefCntBuffer cpi->interp_filter_selected is currently indexed by reference, so a swathe of memcpys are required to keep it in sync with reference frame refreshing and reference reassignment. In this commit I move it to RefCntBuffer so it is automatically refreshed and and assigned. Because the old memcpys did not exactly mirror reference buffer refreshing and re-assignment, behaviour is changed slightly by this patch when the interp filter mask speed-feature is enabled (speed >= 2), however rdcost change seems to be negligible. STATS_CHANGED AWCY for speed=2: https://arewecompressedyet.com/?job=base%402019-02-15T16%3A59%3A48.521Z&job=test%402019-02-15T17%3A01%3A42.026Z AWCY for speed=5: https://arewecompressedyet.com/?job=base%402019-02-16T21%3A13%3A27.877Z&job=test%402019-02-16T21%3A13%3A36.627Z Change-Id: Iad492bc4b76ded5b57576a19b20c348ab5906415
diff --git a/av1/common/onyxc_int.h b/av1/common/onyxc_int.h index d5668a7..4b9529d 100644 --- a/av1/common/onyxc_int.h +++ b/av1/common/onyxc_int.h
@@ -136,8 +136,6 @@ // - Total 'n' of the variables / array elements above have value 'k' (that // is, they are pointing to buffer at index 'k'). // Then, pool->frame_bufs[k].ref_count = n. - // TODO(david.turner@argondesign.com) Check whether this helpful comment is - // still correct after we finish restructuring int ref_count; unsigned int order_hint; @@ -161,6 +159,10 @@ hash_table hash_table; FRAME_TYPE frame_type; + // This is only used in the encoder but needs to be indexed per ref frame + // so it's extremely convenient to keep it here. + int interp_filter_selected[SWITCHABLE]; + // Inter frame reference frame delta for loop filter int8_t ref_deltas[REF_FRAMES]; @@ -643,6 +645,7 @@ cm->cur_frame = &cm->buffer_pool->frame_bufs[new_fb_idx]; cm->cur_frame->buf.buf_8bit_valid = 0; + av1_zero(cm->cur_frame->interp_filter_selected); return cm->cur_frame; }
diff --git a/av1/encoder/bitstream.c b/av1/encoder/bitstream.c index 8e4daf7..766c9e0 100644 --- a/av1/encoder/bitstream.c +++ b/av1/encoder/bitstream.c
@@ -627,7 +627,7 @@ av1_extract_interp_filter(mbmi->interp_filters, dir); aom_write_symbol(w, filter, ec_ctx->switchable_interp_cdf[ctx], SWITCHABLE_FILTERS); - ++cpi->interp_filter_selected[0][filter]; + ++cm->cur_frame->interp_filter_selected[filter]; if (cm->seq_params.enable_dual_filter == 0) return; } }
diff --git a/av1/encoder/encode_strategy.c b/av1/encoder/encode_strategy.c index f47eef2..db38581 100644 --- a/av1/encoder/encode_strategy.c +++ b/av1/encoder/encode_strategy.c
@@ -814,12 +814,6 @@ const int ref_idx = ref_frame - LAST_FRAME; cpi->common.remapped_ref_idx[ref_idx] = cpi->common.remapped_ref_idx[ref_idx - 1]; - - if (!cpi->rc.is_src_frame_alt_ref) { - memcpy(cpi->interp_filter_selected[ref_frame], - cpi->interp_filter_selected[ref_frame - 1], - sizeof(cpi->interp_filter_selected[ref_frame - 1])); - } } } @@ -833,11 +827,6 @@ EXTREF_FRAME }; for (int i = 2; i > 0; --i) { - // [0] is allocated to the current coded frame, i.e. bwdref - memcpy(cpi->interp_filter_selected[ordered_bwd[i]], - cpi->interp_filter_selected[ordered_bwd[i - 1]], - sizeof(cpi->interp_filter_selected[ordered_bwd[i - 1]])); - cpi->common.remapped_ref_idx[ordered_bwd[i] - LAST_FRAME] = cpi->common.remapped_ref_idx[ordered_bwd[i - 1] - LAST_FRAME]; } @@ -853,11 +842,6 @@ EXTREF_FRAME }; for (int i = 0; i < 2; ++i) { - // [0] is allocated to the current coded frame, i.e. bwdref - memcpy(cpi->interp_filter_selected[ordered_bwd[i]], - cpi->interp_filter_selected[ordered_bwd[i + 1]], - sizeof(cpi->interp_filter_selected[ordered_bwd[i + 1]])); - cpi->common.remapped_ref_idx[ordered_bwd[i] - LAST_FRAME] = cpi->common.remapped_ref_idx[ordered_bwd[i + 1] - LAST_FRAME]; } @@ -901,9 +885,6 @@ cm->remapped_ref_idx[ALTREF_FRAME - LAST_FRAME] = get_ref_frame_map_idx(cm, GOLDEN_FRAME); cm->remapped_ref_idx[GOLDEN_FRAME - LAST_FRAME] = tmp; - - // TODO(zoeliu): Do we need to copy cpi->interp_filter_selected[0] over to - // cpi->interp_filter_selected[GOLDEN_FRAME]? } else if (cpi->rc.is_src_frame_ext_arf && encode_show_existing_frame(cm)) { const int bwdref_to_show = (cpi->new_bwdref_update_rule == 1) ? BWDREF_FRAME : ALTREF2_FRAME; @@ -916,9 +897,6 @@ cm->remapped_ref_idx[LAST_FRAME - LAST_FRAME] = get_ref_frame_map_idx(cm, bwdref_to_show); - memcpy(cpi->interp_filter_selected[LAST_FRAME], - cpi->interp_filter_selected[bwdref_to_show], - sizeof(cpi->interp_filter_selected[bwdref_to_show])); if (cpi->new_bwdref_update_rule == 1) { lshift_bwd_ref_frames(cpi); // pass outdated forward reference frame (previous LAST3) to the @@ -928,39 +906,19 @@ cm->remapped_ref_idx[bwdref_to_show - LAST_FRAME] = last3_remapped_idx; } } else { /* For non key/golden frames */ - if (cpi->refresh_alt_ref_frame && !cm->show_existing_frame) { - // === ALTREF_FRAME === - memcpy(cpi->interp_filter_selected[ALTREF_FRAME], - cpi->interp_filter_selected[0], - sizeof(cpi->interp_filter_selected[0])); - } else if (cpi->refresh_golden_frame && !cm->show_existing_frame) { - // === GOLDEN_FRAME === - memcpy(cpi->interp_filter_selected[GOLDEN_FRAME], - cpi->interp_filter_selected[0], - sizeof(cpi->interp_filter_selected[0])); - - } else if (cpi->refresh_bwd_ref_frame && !cm->show_existing_frame) { - if (cpi->new_bwdref_update_rule) { - // === BWDREF_FRAME === - // (Nothing to do for BWDREF_FRAME show_existing_frame because the - // reference frame update has been done previously when handling the - // LAST_BIPRED_FRAME right before BWDREF_FRAME (in the display order)) - // We shift the backward reference frame as follows: - // BWDREF -> ALTREF2 -> EXTREF - // and assign the newly coded frame to BWDREF so that it always - // keeps the nearest future frame - const int tmp = get_ref_frame_map_idx(cm, EXTREF_FRAME); - rshift_bwd_ref_frames(cpi); - cm->remapped_ref_idx[BWDREF_FRAME - LAST_FRAME] = tmp; - } - memcpy(cpi->interp_filter_selected[BWDREF_FRAME], - cpi->interp_filter_selected[0], - sizeof(cpi->interp_filter_selected[0])); - } else if (cpi->refresh_alt2_ref_frame && !cm->show_existing_frame) { - // === ALTREF2_FRAME === - memcpy(cpi->interp_filter_selected[ALTREF2_FRAME], - cpi->interp_filter_selected[0], - sizeof(cpi->interp_filter_selected[0])); + if (cpi->refresh_bwd_ref_frame && !cm->show_existing_frame && + cpi->new_bwdref_update_rule) { + // === BWDREF_FRAME === + // (Nothing to do for BWDREF_FRAME show_existing_frame because the + // reference frame update has been done previously when handling the + // LAST_BIPRED_FRAME right before BWDREF_FRAME (in the display order)) + // We shift the backward reference frame as follows: + // BWDREF -> ALTREF2 -> EXTREF + // and assign the newly coded frame to BWDREF so that it always + // keeps the nearest future frame + const int tmp = get_ref_frame_map_idx(cm, EXTREF_FRAME); + rshift_bwd_ref_frames(cpi); + cm->remapped_ref_idx[BWDREF_FRAME - LAST_FRAME] = tmp; } } @@ -1002,11 +960,6 @@ shift_last_ref_frames(cpi); cm->remapped_ref_idx[LAST_FRAME - LAST_FRAME] = last3_remapped_idx; - // TODO(dwt): This copy should actually follow the assign_frame_buffer. - memcpy(cpi->interp_filter_selected[LAST_FRAME], - cpi->interp_filter_selected[0], - sizeof(cpi->interp_filter_selected[0])); - // If the new structure is used, we will always have overlay frames coupled // with bwdref frames. Therefore, we won't have to perform this update // in advance (we do this update when the overlay frame shows up). @@ -1026,10 +979,6 @@ cm->remapped_ref_idx[LAST_FRAME - LAST_FRAME] = get_ref_frame_map_idx(cm, BWDREF_FRAME); cm->remapped_ref_idx[BWDREF_FRAME - LAST_FRAME] = last3_remapped_idx; - - memcpy(cpi->interp_filter_selected[LAST_FRAME], - cpi->interp_filter_selected[BWDREF_FRAME], - sizeof(cpi->interp_filter_selected[BWDREF_FRAME])); } }
diff --git a/av1/encoder/encoder.c b/av1/encoder/encoder.c index be5471d..af09dad 100644 --- a/av1/encoder/encoder.c +++ b/av1/encoder/encoder.c
@@ -358,10 +358,8 @@ } if (cm->current_frame.frame_type == KEY_FRAME && cm->show_frame) { - av1_zero(cpi->interp_filter_selected); set_sb_size(&cm->seq_params, select_sb_size(cpi)); } else if (frame_is_sframe(cm)) { - av1_zero(cpi->interp_filter_selected); set_sb_size(&cm->seq_params, select_sb_size(cpi)); } else { const RefCntBuffer *const primary_ref_buf = get_primary_ref_frame_buf(cm); @@ -372,9 +370,9 @@ } else { *cm->fc = primary_ref_buf->frame_context; } - av1_zero(cpi->interp_filter_selected[0]); } + av1_zero(cm->cur_frame->interp_filter_selected); cm->prev_frame = get_primary_ref_frame_buf(cm); cpi->vaq_refresh = 0; } @@ -4621,32 +4619,45 @@ } #endif // DUMP_RECON_FRAMES -static int setup_interp_filter_search_mask(AV1_COMP *cpi) { - InterpFilters ifilter; - int ref_total[REF_FRAMES] = { 0 }; - MV_REFERENCE_FRAME ref; - int mask = 0; - int arf_idx = ALTREF_FRAME; - if (cpi->common.last_frame_type == KEY_FRAME || cpi->refresh_alt_ref_frame) - return mask; - for (ref = LAST_FRAME; ref <= ALTREF_FRAME; ++ref) - for (ifilter = EIGHTTAP_REGULAR; ifilter <= MULTITAP_SHARP; ++ifilter) - ref_total[ref] += cpi->interp_filter_selected[ref][ifilter]; +static int get_interp_filter_selected(const AV1_COMMON *const cm, + MV_REFERENCE_FRAME ref, + InterpFilters ifilter) { + const RefCntBuffer *const buf = get_ref_frame_buf(cm, ref); + if (buf == NULL) return 0; + return buf->interp_filter_selected[ifilter]; +} - for (ifilter = EIGHTTAP_REGULAR; ifilter <= MULTITAP_SHARP; ++ifilter) { - if ((ref_total[LAST_FRAME] && - cpi->interp_filter_selected[LAST_FRAME][ifilter] * 30 <= - ref_total[LAST_FRAME]) && - (((cpi->interp_filter_selected[LAST2_FRAME][ifilter] * 20) + - (cpi->interp_filter_selected[LAST3_FRAME][ifilter] * 20) + - (cpi->interp_filter_selected[GOLDEN_FRAME][ifilter] * 20) + - (cpi->interp_filter_selected[BWDREF_FRAME][ifilter] * 10) + - (cpi->interp_filter_selected[ALTREF2_FRAME][ifilter] * 10) + - (cpi->interp_filter_selected[arf_idx][ifilter] * 10)) < - (ref_total[LAST2_FRAME] + ref_total[LAST3_FRAME] + - ref_total[GOLDEN_FRAME] + ref_total[BWDREF_FRAME] + - ref_total[ALTREF2_FRAME] + ref_total[ALTREF_FRAME]))) - mask |= 1 << ifilter; +static int setup_interp_filter_search_mask(AV1_COMP *cpi) { + const AV1_COMMON *const cm = &cpi->common; + int ref_total[REF_FRAMES] = { 0 }; + + if (cpi->common.last_frame_type == KEY_FRAME || cpi->refresh_alt_ref_frame) + return 0; + + for (MV_REFERENCE_FRAME ref = LAST_FRAME; ref <= ALTREF_FRAME; ++ref) { + for (InterpFilters ifilter = EIGHTTAP_REGULAR; ifilter <= MULTITAP_SHARP; + ++ifilter) { + ref_total[ref] += get_interp_filter_selected(cm, ref, ifilter); + } + } + int ref_total_total = (ref_total[LAST2_FRAME] + ref_total[LAST3_FRAME] + + ref_total[GOLDEN_FRAME] + ref_total[BWDREF_FRAME] + + ref_total[ALTREF2_FRAME] + ref_total[ALTREF_FRAME]); + + int mask = 0; + for (InterpFilters ifilter = EIGHTTAP_REGULAR; ifilter <= MULTITAP_SHARP; + ++ifilter) { + int last_score = get_interp_filter_selected(cm, LAST_FRAME, ifilter) * 30; + if (ref_total[LAST_FRAME] && last_score <= ref_total[LAST_FRAME]) { + int filter_score = + get_interp_filter_selected(cm, LAST2_FRAME, ifilter) * 20 + + get_interp_filter_selected(cm, LAST3_FRAME, ifilter) * 20 + + get_interp_filter_selected(cm, GOLDEN_FRAME, ifilter) * 20 + + get_interp_filter_selected(cm, BWDREF_FRAME, ifilter) * 10 + + get_interp_filter_selected(cm, ALTREF2_FRAME, ifilter) * 10 + + get_interp_filter_selected(cm, ALTREF_FRAME, ifilter) * 10; + if (filter_score < ref_total_total) mask |= 1 << ifilter; + } } return mask; }
diff --git a/av1/encoder/encoder.h b/av1/encoder/encoder.h index 5042f4f..794b31f 100644 --- a/av1/encoder/encoder.h +++ b/av1/encoder/encoder.h
@@ -769,12 +769,6 @@ RATE_CONTROL rc; double framerate; - // Relevant for an inter frame. - // - Index '0' corresponds to the values for the currently coded frame. - // - Indices LAST_FRAME ... EXTREF_FRAMES are used to store values for all the - // possible inter reference frames. - int interp_filter_selected[REF_FRAMES + 1][SWITCHABLE]; - struct aom_codec_pkt_list *output_pkt_list; MBGRAPH_FRAME_STATS mbgraph_stats[MAX_LAG_BUFFERS];