media: Merge AndroidVideoSurfaceChooser's Init and Update
This replaces Initialize() with SetClientCallbacks() and UpdateState().
This lets callers not worry about whether they've called Initialize()
yet. For example, without this MCVD needs a bool to know whether it's
called Initialize() or not.
Bug: 660942
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I0b1b1ba2d958b9f27c9963d2e610bc4697b8495e
Reviewed-on: https://chromium-review.googlesource.com/686334
Reviewed-by: Frank Liberato <liberato@chromium.org>
Commit-Queue: Chris Watkins <watk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#505527}diff --git a/media/gpu/android/android_video_decode_accelerator.cc b/media/gpu/android/android_video_decode_accelerator.cc
index 53aa5e2..e333917 100644
--- a/media/gpu/android/android_video_decode_accelerator.cc
+++ b/media/gpu/android/android_video_decode_accelerator.cc
@@ -451,12 +451,12 @@
// the synchronous case. It will be soon, though. For pre-M, we rely on the
// fact that |surface_chooser_| won't tell us to use a SurfaceTexture while
// waiting for an overlay to become ready, for example.
- surface_chooser_->Initialize(
+ surface_chooser_->SetClientCallbacks(
base::Bind(&AndroidVideoDecodeAccelerator::OnSurfaceTransition,
weak_this_factory_.GetWeakPtr()),
base::Bind(&AndroidVideoDecodeAccelerator::OnSurfaceTransition,
- weak_this_factory_.GetWeakPtr(), nullptr),
- std::move(factory), surface_chooser_state_);
+ weak_this_factory_.GetWeakPtr(), nullptr));
+ surface_chooser_->UpdateState(std::move(factory), surface_chooser_state_);
}
void AndroidVideoDecodeAccelerator::OnSurfaceTransition(
diff --git a/media/gpu/android/android_video_decode_accelerator_unittest.cc b/media/gpu/android/android_video_decode_accelerator_unittest.cc
index ae280466..461823a 100644
--- a/media/gpu/android/android_video_decode_accelerator_unittest.cc
+++ b/media/gpu/android/android_video_decode_accelerator_unittest.cc
@@ -281,7 +281,7 @@
config_.overlay_info.surface_id = SurfaceManager::kNoSurfaceID;
- EXPECT_CALL(*chooser_, MockInitialize()).Times(0);
+ EXPECT_CALL(*chooser_, MockUpdateState()).Times(0);
EXPECT_CALL(client_, NotifyInitializationComplete(true));
// It would be nicer if we didn't just force this on, since we might do so
diff --git a/media/gpu/android/android_video_surface_chooser.h b/media/gpu/android/android_video_surface_chooser.h
index ca46509..c50de22 100644
--- a/media/gpu/android/android_video_surface_chooser.h
+++ b/media/gpu/android/android_video_surface_chooser.h
@@ -60,19 +60,14 @@
AndroidVideoSurfaceChooser() {}
virtual ~AndroidVideoSurfaceChooser() {}
- // Notify us that our client is ready for overlays. We will send it a
- // callback telling it whether to start with a SurfaceTexture or overlay,
- // either synchronously or post one very soon. |initial_factory| can be
- // an empty callback to indicate "no factory".
- virtual void Initialize(UseOverlayCB use_overlay_cb,
- UseSurfaceTextureCB use_surface_texture_cb,
- AndroidOverlayFactoryCB initial_factory,
- const State& initial_state) = 0;
+ // Sets the client callbacks to be called when a new surface choice is made.
+ // Must be called before UpdateState();
+ virtual void SetClientCallbacks(UseOverlayCB use_overlay_cb,
+ UseSurfaceTextureCB use_surface_texture_cb);
- // Notify us that a new factory has arrived and / or other state is available.
- // |*new_factory| may be an is_null() callback to indicate that the most
- // recent factory has been revoked. |!new_factory.has_value()| results in no
- // change to the factory.
+ // Updates the current state and makes a new surface choice with the new
+ // state. If |new_factory| is empty, the factory is left as-is. Otherwise,
+ // the factory is updated to |*new_factory|.
virtual void UpdateState(base::Optional<AndroidOverlayFactoryCB> new_factory,
const State& new_state) = 0;
diff --git a/media/gpu/android/android_video_surface_chooser_impl.cc b/media/gpu/android/android_video_surface_chooser_impl.cc
index 13abd65a1..d1882075e 100644
--- a/media/gpu/android/android_video_surface_chooser_impl.cc
+++ b/media/gpu/android/android_video_surface_chooser_impl.cc
@@ -29,82 +29,60 @@
AndroidVideoSurfaceChooserImpl::~AndroidVideoSurfaceChooserImpl() {}
-void AndroidVideoSurfaceChooserImpl::Initialize(
+void AndroidVideoSurfaceChooserImpl::SetClientCallbacks(
UseOverlayCB use_overlay_cb,
- UseSurfaceTextureCB use_surface_texture_cb,
- AndroidOverlayFactoryCB initial_factory,
- const State& initial_state) {
+ UseSurfaceTextureCB use_surface_texture_cb) {
+ DCHECK(use_overlay_cb && use_surface_texture_cb);
use_overlay_cb_ = std::move(use_overlay_cb);
use_surface_texture_cb_ = std::move(use_surface_texture_cb);
-
- overlay_factory_ = std::move(initial_factory);
-
- current_state_ = initial_state;
-
- // Pre-M, we choose now. This lets Choose() never worry about the pre-M path.
- if (!allow_dynamic_) {
- if (overlay_factory_ &&
- (current_state_.is_fullscreen || current_state_.is_secure ||
- current_state_.is_required)) {
- SwitchToOverlay(false);
- } else {
- SwitchToSurfaceTexture();
- }
- } else {
- Choose();
- }
}
void AndroidVideoSurfaceChooserImpl::UpdateState(
base::Optional<AndroidOverlayFactoryCB> new_factory,
const State& new_state) {
+ DCHECK(use_overlay_cb_);
+ bool entered_fullscreen =
+ !current_state_.is_fullscreen && new_state.is_fullscreen;
+ current_state_ = new_state;
+
+ bool factory_changed = new_factory.has_value();
+ if (factory_changed)
+ overlay_factory_ = std::move(*new_factory);
+
+ if (!allow_dynamic_) {
+ if (!initial_state_received_) {
+ initial_state_received_ = true;
+ // Choose here so that Choose() doesn't have to handle non-dynamic.
+ if (overlay_factory_ &&
+ (current_state_.is_fullscreen || current_state_.is_secure ||
+ current_state_.is_required)) {
+ SwitchToOverlay(false);
+ } else {
+ SwitchToSurfaceTexture();
+ }
+ }
+ return;
+ }
+
// If we're entering fullscreen, clear any previous failure attempt. It's
// likely that any previous failure was due to a lack of power efficiency,
// but entering fs likely changes that anyway.
- if (!current_state_.is_fullscreen && new_state.is_fullscreen)
+ if (entered_fullscreen)
most_recent_overlay_failure_ = base::TimeTicks();
- current_state_ = new_state;
-
- if (!new_factory) {
- if (allow_dynamic_)
- Choose();
- return;
- }
-
- overlay_factory_ = std::move(*new_factory);
-
- // If we started construction of an overlay, but it's not ready yet, then
- // just drop it. It's from the wrong factory.
- if (overlay_)
+ // If the factory changed, we should cancel pending overlay requests and
+ // set the client state back to Unknown if they're using an old overlay.
+ if (factory_changed) {
overlay_ = nullptr;
-
- // Pre-M, we can't change the output surface. Just stop here.
- if (!allow_dynamic_) {
- // If we still haven't told the client anything, then it's because an
- // overlay factory was provided to Initialize(), and changed before the
- // overlay request finished. Switch to SurfaceTexture. We could, I guess,
- // try to use the new factory if any.
- if (client_overlay_state_ == kUnknown)
- SwitchToSurfaceTexture();
- return;
- }
-
- // We're switching factories, so any existing overlay should be cancelled.
- // We also clear the state back to Unknown so that there's no confusion about
- // whether the client is using the 'right' overlay; it's not. Any previous
- // overlay was from the previous factory.
- if (client_overlay_state_ == kUsingOverlay) {
- overlay_ = nullptr;
- client_overlay_state_ = kUnknown;
+ if (client_overlay_state_ == kUsingOverlay)
+ client_overlay_state_ = kUnknown;
}
Choose();
}
void AndroidVideoSurfaceChooserImpl::Choose() {
- // Pre-M, we decide based on whether we have or don't have a factory. We
- // shouldn't be called.
+ // Pre-M we shouldn't be called.
DCHECK(allow_dynamic_);
// TODO(liberato): should this depend on resolution?
diff --git a/media/gpu/android/android_video_surface_chooser_impl.h b/media/gpu/android/android_video_surface_chooser_impl.h
index c325d49..fedc2dfb 100644
--- a/media/gpu/android/android_video_surface_chooser_impl.h
+++ b/media/gpu/android/android_video_surface_chooser_impl.h
@@ -29,10 +29,8 @@
~AndroidVideoSurfaceChooserImpl() override;
// AndroidVideoSurfaceChooser
- void Initialize(UseOverlayCB use_overlay_cb,
- UseSurfaceTextureCB use_surface_texture_cb,
- AndroidOverlayFactoryCB initial_factory,
- const State& initial_state) override;
+ void SetClientCallbacks(UseOverlayCB use_overlay_cb,
+ UseSurfaceTextureCB use_surface_texture_cb) override;
void UpdateState(base::Optional<AndroidOverlayFactoryCB> new_factory,
const State& new_state) override;
@@ -83,6 +81,8 @@
State current_state_;
+ bool initial_state_received_ = false;
+
// Not owned by us.
base::TickClock* tick_clock_;
diff --git a/media/gpu/android/android_video_surface_chooser_impl_unittest.cc b/media/gpu/android/android_video_surface_chooser_impl_unittest.cc
index 12451bdd..a7aeaeae 100644
--- a/media/gpu/android/android_video_surface_chooser_impl_unittest.cc
+++ b/media/gpu/android/android_video_surface_chooser_impl_unittest.cc
@@ -130,10 +130,12 @@
void StartChooser(AndroidOverlayFactoryCB factory) {
chooser_ = base::MakeUnique<AndroidVideoSurfaceChooserImpl>(allow_dynamic_,
&tick_clock_);
- chooser_->Initialize(
+ chooser_->SetClientCallbacks(
base::Bind(&MockClient::UseOverlayImpl, base::Unretained(&client_)),
- base::Bind(&MockClient::UseSurfaceTexture, base::Unretained(&client_)),
- std::move(factory), chooser_state_);
+ base::Bind(&MockClient::UseSurfaceTexture, base::Unretained(&client_)));
+ chooser_->UpdateState(
+ factory ? base::make_optional(std::move(factory)) : base::nullopt,
+ chooser_state_);
}
// Start the chooser with |overlay_|, and verify that the client is told to
diff --git a/media/gpu/android/fake_android_video_surface_chooser.cc b/media/gpu/android/fake_android_video_surface_chooser.cc
index 9f0168b..ec005ff 100644
--- a/media/gpu/android/fake_android_video_surface_chooser.cc
+++ b/media/gpu/android/fake_android_video_surface_chooser.cc
@@ -9,15 +9,12 @@
FakeSurfaceChooser::FakeSurfaceChooser() = default;
FakeSurfaceChooser::~FakeSurfaceChooser() = default;
-void FakeSurfaceChooser::Initialize(UseOverlayCB use_overlay_cb,
- UseSurfaceTextureCB use_surface_texture_cb,
- AndroidOverlayFactoryCB initial_factory,
- const State& initial_state) {
- MockInitialize();
+void FakeSurfaceChooser::SetClientCallbacks(
+ UseOverlayCB use_overlay_cb,
+ UseSurfaceTextureCB use_surface_texture_cb) {
+ MockSetClientCallbacks();
use_overlay_cb_ = std::move(use_overlay_cb);
use_surface_texture_cb_ = std::move(use_surface_texture_cb);
- factory_ = std::move(initial_factory);
- current_state_ = initial_state;
}
void FakeSurfaceChooser::UpdateState(
diff --git a/media/gpu/android/fake_android_video_surface_chooser.h b/media/gpu/android/fake_android_video_surface_chooser.h
index 7363012..a7000ca5 100644
--- a/media/gpu/android/fake_android_video_surface_chooser.h
+++ b/media/gpu/android/fake_android_video_surface_chooser.h
@@ -19,17 +19,15 @@
~FakeSurfaceChooser() override;
// Mocks that are called by the fakes below.
- MOCK_METHOD0(MockInitialize, void());
+ MOCK_METHOD0(MockSetClientCallbacks, void());
MOCK_METHOD0(MockUpdateState, void());
// Called by UpdateState if the factory is changed. It is called with true if
// and only if the replacement factory isn't null.
MOCK_METHOD1(MockReplaceOverlayFactory, void(bool));
- void Initialize(UseOverlayCB use_overlay_cb,
- UseSurfaceTextureCB use_surface_texture_cb,
- AndroidOverlayFactoryCB initial_factory,
- const State& initial_state) override;
+ void SetClientCallbacks(UseOverlayCB use_overlay_cb,
+ UseSurfaceTextureCB use_surface_texture_cb) override;
void UpdateState(base::Optional<AndroidOverlayFactoryCB> factory,
const State& new_state) override;
diff --git a/media/gpu/android/media_codec_video_decoder.cc b/media/gpu/android/media_codec_video_decoder.cc
index 522c389..0cfceb0 100644
--- a/media/gpu/android/media_codec_video_decoder.cc
+++ b/media/gpu/android/media_codec_video_decoder.cc
@@ -118,6 +118,11 @@
weak_factory_(this),
codec_allocator_weak_factory_(this) {
DVLOG(2) << __func__;
+ surface_chooser_->SetClientCallbacks(
+ base::Bind(&MediaCodecVideoDecoder::OnSurfaceChosen,
+ weak_factory_.GetWeakPtr()),
+ base::Bind(&MediaCodecVideoDecoder::OnSurfaceChosen,
+ weak_factory_.GetWeakPtr(), nullptr));
}
MediaCodecVideoDecoder::~MediaCodecVideoDecoder() {
@@ -222,12 +227,7 @@
DCHECK_EQ(state_, State::kInitializing);
// Initialize |surface_chooser_| and wait for its decision. Note: the
// callback may be reentrant.
- surface_chooser_->Initialize(
- base::Bind(&MediaCodecVideoDecoder::OnSurfaceChosen,
- weak_factory_.GetWeakPtr()),
- base::Bind(&MediaCodecVideoDecoder::OnSurfaceChosen,
- weak_factory_.GetWeakPtr(), nullptr),
- CreateOverlayFactoryCb(), chooser_state_);
+ surface_chooser_->UpdateState(CreateOverlayFactoryCb(), chooser_state_);
}
void MediaCodecVideoDecoder::OnSurfaceChosen(
diff --git a/media/gpu/android/media_codec_video_decoder_unittest.cc b/media/gpu/android/media_codec_video_decoder_unittest.cc
index 1126c5b..d828bfb 100644
--- a/media/gpu/android/media_codec_video_decoder_unittest.cc
+++ b/media/gpu/android/media_codec_video_decoder_unittest.cc
@@ -210,7 +210,7 @@
TEST_F(MediaCodecVideoDecoderTest, InitializeDoesntInitSurfaceOrCodec) {
EXPECT_CALL(*video_frame_factory_, Initialize(_, _, _)).Times(0);
- EXPECT_CALL(*surface_chooser_, MockInitialize()).Times(0);
+ EXPECT_CALL(*surface_chooser_, MockUpdateState()).Times(0);
EXPECT_CALL(*codec_allocator_, MockCreateMediaCodecAsync(_, _)).Times(0);
Initialize();
}
@@ -223,7 +223,7 @@
TEST_F(MediaCodecVideoDecoderTest, FirstDecodeTriggersSurfaceChooserInit) {
Initialize();
- EXPECT_CALL(*surface_chooser_, MockInitialize());
+ EXPECT_CALL(*surface_chooser_, MockUpdateState());
mcvd_->Decode(fake_decoder_buffer_, decode_cb_.Get());
}
@@ -239,7 +239,7 @@
ON_CALL(*video_frame_factory_, Initialize(_, _, _))
.WillByDefault(RunCallback<2>(nullptr));
EXPECT_CALL(decode_cb_, Run(DecodeStatus::DECODE_ERROR)).Times(1);
- EXPECT_CALL(*surface_chooser_, MockInitialize()).Times(0);
+ EXPECT_CALL(*surface_chooser_, MockUpdateState()).Times(0);
mcvd_->Decode(fake_decoder_buffer_, decode_cb_.Get());
}
