Don't decode frames with an older timestamp than the last decoded timestamp.
This change prevents decoding corruption by not allowing keyframes with a newer frame id but an older timestamp to be decoded. This does not handle reordering well. Bug: none Change-Id: I4a67ca84ee86a782da74a10530c531d893d3bd3c Reviewed-on: https://webrtc-review.googlesource.com/c/107304 Reviewed-by: Rasmus Brandt <brandtr@webrtc.org> Commit-Queue: Philip Eliasson <philipel@webrtc.org> Cr-Commit-Position: refs/heads/master@{#25292}
This commit is contained in:
@ -50,7 +50,6 @@ FrameBuffer::FrameBuffer(Clock* clock,
|
||||
jitter_estimator_(jitter_estimator),
|
||||
timing_(timing),
|
||||
inter_frame_delay_(clock_->TimeInMilliseconds()),
|
||||
last_decoded_frame_timestamp_(0),
|
||||
last_decoded_frame_it_(frames_.end()),
|
||||
last_continuous_frame_it_(frames_.end()),
|
||||
num_frames_history_(0),
|
||||
@ -115,10 +114,16 @@ FrameBuffer::ReturnReason FrameBuffer::NextFrame(
|
||||
if (keyframe_required && !frame->is_keyframe())
|
||||
continue;
|
||||
|
||||
if (last_decoded_frame_timestamp_ &&
|
||||
AheadOf(*last_decoded_frame_timestamp_, frame->Timestamp())) {
|
||||
continue;
|
||||
}
|
||||
|
||||
next_frame_it_ = frame_it;
|
||||
if (frame->RenderTime() == -1)
|
||||
if (frame->RenderTime() == -1) {
|
||||
frame->SetRenderTime(
|
||||
timing_->RenderTimeMs(frame->Timestamp(), now_ms));
|
||||
}
|
||||
wait_ms = timing_->MaxWaitingTime(frame->RenderTime(), now_ms);
|
||||
|
||||
// This will cause the frame buffer to prefer high framerate rather
|
||||
@ -175,33 +180,6 @@ FrameBuffer::ReturnReason FrameBuffer::NextFrame(
|
||||
UpdateTimingFrameInfo();
|
||||
PropagateDecodability(next_frame_it_->second);
|
||||
|
||||
// Sanity check for RTP timestamp monotonicity.
|
||||
if (last_decoded_frame_it_ != frames_.end()) {
|
||||
const VideoLayerFrameId& last_decoded_frame_key =
|
||||
last_decoded_frame_it_->first;
|
||||
const VideoLayerFrameId& frame_key = next_frame_it_->first;
|
||||
|
||||
const bool frame_is_higher_spatial_layer_of_last_decoded_frame =
|
||||
last_decoded_frame_timestamp_ == frame->Timestamp() &&
|
||||
last_decoded_frame_key.picture_id == frame_key.picture_id &&
|
||||
last_decoded_frame_key.spatial_layer < frame_key.spatial_layer;
|
||||
|
||||
if (AheadOrAt(last_decoded_frame_timestamp_, frame->Timestamp()) &&
|
||||
!frame_is_higher_spatial_layer_of_last_decoded_frame) {
|
||||
// TODO(brandtr): Consider clearing the entire buffer when we hit
|
||||
// these conditions.
|
||||
RTC_LOG(LS_WARNING)
|
||||
<< "Frame with (timestamp:picture_id:spatial_id) ("
|
||||
<< frame->Timestamp() << ":" << frame->id.picture_id << ":"
|
||||
<< static_cast<int>(frame->id.spatial_layer) << ")"
|
||||
<< " sent to decoder after frame with"
|
||||
<< " (timestamp:picture_id:spatial_id) ("
|
||||
<< last_decoded_frame_timestamp_ << ":"
|
||||
<< last_decoded_frame_key.picture_id << ":"
|
||||
<< static_cast<int>(last_decoded_frame_key.spatial_layer) << ").";
|
||||
}
|
||||
}
|
||||
|
||||
AdvanceLastDecodedFrame(next_frame_it_);
|
||||
last_decoded_frame_timestamp_ = frame->Timestamp();
|
||||
*frame_out = std::move(frame);
|
||||
@ -348,7 +326,7 @@ int64_t FrameBuffer::InsertFrame(std::unique_ptr<EncodedFrame> frame) {
|
||||
|
||||
if (last_decoded_frame_it_ != frames_.end() &&
|
||||
id <= last_decoded_frame_it_->first) {
|
||||
if (AheadOf(frame->Timestamp(), last_decoded_frame_timestamp_) &&
|
||||
if (AheadOf(frame->Timestamp(), *last_decoded_frame_timestamp_) &&
|
||||
frame->is_keyframe()) {
|
||||
// If this frame has a newer timestamp but an earlier picture id then we
|
||||
// assume there has been a jump in the picture id due to some encoder
|
||||
|
||||
@ -161,7 +161,7 @@ class FrameBuffer {
|
||||
VCMJitterEstimator* const jitter_estimator_ RTC_GUARDED_BY(crit_);
|
||||
VCMTiming* const timing_ RTC_GUARDED_BY(crit_);
|
||||
VCMInterFrameDelay inter_frame_delay_ RTC_GUARDED_BY(crit_);
|
||||
uint32_t last_decoded_frame_timestamp_ RTC_GUARDED_BY(crit_);
|
||||
absl::optional<uint32_t> last_decoded_frame_timestamp_ RTC_GUARDED_BY(crit_);
|
||||
FrameMap::iterator last_decoded_frame_it_ RTC_GUARDED_BY(crit_);
|
||||
FrameMap::iterator last_continuous_frame_it_ RTC_GUARDED_BY(crit_);
|
||||
FrameMap::iterator next_frame_it_ RTC_GUARDED_BY(crit_);
|
||||
|
||||
@ -600,5 +600,21 @@ TEST_F(TestFrameBuffer2, DontUpdateOnUndecodableFrame) {
|
||||
ExtractFrame(0, true);
|
||||
}
|
||||
|
||||
TEST_F(TestFrameBuffer2, DontDecodeOlderTimestamp) {
|
||||
InsertFrame(2, 0, 1, false);
|
||||
InsertFrame(1, 0, 2, false); // Older picture id but newer timestamp.
|
||||
ExtractFrame(0);
|
||||
ExtractFrame(0);
|
||||
CheckFrame(0, 1, 0);
|
||||
CheckNoFrame(1);
|
||||
|
||||
InsertFrame(3, 0, 4, false);
|
||||
InsertFrame(4, 0, 3, false); // Newer picture id but older timestamp.
|
||||
ExtractFrame(0);
|
||||
ExtractFrame(0);
|
||||
CheckFrame(2, 3, 0);
|
||||
CheckNoFrame(3);
|
||||
}
|
||||
|
||||
} // namespace video_coding
|
||||
} // namespace webrtc
|
||||
|
||||
Reference in New Issue
Block a user