Skip to content

GitLab

T
tangled
Commit f6b06f4d authored by Caleb Rouleau's avatar Caleb Rouleau Committed by Commit Bot
Browse files
Options

Revert "Stop using GpuMemoryBufferVideoFramePool when WebMediaPlayerMS is hidden" 

This reverts commit 1f31a184.

Reason for revert: "WebMediaPlayerMSTest.StopsCreatingHardwareFramesWhenHiddenOrClosed" is flaky. https://bugs.chromium.org/p/chromium/issues/detail?id=821839

Original change's description:
> Stop using GpuMemoryBufferVideoFramePool when WebMediaPlayerMS is hidden
> 
> This CL adds calls to track when WebMediaPlayerMS is hidden or shown, so
> that in the time period we can skip creating GMB backed frames. For that
> time period, frames aren't going to be displayed, so copying them to GMBs
> is extra work which should be avoided.
> 
> Bug: 653200
> Change-Id: I67e55c7f1150b434d82321ac90a08c7c3e3e6336
> Reviewed-on: https://chromium-review.googlesource.com/954339
> Reviewed-by: Daniele Castagna <dcastagna@chromium.org>
> Reviewed-by: Dan Sanders <sandersd@chromium.org>
> Commit-Queue: Emircan Uysaler <emircan@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#542950}

TBR=sandersd@chromium.org,dcastagna@chromium.org,emircan@chromium.org

Change-Id: Ib88d6f0419271bf7d5e65b1b017eef1e12aeecbb
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 653200
Reviewed-on: https://chromium-review.googlesource.com/962824Reviewed-by: default avatarCaleb Rouleau <crouleau@chromium.org>
Commit-Queue: Caleb Rouleau <crouleau@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543118}
parent 178a7ae0
Hide whitespace changes
Inline Side-by-side
Showing with 18 additions and 94 deletions
content/renderer/media/stream/webmediaplayer_ms.cc
View file @ f6b06f4d
......@@ -72,7 +72,6 @@ class WebMediaPlayerMS::FrameDeliverer {
player_(player),
enqueue_frame_cb_(enqueue_frame_cb),
media_task_runner_(media_task_runner),
weak_factory_for_pool_(this),
weak_factory_(this) {
io_thread_checker_.DetachFromThread();
......@@ -88,7 +87,6 @@ class WebMediaPlayerMS::FrameDeliverer {
~FrameDeliverer() {
DCHECK(io_thread_checker_.CalledOnValidThread());
if (gpu_memory_buffer_pool_) {
gpu_memory_buffer_pool_->Abort();
media_task_runner_->DeleteSoon(FROM_HERE,
gpu_memory_buffer_pool_.release());
}
......@@ -97,7 +95,6 @@ class WebMediaPlayerMS::FrameDeliverer {
void OnVideoFrame(scoped_refptr<media::VideoFrame> frame) {
DCHECK(io_thread_checker_.CalledOnValidThread());
// On Android, stop passing frames.
#if defined(OS_ANDROID)
if (render_frame_suspended_)
return;
......@@ -108,20 +105,6 @@ class WebMediaPlayerMS::FrameDeliverer {
return;
}
// If |render_frame_suspended_|, we can keep passing the frames to keep the
// latest frame in compositor up to date. However, creating GMB backed
// frames is unnecessary, because the frames are not going to be shown for
// the time period.
if (render_frame_suspended_) {
FrameReady(frame);
// If there are any existing MaybeCreateHardwareFrame() calls, we do not
// want those frames to be placed after the current one, so just drop
// them.
gpu_memory_buffer_pool_->Abort();
weak_factory_for_pool_.InvalidateWeakPtrs();
return;
}
// |gpu_memory_buffer_pool_| deletion is going to be posted to
// |media_task_runner_|. base::Unretained() usage is fine since
// |gpu_memory_buffer_pool_| outlives the task.
......@@ -130,9 +113,8 @@ class WebMediaPlayerMS::FrameDeliverer {
base::BindOnce(
&media::GpuMemoryBufferVideoFramePool::MaybeCreateHardwareFrame,
base::Unretained(gpu_memory_buffer_pool_.get()), frame,
media::BindToCurrentLoop(
base::BindOnce(&FrameDeliverer::FrameReady,
weak_factory_for_pool_.GetWeakPtr()))));
media::BindToCurrentLoop(base::BindRepeating(
&FrameDeliverer::FrameReady, weak_factory_.GetWeakPtr()))));
}
void FrameReady(const scoped_refptr<media::VideoFrame>& frame) {
......@@ -177,10 +159,12 @@ class WebMediaPlayerMS::FrameDeliverer {
enqueue_frame_cb_.Run(frame);
}
#if defined(OS_ANDROID)
void SetRenderFrameSuspended(bool render_frame_suspended) {
DCHECK(io_thread_checker_.CalledOnValidThread());
render_frame_suspended_ = render_frame_suspended;
}
#endif // defined(OS_ANDROID)
MediaStreamVideoRenderer::RepaintCB GetRepaintCallback() {
return base::Bind(&FrameDeliverer::OnVideoFrame,
......@@ -193,7 +177,10 @@ class WebMediaPlayerMS::FrameDeliverer {
bool last_frame_opaque_;
media::VideoRotation last_frame_rotation_;
bool received_first_frame_;
#if defined(OS_ANDROID)
bool render_frame_suspended_ = false;
#endif // defined(OS_ANDROID)
const scoped_refptr<base::SingleThreadTaskRunner> main_task_runner_;
const base::WeakPtr<WebMediaPlayerMS> player_;
......@@ -206,7 +193,6 @@ class WebMediaPlayerMS::FrameDeliverer {
// Used for DCHECKs to ensure method calls are executed on the correct thread.
base::ThreadChecker io_thread_checker_;
base::WeakPtrFactory<FrameDeliverer> weak_factory_for_pool_;
base::WeakPtrFactory<FrameDeliverer> weak_factory_;
DISALLOW_COPY_AND_ASSIGN(FrameDeliverer);
......@@ -750,28 +736,27 @@ size_t WebMediaPlayerMS::VideoDecodedByteCount() const {
}
void WebMediaPlayerMS::OnFrameHidden() {
#if defined(OS_ANDROID)
DCHECK(thread_checker_.CalledOnValidThread());
// This method is called when the RenderFrame is sent to background or
// suspended. During undoable tab closures OnHidden() may be called back to
// back, so we can't rely on |render_frame_suspended_| being false here.
// Method called when the RenderFrame is sent to background and suspended
// (android). Substitute the displayed VideoFrame with a copy to avoid
// holding on to it unnecessarily.
//
// During undoable tab closures OnHidden() may be called back to back, so we
// can't rely on |render_frame_suspended_| being false here.
if (frame_deliverer_) {
io_task_runner_->PostTask(
FROM_HERE, base::Bind(&FrameDeliverer::SetRenderFrameSuspended,
base::Unretained(frame_deliverer_.get()), true));
}
// On Android, substitute the displayed VideoFrame with a copy to avoid holding
// onto it unnecessarily.
#if defined(OS_ANDROID)
if (!paused_)
compositor_->ReplaceCurrentFrameWithACopy();
#endif // defined(OS_ANDROID)
}
void WebMediaPlayerMS::OnFrameClosed() {
DCHECK(thread_checker_.CalledOnValidThread());
// On Android, pause the video completely for this time period.
#if defined(OS_ANDROID)
if (!paused_) {
Pause();
......@@ -779,16 +764,17 @@ void WebMediaPlayerMS::OnFrameClosed() {
}
delegate_->PlayerGone(delegate_id_);
#endif // defined(OS_ANDROID)
if (frame_deliverer_) {
io_task_runner_->PostTask(
FROM_HERE, base::Bind(&FrameDeliverer::SetRenderFrameSuspended,
base::Unretained(frame_deliverer_.get()), true));
}
#endif // defined(OS_ANDROID)
}
void WebMediaPlayerMS::OnFrameShown() {
#if defined(OS_ANDROID)
DCHECK(thread_checker_.CalledOnValidThread());
if (frame_deliverer_) {
......@@ -797,9 +783,7 @@ void WebMediaPlayerMS::OnFrameShown() {
base::Unretained(frame_deliverer_.get()), false));
}
// On Android, resume playback on visibility. play() clears
// |should_play_upon_shown_|.
#if defined(OS_ANDROID)
// Resume playback on visibility. play() clears |should_play_upon_shown_|.
if (should_play_upon_shown_)
Play();
#endif // defined(OS_ANDROID)
......
content/renderer/media/stream/webmediaplayer_ms_unittest.cc
View file @ f6b06f4d
......@@ -8,7 +8,6 @@
#include "base/message_loop/message_loop.h"
#include "base/run_loop.h"
#include "base/single_thread_task_runner.h"
#include "build/build_config.h"
#include "content/public/renderer/media_stream_renderer_factory.h"
#include "content/renderer/media/stream/webmediaplayer_ms.h"
#include "content/renderer/media/stream/webmediaplayer_ms_compositor.h"
......@@ -1092,65 +1091,6 @@ TEST_F(WebMediaPlayerMSTest, CreateHardwareFrames) {
EXPECT_CALL(*this, DoStopRendering());
}
#if !defined(OS_ANDROID)
// Tests that GpuMemoryBufferVideoFramePool is not called when page is hidden.
TEST_F(WebMediaPlayerMSTest, StopsCreatingHardwareFramesWhenHiddenOrClosed) {
MockMediaStreamVideoRenderer* provider = LoadAndGetFrameProvider(true);
SetGpuMemoryBufferVideoForTesting();
const int kTestBrake = static_cast<int>(FrameType::TEST_BRAKE);
static int tokens[] = {0, kTestBrake};
std::vector<int> timestamps(tokens, tokens + sizeof(tokens) / sizeof(int));
provider->QueueFrames(timestamps);
EXPECT_CALL(*this, DoSetWebLayer(true));
EXPECT_CALL(*this, DoStartRendering());
EXPECT_CALL(*this, DoReadyStateChanged(
blink::WebMediaPlayer::kReadyStateHaveMetadata));
EXPECT_CALL(*this, DoReadyStateChanged(
blink::WebMediaPlayer::kReadyStateHaveEnoughData));
EXPECT_CALL(*this,
CheckSizeChanged(gfx::Size(kStandardWidth, kStandardHeight)));
message_loop_controller_.RunAndWaitForStatus(
media::PipelineStatus::PIPELINE_OK);
ASSERT_EQ(1u, frame_ready_cbs_.size());
frame_ready_cbs_.clear();
// Hidden should stop passing frames to GpuMemoryBufferVideoFramePool.
player_->OnFrameHidden();
provider->QueueFrames(timestamps, false);
message_loop_controller_.RunAndWaitForStatus(
media::PipelineStatus::PIPELINE_OK);
ASSERT_EQ(0u, frame_ready_cbs_.size());
// Shown should resume passing frames to GpuMemoryBufferVideoFramePool.
player_->OnFrameShown();
provider->QueueFrames(timestamps, false);
message_loop_controller_.RunAndWaitForStatus(
media::PipelineStatus::PIPELINE_OK);
ASSERT_EQ(1u, frame_ready_cbs_.size());
frame_ready_cbs_.clear();
// Hidden should stop passing frames to GpuMemoryBufferVideoFramePool.
player_->OnFrameClosed();
provider->QueueFrames(timestamps, false);
message_loop_controller_.RunAndWaitForStatus(
media::PipelineStatus::PIPELINE_OK);
ASSERT_EQ(0u, frame_ready_cbs_.size());
// Shown should resume passing frames to GpuMemoryBufferVideoFramePool.
player_->OnFrameShown();
provider->QueueFrames(timestamps, false);
message_loop_controller_.RunAndWaitForStatus(
media::PipelineStatus::PIPELINE_OK);
ASSERT_EQ(1u, frame_ready_cbs_.size());
testing::Mock::VerifyAndClearExpectations(this);
EXPECT_CALL(*this, DoSetWebLayer(false));
EXPECT_CALL(*this, DoStopRendering());
}
#endif // !defined(OS_ANDROID)
#if defined(OS_ANDROID)
TEST_F(WebMediaPlayerMSTest, HiddenPlayerTests) {
LoadAndGetFrameProvider(true);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment