Validate references of frames inserted into FrameBuffer2.
BUG=chromium:730603
Review-Url: https://codereview.webrtc.org/2937243002
Cr-Commit-Position: refs/heads/master@{#18614}
diff --git a/webrtc/modules/video_coding/frame_buffer2.cc b/webrtc/modules/video_coding/frame_buffer2.cc
index de96a52..1114e7c 100644
--- a/webrtc/modules/video_coding/frame_buffer2.cc
+++ b/webrtc/modules/video_coding/frame_buffer2.cc
@@ -238,6 +238,22 @@
new_continuous_frame_event_.Set();
}
+bool FrameBuffer::ValidReferences(const FrameObject& frame) const {
+ for (size_t i = 0; i < frame.num_references; ++i) {
+ if (AheadOrAt(frame.references[i], frame.picture_id))
+ return false;
+ for (size_t j = i + 1; j < frame.num_references; ++j) {
+ if (frame.references[i] == frame.references[j])
+ return false;
+ }
+ }
+
+ if (frame.inter_layer_predicted && frame.spatial_layer == 0)
+ return false;
+
+ return true;
+}
+
void FrameBuffer::UpdatePlayoutDelays(const FrameObject& frame) {
TRACE_EVENT0("webrtc", "FrameBuffer::UpdatePlayoutDelays");
PlayoutDelay playout_delay = frame.EncodedImage().playout_delay_;
@@ -262,6 +278,13 @@
? -1
: last_continuous_frame_it_->first.picture_id;
+ if (!ValidReferences(*frame)) {
+ LOG(LS_WARNING) << "Frame with (picture_id:spatial_id) (" << key.picture_id
+ << ":" << static_cast<int>(key.spatial_layer)
+ << ") has invalid frame references, dropping frame.";
+ return last_continuous_picture_id;
+ }
+
if (num_frames_buffered_ >= kMaxFramesBuffered) {
LOG(LS_WARNING) << "Frame with (picture_id:spatial_id) (" << key.picture_id
<< ":" << static_cast<int>(key.spatial_layer)
@@ -270,13 +293,6 @@
return last_continuous_picture_id;
}
- if (frame->inter_layer_predicted && frame->spatial_layer == 0) {
- LOG(LS_WARNING) << "Frame with (picture_id:spatial_id) (" << key.picture_id
- << ":" << static_cast<int>(key.spatial_layer)
- << ") is marked as inter layer predicted, dropping frame.";
- return last_continuous_picture_id;
- }
-
if (last_decoded_frame_it_ != frames_.end() &&
key <= last_decoded_frame_it_->first) {
if (AheadOf(frame->timestamp, last_decoded_frame_timestamp_) &&
@@ -363,11 +379,15 @@
// any unfulfilled dependencies then that frame is continuous as well.
for (size_t d = 0; d < frame->second.num_dependent_frames; ++d) {
auto frame_ref = frames_.find(frame->second.dependent_frames[d]);
- --frame_ref->second.num_missing_continuous;
+ RTC_DCHECK(frame_ref != frames_.end());
- if (frame_ref->second.num_missing_continuous == 0) {
- frame_ref->second.continuous = true;
- continuous_frames.push(frame_ref);
+ // TODO(philipel): Look into why we've seen this happen.
+ if (frame_ref != frames_.end()) {
+ --frame_ref->second.num_missing_continuous;
+ if (frame_ref->second.num_missing_continuous == 0) {
+ frame_ref->second.continuous = true;
+ continuous_frames.push(frame_ref);
+ }
}
}
}
diff --git a/webrtc/modules/video_coding/frame_buffer2.h b/webrtc/modules/video_coding/frame_buffer2.h
index 6ce3b4b..ffeb2aa 100644
--- a/webrtc/modules/video_coding/frame_buffer2.h
+++ b/webrtc/modules/video_coding/frame_buffer2.h
@@ -97,6 +97,8 @@
// Which other frames that have direct unfulfilled dependencies
// on this frame.
+ // TODO(philipel): Add simple modify/access functions to prevent adding too
+ // many |dependent_frames|.
FrameKey dependent_frames[kMaxNumDependentFrames];
size_t num_dependent_frames = 0;
@@ -120,6 +122,9 @@
using FrameMap = std::map<FrameKey, FrameInfo>;
+ // Check that the references of |frame| are valid.
+ bool ValidReferences(const FrameObject& frame) const;
+
// Updates the minimal and maximal playout delays
// depending on the frame.
void UpdatePlayoutDelays(const FrameObject& frame)
diff --git a/webrtc/modules/video_coding/frame_buffer2_unittest.cc b/webrtc/modules/video_coding/frame_buffer2_unittest.cc
index 488b785..58b3f7a 100644
--- a/webrtc/modules/video_coding/frame_buffer2_unittest.cc
+++ b/webrtc/modules/video_coding/frame_buffer2_unittest.cc
@@ -531,5 +531,13 @@
EXPECT_EQ(22256, InsertFrame(22256, 0, 1, false));
}
+// TODO(philipel): implement more unittests related to invalid references.
+TEST_F(TestFrameBuffer2, InvalidReferences) {
+ EXPECT_EQ(-1, InsertFrame(0, 0, 1000, false, 2));
+ EXPECT_EQ(1, InsertFrame(1, 0, 2000, false));
+ ExtractFrame();
+ EXPECT_EQ(2, InsertFrame(2, 0, 3000, false, 1));
+}
+
} // namespace video_coding
} // namespace webrtc
diff --git a/webrtc/modules/video_coding/frame_object.h b/webrtc/modules/video_coding/frame_object.h
index 413cc33..fa0a2c1 100644
--- a/webrtc/modules/video_coding/frame_object.h
+++ b/webrtc/modules/video_coding/frame_object.h
@@ -51,6 +51,8 @@
uint8_t spatial_layer;
uint32_t timestamp;
+ // TODO(philipel): Add simple modify/access functions to prevent adding too
+ // many |references|.
size_t num_references;
uint16_t references[kMaxFrameReferences];
bool inter_layer_predicted;