From 8227659075523e79025cecf15a237cfb8925762f Mon Sep 17 00:00:00 2001 From: Sergey Silkin Date: Mon, 27 Aug 2018 14:54:20 +0200 Subject: [PATCH] Prevent duplicates in VP9 RTP p_diff list. VP9 encoder wrapper duplicated p_diff values in VP9 RTP payload descriptor for some frames. This confused ref frame finder which invalidated such frames on receiver. Bug: webrtc:9657 Change-Id: I5ddfed30d2908c5fbb8bce9d3dac0a519830e978 Reviewed-on: https://webrtc-review.googlesource.com/95701 Commit-Queue: Sergey Silkin Reviewed-by: Rasmus Brandt Cr-Commit-Position: refs/heads/master@{#24456} --- .../codecs/vp9/test/vp9_impl_unittest.cc | 9 ++++++++- modules/video_coding/codecs/vp9/vp9_impl.cc | 15 ++++++++++++--- modules/video_coding/codecs/vp9/vp9_impl.h | 6 ++++++ 3 files changed, 26 insertions(+), 4 deletions(-) diff --git a/modules/video_coding/codecs/vp9/test/vp9_impl_unittest.cc b/modules/video_coding/codecs/vp9/test/vp9_impl_unittest.cc index 5b5db88b6e..f6594c1f7c 100644 --- a/modules/video_coding/codecs/vp9/test/vp9_impl_unittest.cc +++ b/modules/video_coding/codecs/vp9/test/vp9_impl_unittest.cc @@ -67,7 +67,14 @@ class TestVp9Impl : public VideoCodecUnitTest { EXPECT_EQ(vp9.temporal_idx, temporal_idx); } EXPECT_EQ(vp9.temporal_up_switch, temporal_up_switch); - EXPECT_EQ(vp9.num_ref_pics, num_ref_pics); + + // Ensure there are no duplicates in reference list. + std::vector vp9_p_diff(vp9.p_diff, + vp9.p_diff + vp9.num_ref_pics); + std::sort(vp9_p_diff.begin(), vp9_p_diff.end()); + EXPECT_EQ(std::unique(vp9_p_diff.begin(), vp9_p_diff.end()), + vp9_p_diff.end()); + for (size_t ref_pic_num = 0; ref_pic_num < num_ref_pics; ++ref_pic_num) { EXPECT_NE( std::find(p_diff.begin(), p_diff.end(), vp9.p_diff[ref_pic_num]), diff --git a/modules/video_coding/codecs/vp9/vp9_impl.cc b/modules/video_coding/codecs/vp9/vp9_impl.cc index 5829bc1bf7..b4c13f9ccf 100644 --- a/modules/video_coding/codecs/vp9/vp9_impl.cc +++ b/modules/video_coding/codecs/vp9/vp9_impl.cc @@ -870,21 +870,30 @@ void VP9EncoderImpl::FillReferenceIndices(const vpx_codec_cx_pkt& pkt, const size_t fb_idx = enc_layer_conf.lst_fb_idx[layer_id.spatial_layer_id]; RTC_DCHECK(ref_buf_.find(fb_idx) != ref_buf_.end()); - ref_buf_list.push_back(ref_buf_.at(fb_idx)); + if (std::find(ref_buf_list.begin(), ref_buf_list.end(), + ref_buf_.at(fb_idx)) == ref_buf_list.end()) { + ref_buf_list.push_back(ref_buf_.at(fb_idx)); + } } if (enc_layer_conf.reference_alt_ref[layer_id.spatial_layer_id]) { const size_t fb_idx = enc_layer_conf.alt_fb_idx[layer_id.spatial_layer_id]; RTC_DCHECK(ref_buf_.find(fb_idx) != ref_buf_.end()); - ref_buf_list.push_back(ref_buf_.at(fb_idx)); + if (std::find(ref_buf_list.begin(), ref_buf_list.end(), + ref_buf_.at(fb_idx)) == ref_buf_list.end()) { + ref_buf_list.push_back(ref_buf_.at(fb_idx)); + } } if (enc_layer_conf.reference_golden[layer_id.spatial_layer_id]) { const size_t fb_idx = enc_layer_conf.gld_fb_idx[layer_id.spatial_layer_id]; RTC_DCHECK(ref_buf_.find(fb_idx) != ref_buf_.end()); - ref_buf_list.push_back(ref_buf_.at(fb_idx)); + if (std::find(ref_buf_list.begin(), ref_buf_list.end(), + ref_buf_.at(fb_idx)) == ref_buf_list.end()) { + ref_buf_list.push_back(ref_buf_.at(fb_idx)); + } } } else if (!is_key_frame) { RTC_DCHECK_EQ(num_spatial_layers_, 1); diff --git a/modules/video_coding/codecs/vp9/vp9_impl.h b/modules/video_coding/codecs/vp9/vp9_impl.h index 7009311072..d4483060ee 100644 --- a/modules/video_coding/codecs/vp9/vp9_impl.h +++ b/modules/video_coding/codecs/vp9/vp9_impl.h @@ -131,6 +131,12 @@ class VP9EncoderImpl : public VP9Encoder { spatial_layer_id(spatial_layer_id), temporal_layer_id(temporal_layer_id) {} RefFrameBuffer() {} + + bool operator==(const RefFrameBuffer& o) { + return pic_num == o.pic_num && spatial_layer_id == o.spatial_layer_id && + temporal_layer_id == o.temporal_layer_id; + } + size_t pic_num = 0; size_t spatial_layer_id = 0; size_t temporal_layer_id = 0;