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/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