Reland of Add framerate to VideoSinkWants and ability to signal on overuse (patchset #1 id:1 of https://codereview.webrtc.org/2783183003/ )
Reason for revert:
Seem to be a flaky test rather than an issue with this cl. Creating reland, will add code to reduce flakiness to that test.
Original issue's description:
> Revert of Add framerate to VideoSinkWants and ability to signal on overuse (patchset #8 id:410001 of https://codereview.webrtc.org/2781433002/ )
>
> Reason for revert:
> This has resulted in failure of CallPerfTest.ReceivesCpuOveruseAndUnderuse test on the Win7 build bot https://build.chromium.org/p/client.webrtc.perf/builders/Win7/builds/1780
>
> Original issue's description:
> > Reland of Add framerate to VideoSinkWants and ability to signal on overuse (patchset #1 id:1 of https://codereview.webrtc.org/2764133002/ )
> >
> > Reason for revert:
> > Found issue with test case, will add fix to reland cl.
> >
> > Original issue's description:
> > > Revert of Add framerate to VideoSinkWants and ability to signal on overuse (patchset #14 id:250001 of https://codereview.webrtc.org/2716643002/ )
> > >
> > > Reason for revert:
> > > Breaks perf tests:
> > > https://build.chromium.org/p/client.webrtc.perf/builders/Win7/builds/1679
> > > https://build.chromium.org/p/client.webrtc.perf/builders/Android32%20Tests%20%28L%20Nexus5%29/builds/2325
> > >
> > > Original issue's description:
> > > > Add framerate to VideoSinkWants and ability to signal on overuse
> > > >
> > > > In ViEEncoder, try to reduce framerate instead of resolution if the
> > > > current degradation preference is maintain-resolution rather than
> > > > balanced.
> > > >
> > > > BUG=webrtc:4172
> > > >
> > > > Review-Url: https://codereview.webrtc.org/2716643002
> > > > Cr-Commit-Position: refs/heads/master@{#17327}
> > > > Committed: https://chromium.googlesource.com/external/webrtc/+/72acf2526177bb4dbb5103cd6e165eb4361a5ae6
> > >
> > > TBR=nisse@webrtc.org,magjed@webrtc.org,kthelgason@webrtc.org,ilnik@webrtc.org,stefan@webrtc.org,sprang@webrtc.org
> > > # Skipping CQ checks because original CL landed less than 1 days ago.
> > > NOPRESUBMIT=true
> > > NOTREECHECKS=true
> > > NOTRY=true
> > > BUG=webrtc:4172
> > >
> > > Review-Url: https://codereview.webrtc.org/2764133002
> > > Cr-Commit-Position: refs/heads/master@{#17331}
> > > Committed: https://chromium.googlesource.com/external/webrtc/+/8b45b11144c968b4173215c76f78c710c9a2ed0b
> >
> > TBR=nisse@webrtc.org,magjed@webrtc.org,kthelgason@webrtc.org,ilnik@webrtc.org,stefan@webrtc.org,skvlad@webrtc.org
> > # Not skipping CQ checks because original CL landed more than 1 days ago.
> > BUG=webrtc:4172
> >
> > Review-Url: https://codereview.webrtc.org/2781433002
> > Cr-Commit-Position: refs/heads/master@{#17474}
> > Committed: https://chromium.googlesource.com/external/webrtc/+/3ea3c77e93121b1ab9d5e46641e6764f2cca0d51
>
> TBR=ilnik@webrtc.org,stefan@webrtc.org,asapersson@webrtc.org,sprang@webrtc.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=webrtc:4172
>
> Review-Url: https://codereview.webrtc.org/2783183003
> Cr-Commit-Position: refs/heads/master@{#17477}
> Committed: https://chromium.googlesource.com/external/webrtc/+/f9ed235c9b7248694edcb46feb1f29ce7456ab59
R=ilnik@webrtc.org,stefan@webrtc.org
BUG=webrtc:4172
Review-Url: https://codereview.webrtc.org/2789823002
Cr-Commit-Position: refs/heads/master@{#17498}
diff --git a/webrtc/test/BUILD.gn b/webrtc/test/BUILD.gn
index 4a49ef7..a159567 100644
--- a/webrtc/test/BUILD.gn
+++ b/webrtc/test/BUILD.gn
@@ -43,6 +43,7 @@
"frame_utils.h",
"vcm_capturer.cc",
"vcm_capturer.h",
+ "video_capturer.cc",
"video_capturer.h",
]
@@ -53,6 +54,7 @@
deps = [
"../common_video",
+ "../media:rtc_media_base",
"../modules/video_capture:video_capture_module",
]
}
diff --git a/webrtc/test/call_test.cc b/webrtc/test/call_test.cc
index d8297d5..023ff1a 100644
--- a/webrtc/test/call_test.cc
+++ b/webrtc/test/call_test.cc
@@ -326,7 +326,7 @@
width, height, framerate * speed, clock));
video_send_stream_->SetSource(
frame_generator_capturer_.get(),
- VideoSendStream::DegradationPreference::kBalanced);
+ VideoSendStream::DegradationPreference::kMaintainFramerate);
}
void CallTest::CreateFrameGeneratorCapturer(int framerate,
@@ -336,7 +336,7 @@
test::FrameGeneratorCapturer::Create(width, height, framerate, clock_));
video_send_stream_->SetSource(
frame_generator_capturer_.get(),
- VideoSendStream::DegradationPreference::kBalanced);
+ VideoSendStream::DegradationPreference::kMaintainFramerate);
}
void CallTest::CreateFakeAudioDevices(
diff --git a/webrtc/test/frame_generator_capturer.cc b/webrtc/test/frame_generator_capturer.cc
index 9e74e40..d16d8c8 100644
--- a/webrtc/test/frame_generator_capturer.cc
+++ b/webrtc/test/frame_generator_capturer.cc
@@ -37,30 +37,47 @@
private:
bool Run() override {
+ bool task_completed = true;
if (repeat_interval_ms_ > 0) {
- int64_t delay_ms;
- int64_t time_now_ms = rtc::TimeMillis();
- if (intended_run_time_ms_ > 0) {
- delay_ms = time_now_ms - intended_run_time_ms_;
- } else {
- delay_ms = 0;
- intended_run_time_ms_ = time_now_ms;
- }
- intended_run_time_ms_ += repeat_interval_ms_;
- if (delay_ms < repeat_interval_ms_) {
+ // This is not a one-off frame. Check if the frame interval for this
+ // task queue is the same same as the current configured frame rate.
+ uint32_t current_interval_ms =
+ 1000 / frame_generator_capturer_->GetCurrentConfiguredFramerate();
+ if (repeat_interval_ms_ != current_interval_ms) {
+ // Frame rate has changed since task was started, create a new instance.
rtc::TaskQueue::Current()->PostDelayedTask(
- std::unique_ptr<rtc::QueuedTask>(this),
- repeat_interval_ms_ - delay_ms);
+ std::unique_ptr<rtc::QueuedTask>(new InsertFrameTask(
+ frame_generator_capturer_, current_interval_ms)),
+ current_interval_ms);
} else {
- rtc::TaskQueue::Current()->PostDelayedTask(
- std::unique_ptr<rtc::QueuedTask>(this), 0);
- LOG(LS_ERROR)
- << "Frame Generator Capturer can't keep up with requested fps";
+ // Schedule the next frame capture event to happen at approximately the
+ // correct absolute time point.
+ int64_t delay_ms;
+ int64_t time_now_ms = rtc::TimeMillis();
+ if (intended_run_time_ms_ > 0) {
+ delay_ms = time_now_ms - intended_run_time_ms_;
+ } else {
+ delay_ms = 0;
+ intended_run_time_ms_ = time_now_ms;
+ }
+ intended_run_time_ms_ += repeat_interval_ms_;
+ if (delay_ms < repeat_interval_ms_) {
+ rtc::TaskQueue::Current()->PostDelayedTask(
+ std::unique_ptr<rtc::QueuedTask>(this),
+ repeat_interval_ms_ - delay_ms);
+ } else {
+ rtc::TaskQueue::Current()->PostDelayedTask(
+ std::unique_ptr<rtc::QueuedTask>(this), 0);
+ LOG(LS_ERROR)
+ << "Frame Generator Capturer can't keep up with requested fps";
+ }
+ // Repost of this instance, make sure it is not deleted.
+ task_completed = false;
}
}
frame_generator_capturer_->InsertFrame();
// Task should be deleted only if it's not repeating.
- return repeat_interval_ms_ == 0;
+ return task_completed;
}
webrtc::test::FrameGeneratorCapturer* const frame_generator_capturer_;
@@ -72,14 +89,12 @@
int height,
int target_fps,
Clock* clock) {
- FrameGeneratorCapturer* capturer = new FrameGeneratorCapturer(
- clock, FrameGenerator::CreateSquareGenerator(width, height), target_fps);
- if (!capturer->Init()) {
- delete capturer;
- return NULL;
- }
+ std::unique_ptr<FrameGeneratorCapturer> capturer(new FrameGeneratorCapturer(
+ clock, FrameGenerator::CreateSquareGenerator(width, height), target_fps));
+ if (!capturer->Init())
+ return nullptr;
- return capturer;
+ return capturer.release();
}
FrameGeneratorCapturer* FrameGeneratorCapturer::CreateFromYuvFile(
@@ -88,16 +103,15 @@
size_t height,
int target_fps,
Clock* clock) {
- FrameGeneratorCapturer* capturer = new FrameGeneratorCapturer(
- clock, FrameGenerator::CreateFromYuvFile(
- std::vector<std::string>(1, file_name), width, height, 1),
- target_fps);
- if (!capturer->Init()) {
- delete capturer;
- return NULL;
- }
+ std::unique_ptr<FrameGeneratorCapturer> capturer(new FrameGeneratorCapturer(
+ clock,
+ FrameGenerator::CreateFromYuvFile(std::vector<std::string>(1, file_name),
+ width, height, 1),
+ target_fps));
+ if (!capturer->Init())
+ return nullptr;
- return capturer;
+ return capturer.release();
}
FrameGeneratorCapturer::FrameGeneratorCapturer(
@@ -129,29 +143,32 @@
bool FrameGeneratorCapturer::Init() {
// This check is added because frame_generator_ might be file based and should
// not crash because a file moved.
- if (frame_generator_.get() == NULL)
+ if (frame_generator_.get() == nullptr)
return false;
+ int framerate_fps = GetCurrentConfiguredFramerate();
task_queue_.PostDelayedTask(
std::unique_ptr<rtc::QueuedTask>(
- new InsertFrameTask(this, 1000 / target_fps_)),
- 1000 / target_fps_);
+ new InsertFrameTask(this, 1000 / framerate_fps)),
+ 1000 / framerate_fps);
return true;
}
void FrameGeneratorCapturer::InsertFrame() {
- {
- rtc::CritScope cs(&lock_);
- if (sending_) {
- VideoFrame* frame = frame_generator_->NextFrame();
- frame->set_ntp_time_ms(clock_->CurrentNtpInMilliseconds());
- frame->set_rotation(fake_rotation_);
- if (first_frame_capture_time_ == -1) {
- first_frame_capture_time_ = frame->ntp_time_ms();
- }
- if (sink_)
- sink_->OnFrame(*frame);
+ rtc::CritScope cs(&lock_);
+ if (sending_) {
+ VideoFrame* frame = frame_generator_->NextFrame();
+ frame->set_ntp_time_ms(clock_->CurrentNtpInMilliseconds());
+ frame->set_rotation(fake_rotation_);
+ if (first_frame_capture_time_ == -1) {
+ first_frame_capture_time_ = frame->ntp_time_ms();
+ }
+
+ if (sink_) {
+ rtc::Optional<VideoFrame> out_frame = AdaptFrame(*frame);
+ if (out_frame)
+ sink_->OnFrame(*out_frame);
}
}
}
@@ -185,6 +202,19 @@
sink_ = sink;
if (sink_wants_observer_)
sink_wants_observer_->OnSinkWantsChanged(sink, wants);
+
+ // Handle framerate within this class, just pass on resolution for possible
+ // adaptation.
+ rtc::VideoSinkWants resolution_wants = wants;
+ resolution_wants.max_framerate_fps = std::numeric_limits<int>::max();
+ VideoCapturer::AddOrUpdateSink(sink, resolution_wants);
+
+ // Ignore any requests for framerate higher than initially configured.
+ if (wants.max_framerate_fps < target_fps_) {
+ wanted_fps_.emplace(wants.max_framerate_fps);
+ } else {
+ wanted_fps_.reset();
+ }
}
void FrameGeneratorCapturer::RemoveSink(
@@ -201,5 +231,12 @@
std::unique_ptr<rtc::QueuedTask>(new InsertFrameTask(this, 0)));
}
+int FrameGeneratorCapturer::GetCurrentConfiguredFramerate() {
+ rtc::CritScope cs(&lock_);
+ if (wanted_fps_ && *wanted_fps_ < target_fps_)
+ return *wanted_fps_;
+ return target_fps_;
+}
+
} // namespace test
} // namespace webrtc
diff --git a/webrtc/test/frame_generator_capturer.h b/webrtc/test/frame_generator_capturer.h
index 3b2355f..36a79f9 100644
--- a/webrtc/test/frame_generator_capturer.h
+++ b/webrtc/test/frame_generator_capturer.h
@@ -77,6 +77,7 @@
void InsertFrame();
static bool Run(void* obj);
+ int GetCurrentConfiguredFramerate();
Clock* const clock_;
bool sending_;
@@ -86,7 +87,8 @@
rtc::CriticalSection lock_;
std::unique_ptr<FrameGenerator> frame_generator_;
- int target_fps_;
+ int target_fps_ GUARDED_BY(&lock_);
+ rtc::Optional<int> wanted_fps_ GUARDED_BY(&lock_);
VideoRotation fake_rotation_ = kVideoRotation_0;
int64_t first_frame_capture_time_;
diff --git a/webrtc/test/vcm_capturer.cc b/webrtc/test/vcm_capturer.cc
index 535e9bf..d66cf23 100644
--- a/webrtc/test/vcm_capturer.cc
+++ b/webrtc/test/vcm_capturer.cc
@@ -10,17 +10,18 @@
#include "webrtc/test/vcm_capturer.h"
+#include "webrtc/base/logging.h"
#include "webrtc/modules/video_capture/video_capture_factory.h"
#include "webrtc/video_send_stream.h"
namespace webrtc {
namespace test {
-VcmCapturer::VcmCapturer() : started_(false), sink_(nullptr), vcm_(NULL) {}
+VcmCapturer::VcmCapturer() : started_(false), sink_(nullptr), vcm_(nullptr) {}
bool VcmCapturer::Init(size_t width, size_t height, size_t target_fps) {
- VideoCaptureModule::DeviceInfo* device_info =
- VideoCaptureFactory::CreateDeviceInfo();
+ std::unique_ptr<VideoCaptureModule::DeviceInfo> device_info(
+ VideoCaptureFactory::CreateDeviceInfo());
char device_name[256];
char unique_name[256];
@@ -35,7 +36,6 @@
vcm_->RegisterCaptureDataCallback(this);
device_info->GetCapability(vcm_->CurrentDeviceName(), 0, capability_);
- delete device_info;
capability_.width = static_cast<int32_t>(width);
capability_.height = static_cast<int32_t>(height);
@@ -47,7 +47,7 @@
return false;
}
- assert(vcm_->CaptureStarted());
+ RTC_CHECK(vcm_->CaptureStarted());
return true;
}
@@ -55,13 +55,13 @@
VcmCapturer* VcmCapturer::Create(size_t width,
size_t height,
size_t target_fps) {
- VcmCapturer* vcm_capturer = new VcmCapturer();
+ std::unique_ptr<VcmCapturer> vcm_capturer(new VcmCapturer());
if (!vcm_capturer->Init(width, height, target_fps)) {
- // TODO(pbos): Log a warning that this failed.
- delete vcm_capturer;
- return NULL;
+ LOG(LS_WARNING) << "Failed to create VcmCapturer(w = " << width
+ << ", h = " << height << ", fps = " << target_fps << ")";
+ return nullptr;
}
- return vcm_capturer;
+ return vcm_capturer.release();
}
@@ -80,6 +80,7 @@
rtc::CritScope lock(&crit_);
RTC_CHECK(!sink_ || sink_ == sink);
sink_ = sink;
+ VideoCapturer::AddOrUpdateSink(sink, wants);
}
void VcmCapturer::RemoveSink(rtc::VideoSinkInterface<VideoFrame>* sink) {
@@ -102,8 +103,11 @@
void VcmCapturer::OnFrame(const VideoFrame& frame) {
rtc::CritScope lock(&crit_);
- if (started_ && sink_)
- sink_->OnFrame(frame);
+ if (started_ && sink_) {
+ rtc::Optional<VideoFrame> out_frame = AdaptFrame(frame);
+ if (out_frame)
+ sink_->OnFrame(*out_frame);
+ }
}
} // test
diff --git a/webrtc/test/vcm_capturer.h b/webrtc/test/vcm_capturer.h
index c4dae65..5e40c2b 100644
--- a/webrtc/test/vcm_capturer.h
+++ b/webrtc/test/vcm_capturer.h
@@ -10,6 +10,8 @@
#ifndef WEBRTC_TEST_VCM_CAPTURER_H_
#define WEBRTC_TEST_VCM_CAPTURER_H_
+#include <memory>
+
#include "webrtc/base/criticalsection.h"
#include "webrtc/base/scoped_ref_ptr.h"
#include "webrtc/common_types.h"
diff --git a/webrtc/test/video_capturer.cc b/webrtc/test/video_capturer.cc
new file mode 100644
index 0000000..c8b3826
--- /dev/null
+++ b/webrtc/test/video_capturer.cc
@@ -0,0 +1,58 @@
+/*
+ * Copyright (c) 2017 The WebRTC project authors. All Rights Reserved.
+ *
+ * Use of this source code is governed by a BSD-style license
+ * that can be found in the LICENSE file in the root of the source
+ * tree. An additional intellectual property rights grant can be found
+ * in the file PATENTS. All contributing project authors may
+ * be found in the AUTHORS file in the root of the source tree.
+ */
+
+#include "webrtc/test/video_capturer.h"
+
+#include "webrtc/base/basictypes.h"
+#include "webrtc/base/constructormagic.h"
+
+namespace webrtc {
+namespace test {
+VideoCapturer::VideoCapturer() : video_adapter_(new cricket::VideoAdapter()) {}
+VideoCapturer::~VideoCapturer() {}
+
+rtc::Optional<VideoFrame> VideoCapturer::AdaptFrame(const VideoFrame& frame) {
+ int cropped_width = 0;
+ int cropped_height = 0;
+ int out_width = 0;
+ int out_height = 0;
+
+ if (!video_adapter_->AdaptFrameResolution(
+ frame.width(), frame.height(), frame.timestamp_us() * 1000,
+ &cropped_width, &cropped_height, &out_width, &out_height)) {
+ // Drop frame in order to respect frame rate constraint.
+ return rtc::Optional<VideoFrame>();
+ }
+
+ rtc::Optional<VideoFrame> out_frame;
+ if (out_height != frame.height() || out_width != frame.width()) {
+ // Video adapter has requested a down-scale. Allocate a new buffer and
+ // return scaled version.
+ rtc::scoped_refptr<I420Buffer> scaled_buffer =
+ I420Buffer::Create(out_width, out_height);
+ scaled_buffer->ScaleFrom(*frame.video_frame_buffer().get());
+ out_frame.emplace(
+ VideoFrame(scaled_buffer, kVideoRotation_0, frame.timestamp_us()));
+ } else {
+ // No adaptations needed, just return the frame as is.
+ out_frame.emplace(frame);
+ }
+
+ return out_frame;
+}
+
+void VideoCapturer::AddOrUpdateSink(rtc::VideoSinkInterface<VideoFrame>* sink,
+ const rtc::VideoSinkWants& wants) {
+ video_adapter_->OnResolutionFramerateRequest(
+ wants.target_pixel_count, wants.max_pixel_count, wants.max_framerate_fps);
+}
+
+} // namespace test
+} // namespace webrtc
diff --git a/webrtc/test/video_capturer.h b/webrtc/test/video_capturer.h
index 111f986..667d89c 100644
--- a/webrtc/test/video_capturer.h
+++ b/webrtc/test/video_capturer.h
@@ -12,23 +12,42 @@
#include <stddef.h>
+#include <memory>
+
+#include "webrtc/api/video/i420_buffer.h"
#include "webrtc/api/video/video_frame.h"
+#include "webrtc/base/criticalsection.h"
+#include "webrtc/base/optional.h"
+#include "webrtc/media/base/videoadapter.h"
#include "webrtc/media/base/videosourceinterface.h"
+namespace cricket {
+class VideoAdapter;
+} // namespace cricket
+
namespace webrtc {
-
class Clock;
-
namespace test {
class VideoCapturer : public rtc::VideoSourceInterface<VideoFrame> {
public:
- virtual ~VideoCapturer() {}
+ VideoCapturer();
+ virtual ~VideoCapturer();
virtual void Start() = 0;
virtual void Stop() = 0;
+
+ void AddOrUpdateSink(rtc::VideoSinkInterface<VideoFrame>* sink,
+ const rtc::VideoSinkWants& wants) override;
+
+ protected:
+ rtc::Optional<VideoFrame> AdaptFrame(const VideoFrame& frame);
+ rtc::VideoSinkWants GetSinkWants();
+
+ private:
+ const std::unique_ptr<cricket::VideoAdapter> video_adapter_;
};
-} // test
-} // webrtc
+} // namespace test
+} // namespace webrtc
#endif // WEBRTC_TEST_VIDEO_CAPTURER_H_