Don't update internal state of the FrameBuffer2 when an undecodable frame is inserted.
Bug: chromium:844313
Change-Id: I034bcb47092815695084e37c81150bafbfbc6b9c
Reviewed-on: https://webrtc-review.googlesource.com/79944
Commit-Queue: Philip Eliasson <philipel@webrtc.org>
Reviewed-by: Björn Terelius <terelius@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#23577}diff --git a/modules/video_coding/frame_buffer2.cc b/modules/video_coding/frame_buffer2.cc
index 2a437d7..37feae1 100644
--- a/modules/video_coding/frame_buffer2.cc
+++ b/modules/video_coding/frame_buffer2.cc
@@ -13,6 +13,7 @@
#include <algorithm>
#include <cstring>
#include <queue>
+#include <vector>
#include "modules/video_coding/include/video_coding_defines.h"
#include "modules/video_coding/jitter_estimator.h"
@@ -486,20 +487,34 @@
FrameMap::iterator info) {
TRACE_EVENT0("webrtc", "FrameBuffer::UpdateFrameInfoWithIncomingFrame");
const VideoLayerFrameId& id = frame.id;
- info->second.num_missing_continuous = frame.num_references;
- info->second.num_missing_decodable = frame.num_references;
RTC_DCHECK(last_decoded_frame_it_ == frames_.end() ||
last_decoded_frame_it_->first < info->first);
- // Check how many dependencies that have already been fulfilled.
+ // In this function we determine how many missing dependencies this |frame|
+ // has to become continuous/decodable. If a frame that this |frame| depend
+ // on has already been decoded then we can ignore that dependency since it has
+ // already been fulfilled.
+ //
+ // For all other frames we will register a backwards reference to this |frame|
+ // so that |num_missing_continuous| and |num_missing_decodable| can be
+ // decremented as frames become continuous/are decoded.
+ struct Dependency {
+ VideoLayerFrameId id;
+ bool continuous;
+ };
+ std::vector<Dependency> not_yet_fulfilled_dependencies;
+
+ // Find all dependencies that have not yet been fulfilled.
for (size_t i = 0; i < frame.num_references; ++i) {
VideoLayerFrameId ref_key(frame.references[i], frame.id.spatial_layer);
auto ref_info = frames_.find(ref_key);
- // Does |frame| depend on a frame earlier than the last decoded frame?
+ // Does |frame| depend on a frame earlier than the last decoded one?
if (last_decoded_frame_it_ != frames_.end() &&
ref_key <= last_decoded_frame_it_->first) {
+ // Was that frame decoded? If not, this |frame| will never become
+ // decodable.
if (ref_info == frames_.end()) {
int64_t now_ms = clock_->TimeInMilliseconds();
if (last_log_non_decoded_ms_ + kLogNonDecodedIntervalMs < now_ms) {
@@ -512,57 +527,51 @@
}
return false;
}
-
- --info->second.num_missing_continuous;
- --info->second.num_missing_decodable;
} else {
- if (ref_info == frames_.end())
- ref_info = frames_.insert(std::make_pair(ref_key, FrameInfo())).first;
-
- if (ref_info->second.continuous)
- --info->second.num_missing_continuous;
-
- // Add backwards reference so |frame| can be updated when new
- // frames are inserted or decoded.
- ref_info->second.dependent_frames[ref_info->second.num_dependent_frames] =
- id;
- RTC_DCHECK_LT(ref_info->second.num_dependent_frames,
- (FrameInfo::kMaxNumDependentFrames - 1));
- // TODO(philipel): Look into why this could happen and handle
- // appropriately.
- if (ref_info->second.num_dependent_frames <
- (FrameInfo::kMaxNumDependentFrames - 1)) {
- ++ref_info->second.num_dependent_frames;
- }
+ bool ref_continuous =
+ ref_info != frames_.end() && ref_info->second.continuous;
+ not_yet_fulfilled_dependencies.push_back({ref_key, ref_continuous});
}
- RTC_DCHECK_LE(ref_info->second.num_missing_continuous,
- ref_info->second.num_missing_decodable);
}
- // Check if we have the lower spatial layer frame.
+ // Does |frame| depend on the lower spatial layer?
if (frame.inter_layer_predicted) {
- ++info->second.num_missing_continuous;
- ++info->second.num_missing_decodable;
-
VideoLayerFrameId ref_key(frame.id.picture_id, frame.id.spatial_layer - 1);
- // Gets or create the FrameInfo for the referenced frame.
- auto ref_info = frames_.insert(std::make_pair(ref_key, FrameInfo())).first;
- if (ref_info->second.continuous)
- --info->second.num_missing_continuous;
+ auto ref_info = frames_.find(ref_key);
- if (ref_info == last_decoded_frame_it_) {
- --info->second.num_missing_decodable;
- } else {
- ref_info->second.dependent_frames[ref_info->second.num_dependent_frames] =
- id;
- ++ref_info->second.num_dependent_frames;
+ bool lower_layer_continuous =
+ ref_info != frames_.end() && ref_info->second.continuous;
+ bool lower_layer_decoded = last_decoded_frame_it_ != frames_.end() &&
+ last_decoded_frame_it_->first == ref_key;
+
+ if (!lower_layer_continuous || !lower_layer_decoded) {
+ not_yet_fulfilled_dependencies.push_back(
+ {ref_key, lower_layer_continuous});
}
- RTC_DCHECK_LE(ref_info->second.num_missing_continuous,
- ref_info->second.num_missing_decodable);
}
- RTC_DCHECK_LE(info->second.num_missing_continuous,
- info->second.num_missing_decodable);
+ info->second.num_missing_continuous = not_yet_fulfilled_dependencies.size();
+ info->second.num_missing_decodable = not_yet_fulfilled_dependencies.size();
+
+ for (const Dependency& dep : not_yet_fulfilled_dependencies) {
+ if (dep.continuous)
+ --info->second.num_missing_continuous;
+
+ // At this point we know we want to insert this frame, so here we
+ // intentionally get or create the FrameInfo for this dependency.
+ FrameInfo* dep_info = &frames_[dep.id];
+
+ if (dep_info->num_dependent_frames <
+ (FrameInfo::kMaxNumDependentFrames - 1)) {
+ dep_info->dependent_frames[dep_info->num_dependent_frames] = id;
+ ++dep_info->num_dependent_frames;
+ } else {
+ RTC_LOG(LS_WARNING) << "Frame with (picture_id:spatial_id) ("
+ << dep.id.picture_id << ":"
+ << static_cast<int>(dep.id.spatial_layer)
+ << ") is referenced by too many frames.";
+ }
+ }
return true;
}