Revert "Add fine grained dropped video frames counters on sending side"
This reverts commit 4b1a363e4c238f2e1ec2d8a9ce1f819f59d710ce.
Reason for revert: Breaks dependent android projects.
Original change's description:
> Add fine grained dropped video frames counters on sending side
>
> 4 new counters added to SendStatisticsProxy and reported to UMA and logs.
>
> Bug: webrtc:8355
> Change-Id: Idf9b8dfc295c92821e058a97cb3894dc6a446082
> Reviewed-on: https://webrtc-review.googlesource.com/12260
> Commit-Queue: Ilya Nikolaevskiy <ilnik@webrtc.org>
> Reviewed-by: Stefan Holmer <stefan@webrtc.org>
> Reviewed-by: Erik Språng <sprang@webrtc.org>
> Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#20347}
TBR=deadbeef@webrtc.org,ilnik@webrtc.org,sprang@webrtc.org,stefan@webrtc.org
# Not skipping CQ checks because original CL landed > 1 day ago.
Bug: webrtc:8355
Change-Id: I59b02f4eb77abad7ff1fbcbfa61844918c95d723
Reviewed-on: https://webrtc-review.googlesource.com/14500
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Commit-Queue: Ilya Nikolaevskiy <ilnik@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#20378}diff --git a/api/video_codecs/video_encoder.h b/api/video_codecs/video_encoder.h
index eb401af..505564c 100644
--- a/api/video_codecs/video_encoder.h
+++ b/api/video_codecs/video_encoder.h
@@ -56,25 +56,13 @@
bool drop_next_frame = false;
};
- // Used to signal the encoder about reason a frame is dropped.
- // kDroppedByMediaOptimizations - dropped by MediaOptimizations (for rate
- // limiting purposes).
- // kDroppedByEncoder - dropped by encoder's internal rate limiter.
- enum class DropReason : uint8_t {
- kDroppedByMediaOptimizations,
- kDroppedByEncoder
- };
-
// Callback function which is called when an image has been encoded.
virtual Result OnEncodedImage(
const EncodedImage& encoded_image,
const CodecSpecificInfo* codec_specific_info,
const RTPFragmentationHeader* fragmentation) = 0;
- // Deprecated. TODO(ilnik): Remove this in few weeks.
virtual void OnDroppedFrame() {}
-
- virtual void OnDroppedFrame(DropReason reason) {}
};
class VideoEncoder {
diff --git a/call/video_send_stream.h b/call/video_send_stream.h
index 3bdb8ce..1bc08cb 100644
--- a/call/video_send_stream.h
+++ b/call/video_send_stream.h
@@ -63,10 +63,6 @@
int avg_encode_time_ms = 0;
int encode_usage_percent = 0;
uint32_t frames_encoded = 0;
- uint32_t frames_dropped_by_capturer = 0;
- uint32_t frames_dropped_by_encoder_queue = 0;
- uint32_t frames_dropped_by_rate_limiter = 0;
- uint32_t frames_dropped_by_encoder = 0;
rtc::Optional<uint64_t> qp_sum;
// Bitrate the encoder is currently configured to use due to bandwidth
// limitations.
diff --git a/media/base/adaptedvideotracksource.cc b/media/base/adaptedvideotracksource.cc
index 16f11b9..4b41361 100644
--- a/media/base/adaptedvideotracksource.cc
+++ b/media/base/adaptedvideotracksource.cc
@@ -105,7 +105,6 @@
if (!video_adapter_.AdaptFrameResolution(
width, height, time_us * rtc::kNumNanosecsPerMicrosec,
crop_width, crop_height, out_width, out_height)) {
- broadcaster_.OnDiscardedFrame();
// VideoAdapter dropped the frame.
return false;
}
diff --git a/media/base/videobroadcaster.cc b/media/base/videobroadcaster.cc
index 344c3cc..83ed844 100644
--- a/media/base/videobroadcaster.cc
+++ b/media/base/videobroadcaster.cc
@@ -73,12 +73,6 @@
}
}
-void VideoBroadcaster::OnDiscardedFrame() {
- for (auto& sink_pair : sink_pairs()) {
- sink_pair.sink->OnDiscardedFrame();
- }
-}
-
void VideoBroadcaster::UpdateWants() {
RTC_DCHECK(thread_checker_.CalledOnValidThread());
diff --git a/media/base/videobroadcaster.h b/media/base/videobroadcaster.h
index a8e21fa..4236e96 100644
--- a/media/base/videobroadcaster.h
+++ b/media/base/videobroadcaster.h
@@ -50,8 +50,6 @@
// to the VideoSinkWants.
void OnFrame(const webrtc::VideoFrame& frame) override;
- void OnDiscardedFrame() override;
-
protected:
void UpdateWants() RTC_EXCLUSIVE_LOCKS_REQUIRED(sinks_and_wants_lock_);
const rtc::scoped_refptr<webrtc::VideoFrameBuffer>& GetBlackFrameBuffer(
diff --git a/media/base/videocapturer.cc b/media/base/videocapturer.cc
index 43f0630..592f3e2 100644
--- a/media/base/videocapturer.cc
+++ b/media/base/videocapturer.cc
@@ -179,7 +179,6 @@
width, height, camera_time_us * rtc::kNumNanosecsPerMicrosec,
crop_width, crop_height, out_width, out_height)) {
// VideoAdapter dropped the frame.
- broadcaster_.OnDiscardedFrame();
return false;
}
*crop_x = (width - *crop_width) / 2;
diff --git a/media/base/videosinkinterface.h b/media/base/videosinkinterface.h
index e8e48d8..bbcd529 100644
--- a/media/base/videosinkinterface.h
+++ b/media/base/videosinkinterface.h
@@ -11,7 +11,6 @@
#ifndef MEDIA_BASE_VIDEOSINKINTERFACE_H_
#define MEDIA_BASE_VIDEOSINKINTERFACE_H_
-#include <rtc_base/checks.h>
// TODO(nisse): Consider moving this interface (and possibly
// VideoSourceInterface too) from media/base to common_video, to
// reduce dependency cycles.
@@ -23,10 +22,6 @@
virtual ~VideoSinkInterface() {}
virtual void OnFrame(const VideoFrameT& frame) = 0;
-
- // Should be called by the source when it discards the frame due to rate
- // limiting.
- virtual void OnDiscardedFrame(){};
};
} // namespace rtc
diff --git a/modules/video_coding/generic_encoder.cc b/modules/video_coding/generic_encoder.cc
index 5066ba1..c4affcf 100644
--- a/modules/video_coding/generic_encoder.cc
+++ b/modules/video_coding/generic_encoder.cc
@@ -216,20 +216,8 @@
rtc::CritScope crit(&timing_params_lock_);
if (timing_frames_info_.size() < simulcast_svc_idx + 1)
timing_frames_info_.resize(simulcast_svc_idx + 1);
- RTC_DCHECK(
- timing_frames_info_[simulcast_svc_idx].encode_start_list.empty() ||
- rtc::TimeDiff(capture_time_ms, timing_frames_info_[simulcast_svc_idx]
- .encode_start_list.back()
- .capture_time_ms) >= 0);
- if (timing_frames_info_[simulcast_svc_idx].encode_start_list.size() ==
- kMaxEncodeStartTimeListSize) {
- LOG(LS_WARNING) << "Too many frames in the encode_start_list."
- " Did encoder stall?";
- post_encode_callback_->OnDroppedFrame(DropReason::kDroppedByEncoder);
- timing_frames_info_[simulcast_svc_idx].encode_start_list.pop_front();
- }
- timing_frames_info_[simulcast_svc_idx].encode_start_list.emplace_back(
- capture_time_ms, rtc::TimeMillis());
+ timing_frames_info_[simulcast_svc_idx].encode_start_time_ms[capture_time_ms] =
+ rtc::TimeMillis();
}
EncodedImageCallback::Result VCMEncodedFrameCallback::OnEncodedImage(
@@ -261,22 +249,20 @@
// OnFrameRateChanged. |timing_frames_info_| may be not filled here.
num_simulcast_svc_streams = timing_frames_info_.size();
if (simulcast_svc_idx < num_simulcast_svc_streams) {
- auto encode_start_list =
- &timing_frames_info_[simulcast_svc_idx].encode_start_list;
- // Skip frames for which there was OnEncodeStarted but no OnEncodedImage
- // call. These are dropped by encoder internally.
- while (!encode_start_list->empty() &&
- encode_start_list->front().capture_time_ms <
- encoded_image.capture_time_ms_) {
- post_encode_callback_->OnDroppedFrame(DropReason::kDroppedByEncoder);
- encode_start_list->pop_front();
- }
- if (encode_start_list->size() > 0 &&
- encode_start_list->front().capture_time_ms ==
- encoded_image.capture_time_ms_) {
- encode_start_ms.emplace(
- encode_start_list->front().encode_start_time_ms);
- encode_start_list->pop_front();
+ 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.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 {
+ // Encoder is with internal source: free our records of any frames just
+ // in case to free memory.
+ encode_start_map->clear();
}
size_t target_bitrate =
diff --git a/modules/video_coding/generic_encoder.h b/modules/video_coding/generic_encoder.h
index 2cdf664..9b003d6 100644
--- a/modules/video_coding/generic_encoder.h
+++ b/modules/video_coding/generic_encoder.h
@@ -12,7 +12,7 @@
#define MODULES_VIDEO_CODING_GENERIC_ENCODER_H_
#include <stdio.h>
-#include <list>
+#include <map>
#include <vector>
#include "modules/video_coding/include/video_codec_interface.h"
@@ -71,16 +71,9 @@
EncodedImageCallback* const post_encode_callback_;
media_optimization::MediaOptimization* const media_opt_;
- struct EncodeStartTimeRecord {
- EncodeStartTimeRecord(int64_t capture_time, int64_t encode_start_time)
- : capture_time_ms(capture_time),
- encode_start_time_ms(encode_start_time) {}
- int64_t capture_time_ms;
- int64_t encode_start_time_ms;
- };
struct TimingFramesLayerInfo {
size_t target_bitrate_bytes_per_sec = 0;
- std::list<EncodeStartTimeRecord> encode_start_list;
+ std::map<int64_t, int64_t> encode_start_time_ms;
};
// Separate instance for each simulcast stream or spatial layer.
std::vector<TimingFramesLayerInfo> timing_frames_info_
@@ -118,6 +111,7 @@
int32_t SetPeriodicKeyFrames(bool enable);
int32_t RequestFrame(const std::vector<FrameType>& frame_types);
bool InternalSource() const;
+ void OnDroppedFrame();
bool SupportsNativeHandle() const;
private:
diff --git a/modules/video_coding/generic_encoder_unittest.cc b/modules/video_coding/generic_encoder_unittest.cc
index 9c879e3..90d3c05 100644
--- a/modules/video_coding/generic_encoder_unittest.cc
+++ b/modules/video_coding/generic_encoder_unittest.cc
@@ -27,8 +27,7 @@
class FakeEncodedImageCallback : public EncodedImageCallback {
public:
- FakeEncodedImageCallback()
- : last_frame_was_timing_(false), num_frames_dropped_(0) {}
+ FakeEncodedImageCallback() : last_frame_was_timing_(false) {}
Result OnEncodedImage(const EncodedImage& encoded_image,
const CodecSpecificInfo* codec_specific_info,
const RTPFragmentationHeader* fragmentation) override {
@@ -37,15 +36,10 @@
return Result(Result::OK);
};
- void OnDroppedFrame(DropReason reason) override { ++num_frames_dropped_; }
-
bool WasTimingFrame() { return last_frame_was_timing_; }
- size_t GetNumFramesDropped() { return num_frames_dropped_; }
-
private:
bool last_frame_was_timing_;
- size_t num_frames_dropped_;
};
enum class FrameType {
@@ -196,34 +190,5 @@
EXPECT_FALSE(sink.WasTimingFrame());
}
-TEST(TestVCMEncodedFrameCallback, NotifiesAboutDroppedFrames) {
- EncodedImage image;
- CodecSpecificInfo codec_specific;
- const int64_t timestamp1 = 100;
- const int64_t timestamp2 = 110;
- const int64_t timestamp3 = 120;
- const int64_t timestamp4 = 130;
- codec_specific.codecType = kVideoCodecGeneric;
- codec_specific.codecSpecific.generic.simulcast_idx = 0;
- FakeEncodedImageCallback sink;
- VCMEncodedFrameCallback callback(&sink, nullptr);
- callback.OnEncodeStarted(timestamp1, 0);
- EXPECT_EQ(0u, sink.GetNumFramesDropped());
- image.capture_time_ms_ = timestamp1;
- callback.OnEncodedImage(image, &codec_specific, nullptr);
- callback.OnEncodeStarted(timestamp2, 0);
- // No OnEncodedImageCall for timestamp2. Yet, at this moment it's not known
- // that frame with timestamp2 was dropped.
- EXPECT_EQ(0u, sink.GetNumFramesDropped());
- callback.OnEncodeStarted(timestamp3, 0);
- image.capture_time_ms_ = timestamp3;
- callback.OnEncodedImage(image, &codec_specific, nullptr);
- EXPECT_EQ(1u, sink.GetNumFramesDropped());
- callback.OnEncodeStarted(timestamp4, 0);
- image.capture_time_ms_ = timestamp4;
- callback.OnEncodedImage(image, &codec_specific, nullptr);
- EXPECT_EQ(1u, sink.GetNumFramesDropped());
-}
-
} // namespace test
} // namespace webrtc
diff --git a/modules/video_coding/include/video_coding_defines.h b/modules/video_coding/include/video_coding_defines.h
index 5bf019d..7209792 100644
--- a/modules/video_coding/include/video_coding_defines.h
+++ b/modules/video_coding/include/video_coding_defines.h
@@ -45,8 +45,6 @@
// |kDefaultOutliserFrameSizePercent| in size of average frame.
kDefaultTimingFramesDelayMs = 200,
kDefaultOutlierFrameSizePercent = 250,
- // Maximum number of frames for what we store encode start timing information.
- kMaxEncodeStartTimeListSize = 50,
};
enum VCMVideoProtection {
diff --git a/modules/video_coding/video_sender.cc b/modules/video_coding/video_sender.cc
index 5b7bcda..fd35245 100644
--- a/modules/video_coding/video_sender.cc
+++ b/modules/video_coding/video_sender.cc
@@ -324,8 +324,7 @@
<< " loss rate " << encoder_params.loss_rate << " rtt "
<< encoder_params.rtt << " input frame rate "
<< encoder_params.input_frame_rate;
- post_encode_callback_->OnDroppedFrame(
- EncodedImageCallback::DropReason::kDroppedByMediaOptimizations);
+ post_encode_callback_->OnDroppedFrame();
return VCM_OK;
}
// TODO(pbos): Make sure setting send codec is synchronized with video
diff --git a/video/send_statistics_proxy.cc b/video/send_statistics_proxy.cc
index cf277e3..a175253 100644
--- a/video/send_statistics_proxy.cc
+++ b/video/send_statistics_proxy.cc
@@ -560,23 +560,6 @@
<< fec_bytes_per_sec.ToStringWithMultiplier(8);
}
}
- LOG(LS_INFO) << "Frames encoded " << current_stats.frames_encoded;
- LOG(LS_INFO) << uma_prefix_ << "DroppedFrames.Capturer "
- << current_stats.frames_dropped_by_capturer;
- RTC_HISTOGRAMS_COUNTS_1000(kIndex, uma_prefix_ + "DroppedFrames.Capturer",
- current_stats.frames_dropped_by_capturer);
- LOG(LS_INFO) << uma_prefix_ << "DroppedFrames.EncoderQueue "
- << current_stats.frames_dropped_by_encoder_queue;
- RTC_HISTOGRAMS_COUNTS_1000(kIndex, uma_prefix_ + "DroppedFrames.EncoderQueue",
- current_stats.frames_dropped_by_encoder_queue);
- LOG(LS_INFO) << uma_prefix_ << "DroppedFrames.Encoder "
- << current_stats.frames_dropped_by_encoder;
- RTC_HISTOGRAMS_COUNTS_1000(kIndex, uma_prefix_ + "DroppedFrames.Encoder",
- current_stats.frames_dropped_by_encoder);
- LOG(LS_INFO) << uma_prefix_ << "DroppedFrames.Ratelimiter "
- << current_stats.frames_dropped_by_rate_limiter;
- RTC_HISTOGRAMS_COUNTS_1000(kIndex, uma_prefix_ + "DroppedFrames.Ratelimiter",
- current_stats.frames_dropped_by_rate_limiter);
}
void SendStatisticsProxy::OnEncoderReconfigured(
@@ -882,26 +865,6 @@
}
}
-void SendStatisticsProxy::OnFrameDroppedBySource() {
- rtc::CritScope lock(&crit_);
- ++stats_.frames_dropped_by_capturer;
-}
-
-void SendStatisticsProxy::OnFrameDroppedInEncoderQueue() {
- rtc::CritScope lock(&crit_);
- ++stats_.frames_dropped_by_encoder_queue;
-}
-
-void SendStatisticsProxy::OnFrameDroppedByEncoder() {
- rtc::CritScope lock(&crit_);
- ++stats_.frames_dropped_by_encoder;
-}
-
-void SendStatisticsProxy::OnFrameDroppedByMediaOptimizations() {
- rtc::CritScope lock(&crit_);
- ++stats_.frames_dropped_by_rate_limiter;
-}
-
void SendStatisticsProxy::SetAdaptationStats(
const VideoStreamEncoder::AdaptCounts& cpu_counts,
const VideoStreamEncoder::AdaptCounts& quality_counts) {
diff --git a/video/send_statistics_proxy.h b/video/send_statistics_proxy.h
index 19c8ca7..fb133de 100644
--- a/video/send_statistics_proxy.h
+++ b/video/send_statistics_proxy.h
@@ -57,12 +57,6 @@
// Used to update incoming frame rate.
void OnIncomingFrame(int width, int height);
- // Dropped frame stats.
- void OnFrameDroppedBySource();
- void OnFrameDroppedInEncoderQueue();
- void OnFrameDroppedByEncoder();
- void OnFrameDroppedByMediaOptimizations();
-
// Adaptation stats.
void SetAdaptationStats(
const VideoStreamEncoder::AdaptCounts& cpu_counts,
diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc
index 3793737..f839665 100644
--- a/video/video_stream_encoder.cc
+++ b/video/video_stream_encoder.cc
@@ -149,7 +149,6 @@
LOG(LS_VERBOSE)
<< "Incoming frame dropped due to that the encoder is blocked.";
++video_stream_encoder_->dropped_frame_count_;
- video_stream_encoder_->stats_proxy_->OnFrameDroppedInEncoderQueue();
}
if (log_stats_) {
LOG(LS_INFO) << "Number of frames: captured "
@@ -724,10 +723,6 @@
incoming_frame, this, rtc::TimeMicros(), log_stats)));
}
-void VideoStreamEncoder::OnDiscardedFrame() {
- stats_proxy_->OnFrameDroppedBySource();
-}
-
bool VideoStreamEncoder::EncoderPaused() const {
RTC_DCHECK_RUN_ON(&encoder_queue_);
// Pause video if paused by caller or as long as the network is down or the
@@ -866,20 +861,12 @@
return result;
}
-void VideoStreamEncoder::OnDroppedFrame(DropReason reason) {
- switch (reason) {
- case DropReason::kDroppedByMediaOptimizations:
- stats_proxy_->OnFrameDroppedByMediaOptimizations();
- encoder_queue_.PostTask([this] {
- RTC_DCHECK_RUN_ON(&encoder_queue_);
- if (quality_scaler_)
- quality_scaler_->ReportDroppedFrame();
- });
- break;
- case DropReason::kDroppedByEncoder:
- stats_proxy_->OnFrameDroppedByEncoder();
- break;
- }
+void VideoStreamEncoder::OnDroppedFrame() {
+ encoder_queue_.PostTask([this] {
+ RTC_DCHECK_RUN_ON(&encoder_queue_);
+ if (quality_scaler_)
+ quality_scaler_->ReportDroppedFrame();
+ });
}
void VideoStreamEncoder::OnReceivedIntraFrameRequest(size_t stream_index) {
diff --git a/video/video_stream_encoder.h b/video/video_stream_encoder.h
index 7c6ac6c..1e00397 100644
--- a/video/video_stream_encoder.h
+++ b/video/video_stream_encoder.h
@@ -159,7 +159,6 @@
// Implements VideoSinkInterface.
void OnFrame(const VideoFrame& video_frame) override;
- void OnDiscardedFrame() override;
void EncodeVideoFrame(const VideoFrame& frame,
int64_t time_when_posted_in_ms);
@@ -170,7 +169,7 @@
const CodecSpecificInfo* codec_specific_info,
const RTPFragmentationHeader* fragmentation) override;
- void OnDroppedFrame(EncodedImageCallback::DropReason reason) override;
+ void OnDroppedFrame() override;
bool EncoderPaused() const;
void TraceFrameDropStart();