From bcfd8a99f9f695e0028d9daabfbc249acbe59d86 Mon Sep 17 00:00:00 2001 From: esum Date: Mon, 14 Dec 2015 11:14:12 -0800 Subject: [Chromecast] Moving VideoPlane caching/logic in VideoPlaneController. The VideoPlane parameters were getting cached in both OverlayCandidatesCast and VideoPlaneController. There was also some VideoPlane logic in OverlayCandidatesCast. This has all been moved to VideoPlaneController. Also did a little clean-up. BUG=internal b/25812819 TEST=Youtube/Netflix playback with debug logging on Earth. ozone_unittests (clang) cast_media_unittests (Earth) Review URL: https://codereview.chromium.org/1518203002 Cr-Commit-Position: refs/heads/master@{#365057} --- chromecast/browser/cast_content_window.cc | 2 +- chromecast/media/base/video_plane_controller.cc | 125 ++++++++++++++++++------ chromecast/media/base/video_plane_controller.h | 47 ++++++--- 3 files changed, 131 insertions(+), 43 deletions(-) (limited to 'chromecast') diff --git a/chromecast/browser/cast_content_window.cc b/chromecast/browser/cast_content_window.cc index 1cdb8cf..804ed51 100644 --- a/chromecast/browser/cast_content_window.cc +++ b/chromecast/browser/cast_content_window.cc @@ -78,7 +78,7 @@ void CastContentWindow::CreateWindowTree( gfx::Screen::SetScreenInstance(gfx::SCREEN_TYPE_NATIVE, cast_screen); if (cast_screen->GetPrimaryDisplay().size() != initial_size) cast_screen->UpdateDisplaySize(initial_size); - media::VideoPlaneController::GetInstance()->OnGraphicsPlaneResolutionChanged( + media::VideoPlaneController::GetInstance()->SetGraphicsPlaneResolution( Size(initial_size.width(), initial_size.height())); CHECK(aura::Env::GetInstance()); diff --git a/chromecast/media/base/video_plane_controller.cc b/chromecast/media/base/video_plane_controller.cc index 8bae226..8f07839 100644 --- a/chromecast/media/base/video_plane_controller.cc +++ b/chromecast/media/base/video_plane_controller.cc @@ -16,6 +16,27 @@ namespace chromecast { namespace media { +namespace { + +bool RectFEqual(const RectF& r1, const RectF& r2) { + return r1.x == r2.x && r1.y == r2.y && r1.width == r2.width && + r1.height == r2.height; +} + +bool SizeEqual(const Size& s1, const Size& s2) { + return s1.width == s2.width && s1.height == s2.height; +} + +bool DisplayRectFValid(const RectF& r) { + return r.width >= 0 && r.height >= 0; +} + +bool ResolutionSizeValid(const Size& s) { + return s.width >= 0 && s.height >= 0; +} + +} // namespace + // Helper class for calling VideoPlane::SetGeometry with rate-limiting. // SetGeometry can take on the order of 100ms to run in some implementations // and can be called on the order of 20x / second (as fast as graphics frames @@ -38,6 +59,7 @@ class VideoPlaneController::RateLimitedSetVideoPlaneGeometry void SetGeometry(const chromecast::RectF& display_rect, VideoPlane::Transform transform) { DCHECK(task_runner_->BelongsToCurrentThread()); + DCHECK(DisplayRectFValid(display_rect)); base::TimeTicks now = base::TimeTicks::Now(); base::TimeDelta elapsed = now - last_set_geometry_time_; @@ -119,66 +141,109 @@ VideoPlaneController* VideoPlaneController::GetInstance() { return base::Singleton::get(); } +// TODO(esum): SetGeometry, SetDeviceResolution, and SetGraphicsPlaneResolution +// follow the same pattern (copy/paste). Currently it's not worth modularizing +// since there are only 3 fields. If more fields are needed in the future, +// consider making a generic method to implement this pattern. void VideoPlaneController::SetGeometry(const RectF& display_rect, VideoPlane::Transform transform) { - DCHECK(graphics_res_.width != 0 && graphics_res_.height != 0); - - RectF scaled_rect = display_rect; - if (graphics_res_.width != output_res_.width || - graphics_res_.height != output_res_.height) { - float sx = static_cast(output_res_.width) / graphics_res_.width; - float sy = static_cast(output_res_.height) / graphics_res_.height; - scaled_rect.x *= sx; - scaled_rect.y *= sy; - scaled_rect.width *= sx; - scaled_rect.height *= sy; + DCHECK(thread_checker_.CalledOnValidThread()); + DCHECK(DisplayRectFValid(display_rect)); + if (have_video_plane_geometry_ && + RectFEqual(display_rect, video_plane_display_rect_) && + transform == video_plane_transform_) { + VLOG(2) << "No change found in geometry parameters."; + return; } - media_task_runner_->PostTask( - FROM_HERE, base::Bind(&RateLimitedSetVideoPlaneGeometry::SetGeometry, - video_plane_wrapper_, scaled_rect, transform)); + VLOG(1) << "New geometry parameters " + << " rect=" << display_rect.width << "x" << display_rect.height + << " @" << display_rect.x << "," << display_rect.y << " transform " + << transform; have_video_plane_geometry_ = true; video_plane_display_rect_ = display_rect; video_plane_transform_ = transform; + + MaybeRunSetGeometry(); } -void VideoPlaneController::OnDeviceResolutionChanged(const Size& resolution) { - if (output_res_.width == resolution.width && - output_res_.height == resolution.height) +void VideoPlaneController::SetDeviceResolution(const Size& resolution) { + DCHECK(thread_checker_.CalledOnValidThread()); + DCHECK(ResolutionSizeValid(resolution)); + if (have_output_res_ && SizeEqual(resolution, output_res_)) { + VLOG(2) << "No change found in output resolution."; return; + } + VLOG(1) << "New output resolution " << resolution.width << "x" + << resolution.height; + + have_output_res_ = true; output_res_ = resolution; - if (have_video_plane_geometry_) { - SetGeometry(video_plane_display_rect_, video_plane_transform_); - } + MaybeRunSetGeometry(); } -void VideoPlaneController::OnGraphicsPlaneResolutionChanged( - const Size& resolution) { - if (graphics_res_.width == resolution.width && - graphics_res_.height == resolution.height) +void VideoPlaneController::SetGraphicsPlaneResolution(const Size& resolution) { + DCHECK(thread_checker_.CalledOnValidThread()); + DCHECK(ResolutionSizeValid(resolution)); + if (have_graphics_res_ && SizeEqual(resolution, graphics_res_)) { + VLOG(2) << "No change found in graphics resolution."; return; + } + VLOG(1) << "New graphics resolution " << resolution.width << "x" + << resolution.height; + + have_graphics_res_ = true; graphics_res_ = resolution; - if (have_video_plane_geometry_) { - SetGeometry(video_plane_display_rect_, video_plane_transform_); - } + MaybeRunSetGeometry(); } VideoPlaneController::VideoPlaneController() - : output_res_(0, 0), + : have_output_res_(false), + have_graphics_res_(false), + output_res_(0, 0), graphics_res_(0, 0), have_video_plane_geometry_(false), video_plane_display_rect_(0, 0), - video_plane_transform_(media::VideoPlane::TRANSFORM_NONE), - media_task_runner_(chromecast::media::MediaMessageLoop::GetTaskRunner()), + video_plane_transform_(VideoPlane::TRANSFORM_NONE), + media_task_runner_(MediaMessageLoop::GetTaskRunner()), video_plane_wrapper_( new RateLimitedSetVideoPlaneGeometry(media_task_runner_)) {} VideoPlaneController::~VideoPlaneController() {} +void VideoPlaneController::MaybeRunSetGeometry() { + DCHECK(thread_checker_.CalledOnValidThread()); + if (!HaveDataForSetGeometry()) + return; + + DCHECK(graphics_res_.width != 0 && graphics_res_.height != 0); + + RectF scaled_rect = video_plane_display_rect_; + if (graphics_res_.width != output_res_.width || + graphics_res_.height != output_res_.height) { + float sx = static_cast(output_res_.width) / graphics_res_.width; + float sy = static_cast(output_res_.height) / graphics_res_.height; + scaled_rect.x *= sx; + scaled_rect.y *= sy; + scaled_rect.width *= sx; + scaled_rect.height *= sy; + } + + media_task_runner_->PostTask( + FROM_HERE, + base::Bind(&RateLimitedSetVideoPlaneGeometry::SetGeometry, + video_plane_wrapper_, scaled_rect, video_plane_transform_)); +} + +bool VideoPlaneController::HaveDataForSetGeometry() const { + DCHECK(thread_checker_.CalledOnValidThread()); + return have_output_res_ && have_graphics_res_ && have_video_plane_geometry_; +} + } // namespace media } // namespace chromecast diff --git a/chromecast/media/base/video_plane_controller.h b/chromecast/media/base/video_plane_controller.h index c6ac581..302c8fa 100644 --- a/chromecast/media/base/video_plane_controller.h +++ b/chromecast/media/base/video_plane_controller.h @@ -7,6 +7,7 @@ #include "base/memory/ref_counted.h" #include "base/memory/singleton.h" +#include "base/threading/thread_checker.h" #include "chromecast/public/graphics_types.h" #include "chromecast/public/video_plane.h" @@ -26,7 +27,12 @@ namespace media { // * coalesces multiple calls in short space of time to prevent flooding the // media thread with SetGeometry calls (which are expensive on many // platforms). -// up to be informed of certain resolution changes. +// The class collects/caches the data it needs before it can start operating. +// This means SetGeometry, SetDeviceResolution, and SetGraphicsPlaneResolution +// need to be called at least once in any order before the class starts making +// calls to VideoPlane::SetGeometry. All calls to these methods beforehand just +// set/update the cached parameters. +// All calls to public methods should be from the same thread. class VideoPlaneController { public: static VideoPlaneController* GetInstance(); @@ -35,17 +41,22 @@ class VideoPlaneController { // in *graphics plane coordinates*. // * This should be called on UI thread (hopping to media thread is handled // internally). - void SetGeometry(const RectF& display_rect, - media::VideoPlane::Transform transform); - - // Handles a change in physical screen resolution. This must be called when - // the final output resolution (HDMI signal or panel resolution) changes. - void OnDeviceResolutionChanged(const Size& resolution); - - // Handles a change in graphics hardware plane resolution. This must be - // called when the hardware graphics plane resolution changes (same resolution - // as gfx::Screen). - void OnGraphicsPlaneResolutionChanged(const Size& resolution); + // If there is no change to video plane parameters from the last call to this + // method, it is a no-op. + void SetGeometry(const RectF& display_rect, VideoPlane::Transform transform); + + // Sets physical screen resolution. This must be called at least once when + // the final output resolution (HDMI signal or panel resolution) is known, + // then later when it changes. If there is no change to the device resolution + // from the last call to this method, it is a no-op. + void SetDeviceResolution(const Size& resolution); + + // Sets graphics hardware plane resolution. This must be called at least once + // when the hardware graphics plane resolution (same resolution as + // gfx::Screen) is known, then later when it changes. If there is no change to + // the graphics plane resolution from the last call to this method, it is a + // no-op. + void SetGraphicsPlaneResolution(const Size& resolution); private: class RateLimitedSetVideoPlaneGeometry; @@ -54,7 +65,17 @@ class VideoPlaneController { VideoPlaneController(); ~VideoPlaneController(); + // Check if HaveDataForSetGeometry. If not, this method is a no-op. Otherwise + // it scales the display rect from graphics to device resolution coordinates. + // Then posts task to media thread for VideoPlane::SetGeometry. + void MaybeRunSetGeometry(); + // Checks if all data has been collected to make calls to + // VideoPlane::SetGeometry. + bool HaveDataForSetGeometry() const; + // Current resolutions + bool have_output_res_; + bool have_graphics_res_; Size output_res_; Size graphics_res_; @@ -67,6 +88,8 @@ class VideoPlaneController { scoped_refptr media_task_runner_; scoped_refptr video_plane_wrapper_; + base::ThreadChecker thread_checker_; + DISALLOW_COPY_AND_ASSIGN(VideoPlaneController); }; -- cgit v1.1