Cleanup regarding buffer pool ref counts. Change mysterious function name 'ref_cnt_fb' to 'assign_frame_buffer'. Also, add comments for: - ref_count variable, including which variables it depends on - assign_frame_buffer() function Finally, remove unused function: uref_cnt_fb(). BUG=aomedia:2230 Change-Id: I7a086c906c2499c4876d5a7126e225d13aae187b
diff --git a/av1/common/onyxc_int.h b/av1/common/onyxc_int.h index 551e971..8af1626 100644 --- a/av1/common/onyxc_int.h +++ b/av1/common/onyxc_int.h
@@ -110,6 +110,19 @@ } MV_REF; typedef struct { + // For a RefCntBuffer, the following are reference-holding variables: + // - cm->ref_frame_map[] + // - cm->new_fb_idx + // - cm->scaled_ref_idx[] (encoder only) + // - cm->next_ref_frame_map[] (decoder only) + // - pbi->output_frame_index[] (decoder only) + // With that definition, 'ref_count' is the number of reference-holding + // variables that are currently referencing this buffer. + // For example: + // - suppose this buffer is at index 'k' in the buffer pool, and + // - 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. int ref_count; unsigned int cur_frame_offset; @@ -602,17 +615,20 @@ return i; } -static INLINE void ref_cnt_fb(RefCntBuffer *bufs, int *idx, int new_idx) { - const int ref_index = *idx; - - if (ref_index >= 0) { - assert(bufs[ref_index].ref_count > 0); - bufs[ref_index].ref_count--; +// Modify 'idx_ptr' to reference the buffer at 'new_idx', and update the ref +// counts accordingly. +static INLINE void assign_frame_buffer(RefCntBuffer *bufs, int *idx_ptr, + int new_idx) { + const int old_idx = *idx_ptr; + if (old_idx >= 0) { + assert(bufs[old_idx].ref_count > 0); + // One less reference to the buffer at 'old_idx', so decrease ref count. + --bufs[old_idx].ref_count; } - *idx = new_idx; - - bufs[new_idx].ref_count++; + *idx_ptr = new_idx; + // One more reference to the buffer at 'new_idx', so increase ref count. + ++bufs[new_idx].ref_count; } static INLINE int frame_is_intra_only(const AV1_COMMON *const cm) {
diff --git a/av1/decoder/decodeframe.c b/av1/decoder/decodeframe.c index b62c349..e36c9fe 100644 --- a/av1/decoder/decodeframe.c +++ b/av1/decoder/decodeframe.c
@@ -4845,7 +4845,7 @@ // decrease_ref_count(). If frame_bufs[cm->new_fb_idx].raw_frame_buffer // has already been allocated, it will not be released by ref_cnt_fb()! assert(!frame_bufs[cm->new_fb_idx].raw_frame_buffer.data); - ref_cnt_fb(frame_bufs, &cm->new_fb_idx, frame_to_show); + assign_frame_buffer(frame_bufs, &cm->new_fb_idx, frame_to_show); cm->reset_decoder_state = frame_bufs[frame_to_show].frame_type == KEY_FRAME; unlock_buffer_pool(pool);
diff --git a/av1/encoder/bitstream.c b/av1/encoder/bitstream.c index 2c779ed..626b5b7 100644 --- a/av1/encoder/bitstream.c +++ b/av1/encoder/bitstream.c
@@ -2936,7 +2936,7 @@ "Buffer %d does not contain a reconstructed frame", frame_to_show); } - ref_cnt_fb(frame_bufs, &cm->new_fb_idx, frame_to_show); + assign_frame_buffer(frame_bufs, &cm->new_fb_idx, frame_to_show); aom_wb_write_bit(wb, 1); // show_existing_frame aom_wb_write_literal(wb, cpi->existing_fb_idx_to_show, 3);
diff --git a/av1/encoder/encoder.c b/av1/encoder/encoder.c index b108111..7b91720 100644 --- a/av1/encoder/encoder.c +++ b/av1/encoder/encoder.c
@@ -3519,9 +3519,9 @@ // show_frame. This ensures a fwd keyframe does not update all of the buffers if ((cm->frame_type == KEY_FRAME && cm->show_frame) || frame_is_sframe(cm)) { for (int ref_frame = 0; ref_frame < REF_FRAMES; ++ref_frame) { - ref_cnt_fb(pool->frame_bufs, - &cm->ref_frame_map[cpi->remapped_ref_idx[ref_frame]], - cm->new_fb_idx); + assign_frame_buffer(pool->frame_bufs, + &cm->ref_frame_map[cpi->remapped_ref_idx[ref_frame]], + cm->new_fb_idx); } return; } @@ -3541,9 +3541,10 @@ // reference instead of replacing it with overlay. if (!cpi->preserve_arf_as_gld) { - ref_cnt_fb(pool->frame_bufs, - &cm->ref_frame_map[cpi->remapped_ref_idx[ALTREF_FRAME - 1]], - cm->new_fb_idx); + assign_frame_buffer( + pool->frame_bufs, + &cm->ref_frame_map[cpi->remapped_ref_idx[ALTREF_FRAME - 1]], + cm->new_fb_idx); } tmp = cpi->remapped_ref_idx[ALTREF_FRAME - 1]; @@ -3592,7 +3593,8 @@ // === ALTREF_FRAME === if (cpi->refresh_alt_ref_frame) { int arf_idx = cpi->remapped_ref_idx[ALTREF_FRAME - 1]; - ref_cnt_fb(pool->frame_bufs, &cm->ref_frame_map[arf_idx], cm->new_fb_idx); + assign_frame_buffer(pool->frame_bufs, &cm->ref_frame_map[arf_idx], + cm->new_fb_idx); memcpy(cpi->interp_filter_selected[ALTREF_FRAME], cpi->interp_filter_selected[0], @@ -3601,9 +3603,10 @@ // === GOLDEN_FRAME === if (cpi->refresh_golden_frame) { - ref_cnt_fb(pool->frame_bufs, - &cm->ref_frame_map[cpi->remapped_ref_idx[GOLDEN_FRAME - 1]], - cm->new_fb_idx); + assign_frame_buffer( + pool->frame_bufs, + &cm->ref_frame_map[cpi->remapped_ref_idx[GOLDEN_FRAME - 1]], + cm->new_fb_idx); memcpy(cpi->interp_filter_selected[GOLDEN_FRAME], cpi->interp_filter_selected[0], @@ -3619,15 +3622,17 @@ // and assign the newly coded frame to BWDREF so that it always // keeps the nearest future frame int tmp = cpi->remapped_ref_idx[EXTREF_FRAME - 1]; - ref_cnt_fb(pool->frame_bufs, &cm->ref_frame_map[tmp], cm->new_fb_idx); + assign_frame_buffer(pool->frame_bufs, &cm->ref_frame_map[tmp], + cm->new_fb_idx); rshift_bwd_ref_frames(cpi); cpi->remapped_ref_idx[BWDREF_FRAME - 1] = tmp; } else { #endif // USE_SYMM_MULTI_LAYER - ref_cnt_fb(pool->frame_bufs, - &cm->ref_frame_map[cpi->remapped_ref_idx[BWDREF_FRAME - 1]], - cm->new_fb_idx); + assign_frame_buffer( + pool->frame_bufs, + &cm->ref_frame_map[cpi->remapped_ref_idx[BWDREF_FRAME - 1]], + cm->new_fb_idx); #if USE_SYMM_MULTI_LAYER } #endif @@ -3638,9 +3643,10 @@ // === ALTREF2_FRAME === if (cpi->refresh_alt2_ref_frame) { - ref_cnt_fb(pool->frame_bufs, - &cm->ref_frame_map[cpi->remapped_ref_idx[ALTREF2_FRAME - 1]], - cm->new_fb_idx); + assign_frame_buffer( + pool->frame_bufs, + &cm->ref_frame_map[cpi->remapped_ref_idx[ALTREF2_FRAME - 1]], + cm->new_fb_idx); memcpy(cpi->interp_filter_selected[ALTREF2_FRAME], cpi->interp_filter_selected[0], @@ -3681,9 +3687,10 @@ // remapped_ref_idx[2], remapped_ref_idx[0], remapped_ref_idx[1] int tmp; - ref_cnt_fb(pool->frame_bufs, - &cm->ref_frame_map[cpi->remapped_ref_idx[LAST_REF_FRAMES - 1]], - cm->new_fb_idx); + assign_frame_buffer( + pool->frame_bufs, + &cm->ref_frame_map[cpi->remapped_ref_idx[LAST_REF_FRAMES - 1]], + cm->new_fb_idx); tmp = cpi->remapped_ref_idx[LAST_REF_FRAMES - 1];
diff --git a/av1/encoder/encoder.h b/av1/encoder/encoder.h index fcf9f2a..f7747e0 100644 --- a/av1/encoder/encoder.h +++ b/av1/encoder/encoder.h
@@ -1043,18 +1043,6 @@ #define LAYER_IDS_TO_IDX(sl, tl, num_tl) ((sl) * (num_tl) + (tl)) -// Update up-sampled reference frame index. -static INLINE void uref_cnt_fb(EncRefCntBuffer *ubufs, int *uidx, - int new_uidx) { - const int ref_index = *uidx; - - if (ref_index >= 0 && ubufs[ref_index].ref_count > 0) - ubufs[ref_index].ref_count--; - - *uidx = new_uidx; - ubufs[new_uidx].ref_count++; -} - // Returns 1 if a frame is scaled and 0 otherwise. static INLINE int av1_resize_scaled(const AV1_COMMON *cm) { return !(cm->superres_upscaled_width == cm->render_width &&
diff --git a/av1/encoder/firstpass.c b/av1/encoder/firstpass.c index 00aa0ad..ca2761e 100644 --- a/av1/encoder/firstpass.c +++ b/av1/encoder/firstpass.c
@@ -1060,9 +1060,10 @@ ((twopass->this_frame_stats.intra_error / DOUBLE_DIVIDE_CHECK(twopass->this_frame_stats.coded_error)) > 2.0))) { if (gld_yv12 != NULL) { - ref_cnt_fb(pool->frame_bufs, - &cm->ref_frame_map[cpi->remapped_ref_idx[GOLDEN_FRAME - 1]], - cm->ref_frame_map[cpi->remapped_ref_idx[LAST_FRAME - 1]]); + assign_frame_buffer( + pool->frame_bufs, + &cm->ref_frame_map[cpi->remapped_ref_idx[GOLDEN_FRAME - 1]], + cm->ref_frame_map[cpi->remapped_ref_idx[LAST_FRAME - 1]]); } twopass->sr_update_lag = 1; } else { @@ -1072,17 +1073,18 @@ aom_extend_frame_borders(new_yv12, num_planes); // The frame we just compressed now becomes the last frame. - ref_cnt_fb(pool->frame_bufs, - &cm->ref_frame_map[cpi->remapped_ref_idx[LAST_FRAME - 1]], - cm->new_fb_idx); + assign_frame_buffer(pool->frame_bufs, + &cm->ref_frame_map[cpi->remapped_ref_idx[LAST_FRAME - 1]], + cm->new_fb_idx); // Special case for the first frame. Copy into the GF buffer as a second // reference. if (cm->current_video_frame == 0 && cpi->remapped_ref_idx[GOLDEN_FRAME - 1] != INVALID_IDX) { - ref_cnt_fb(pool->frame_bufs, - &cm->ref_frame_map[cpi->remapped_ref_idx[GOLDEN_FRAME - 1]], - cm->ref_frame_map[cpi->remapped_ref_idx[LAST_FRAME - 1]]); + assign_frame_buffer( + pool->frame_bufs, + &cm->ref_frame_map[cpi->remapped_ref_idx[GOLDEN_FRAME - 1]], + cm->ref_frame_map[cpi->remapped_ref_idx[LAST_FRAME - 1]]); } // Use this to see what the first pass reconstruction looks like.