From 29d0840b5c79df20fab70e5552f1bceb43a8bddb Mon Sep 17 00:00:00 2001 From: ilnik Date: Tue, 11 Jul 2017 08:08:12 -0700 Subject: [PATCH] Reland of Refactor timing frame logic to work with encoders with internal sources (patchset #1 id:1 of https://codereview.webrtc.org/2980533002/ ) BUG=webrtc:7594,webrtc:7893 Review-Url: https://codereview.webrtc.org/2974893002 Cr-Commit-Position: refs/heads/master@{#18974} --- .../modules/video_coding/generic_encoder.cc | 87 ++++++++++--------- 1 file changed, 45 insertions(+), 42 deletions(-) diff --git a/webrtc/modules/video_coding/generic_encoder.cc b/webrtc/modules/video_coding/generic_encoder.cc index 17e4032144..78eeef288a 100644 --- a/webrtc/modules/video_coding/generic_encoder.cc +++ b/webrtc/modules/video_coding/generic_encoder.cc @@ -17,6 +17,7 @@ #include "webrtc/modules/video_coding/media_optimization.h" #include "webrtc/rtc_base/checks.h" #include "webrtc/rtc_base/logging.h" +#include "webrtc/rtc_base/optional.h" #include "webrtc/rtc_base/timeutils.h" #include "webrtc/rtc_base/trace_event.h" @@ -216,9 +217,6 @@ EncodedImageCallback::Result VCMEncodedFrameCallback::OnEncodedImage( const RTPFragmentationHeader* fragmentation_header) { TRACE_EVENT_INSTANT1("webrtc", "VCMEncodedFrameCallback::Encoded", "timestamp", encoded_image._timeStamp); - bool is_timing_frame = false; - size_t outlier_frame_size = 0; - int64_t encode_start_ms = -1; size_t simulcast_svc_idx = 0; if (codec_specific->codecType == kVideoCodecVP9) { if (codec_specific->codecSpecific.VP9.num_spatial_layers > 1) @@ -231,64 +229,69 @@ EncodedImageCallback::Result VCMEncodedFrameCallback::OnEncodedImage( // TODO(ilnik): When h264 simulcast is landed, extract simulcast idx here. } + rtc::Optional outlier_frame_size; + rtc::Optional encode_start_ms; + bool is_timing_frame = false; { rtc::CritScope crit(&timing_params_lock_); - // TODO(ilnik): Workaround for hardware encoders, which do not call - // |OnEncodeStarted| correctly. Once fixed, remove conditional check. - if (simulcast_svc_idx < timing_frames_info_.size()) { - RTC_CHECK_LT(simulcast_svc_idx, timing_frames_info_.size()); + // Encoders with internal sources do not call OnEncodeStarted and + // OnFrameRateChanged. |timing_frames_info_| may be not filled here. + if (simulcast_svc_idx < timing_frames_info_.size()) { auto encode_start_map = &timing_frames_info_[simulcast_svc_idx].encode_start_time_ms; auto it = encode_start_map->find(encoded_image.capture_time_ms_); if (it != encode_start_map->end()) { - encode_start_ms = it->second; + encode_start_ms.emplace(it->second); // Assuming all encoders do not reorder frames within single stream, // there may be some dropped frames with smaller timestamps. These // should be purged. encode_start_map->erase(encode_start_map->begin(), it); encode_start_map->erase(it); } else { - // Some chromium remoting unittests use generic encoder incorrectly - // If timestamps do not match, purge them all. - encode_start_map->erase(encode_start_map->begin(), - encode_start_map->end()); + // Encoder is with internal source: free our records of any frames just + // in case to free memory. + encode_start_map->clear(); } - int64_t timing_frame_delay_ms = - encoded_image.capture_time_ms_ - last_timing_frame_time_ms_; - if (last_timing_frame_time_ms_ == -1 || - timing_frame_delay_ms >= timing_frames_thresholds_.delay_ms || - timing_frame_delay_ms == 0) { - is_timing_frame = true; - last_timing_frame_time_ms_ = encoded_image.capture_time_ms_; + size_t target_bitrate = + timing_frames_info_[simulcast_svc_idx].target_bitrate_bytes_per_sec; + if (framerate_ > 0 && target_bitrate > 0) { + // framerate and target bitrate were reported by encoder. + size_t average_frame_size = target_bitrate / framerate_; + outlier_frame_size.emplace( + average_frame_size * + timing_frames_thresholds_.outlier_ratio_percent / 100); } - // TODO(ilnik): Once OnFramerateChanged is called correctly by hardware - // encoders, remove the conditional check below. - if (framerate_ > 0) { - RTC_CHECK_GT(framerate_, 0); - size_t average_frame_size = - timing_frames_info_[simulcast_svc_idx].target_bitrate_bytes_per_sec - / framerate_; - outlier_frame_size = average_frame_size * - timing_frames_thresholds_.outlier_ratio_percent / - 100; - } else { - outlier_frame_size = encoded_image._length + 1; - } - } else { - // We don't have any information prior to encode start, thus we can't - // reliably detect outliers. Set outlier size to anything larger than - // current frame size. - outlier_frame_size = encoded_image._length + 1; + } + + // Check if it's time to send a timing frame. + int64_t timing_frame_delay_ms = + encoded_image.capture_time_ms_ - last_timing_frame_time_ms_; + // Trigger threshold if it's a first frame, too long passed since the last + // timing frame, or we already sent timing frame on a different simulcast + // stream with the same capture time. + if (last_timing_frame_time_ms_ == -1 || + timing_frame_delay_ms >= timing_frames_thresholds_.delay_ms || + timing_frame_delay_ms == 0) { + is_timing_frame = true; + last_timing_frame_time_ms_ = encoded_image.capture_time_ms_; + } + + // Outliers trigger timing frames, but do not affect scheduled timing + // frames. + if (outlier_frame_size && encoded_image._length >= *outlier_frame_size) { + is_timing_frame = true; } } - if (encoded_image._length >= outlier_frame_size) { - is_timing_frame = true; - } - if (encode_start_ms >= 0 && is_timing_frame) { - encoded_image.SetEncodeTime(encode_start_ms, rtc::TimeMillis()); + // If encode start is not available that means that encoder uses internal + // source. In that case capture timestamp may be from a different clock with a + // drift relative to rtc::TimeMillis(). We can't use it for Timing frames, + // because to being sent in the network capture time required to be less than + // all the other timestamps. + if (is_timing_frame && encode_start_ms) { + encoded_image.SetEncodeTime(*encode_start_ms, rtc::TimeMillis()); } Result result = post_encode_callback_->OnEncodedImage(