Commit 1ed6ada5 authored by Thomas Guilbert's avatar Thomas Guilbert Committed by Commit Bot

Improve WMPI's Cast play/pause responsiveness

Currently, tapping on the cast notifications' play/pause button in short
succession only results in WMPI's play/pause state being updated half
the time.

The problem comes from the fact that there is no way to tell if WMPI's
play/pause state toggled due to a user gesture, or due to the
FlingingRenderer asking it to toggle to match the cast device's state.
In both cases, we expect the remote device to transition, and we ignore
transitory play state updates.

This CL fixes the issue by saving the last state the remote device has
sent, and comparing it against WMPI's play/pause commands.

Change-Id: Ifa1cbe64fcfedb9e43d8550ad010ba07e68b4219
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1804223Reviewed-by: default avatarDale Curtis <dalecurtis@chromium.org>
Commit-Queue: Thomas Guilbert <tguilbert@chromium.org>
Cr-Commit-Position: refs/heads/master@{#696603}
parent c07fda2b
...@@ -105,10 +105,10 @@ void FlingingRenderer::StartPlayingFrom(base::TimeDelta time) { ...@@ -105,10 +105,10 @@ void FlingingRenderer::StartPlayingFrom(base::TimeDelta time) {
void FlingingRenderer::SetPlaybackRate(double playback_rate) { void FlingingRenderer::SetPlaybackRate(double playback_rate) {
DVLOG(2) << __func__; DVLOG(2) << __func__;
if (playback_rate == 0) { if (playback_rate == 0) {
SetTargetPlayState(PlayState::PAUSED); SetExpectedPlayState(PlayState::PAUSED);
controller_->GetMediaController()->Pause(); controller_->GetMediaController()->Pause();
} else { } else {
SetTargetPlayState(PlayState::PLAYING); SetExpectedPlayState(PlayState::PLAYING);
controller_->GetMediaController()->Play(); controller_->GetMediaController()->Play();
} }
} }
...@@ -122,18 +122,19 @@ base::TimeDelta FlingingRenderer::GetMediaTime() { ...@@ -122,18 +122,19 @@ base::TimeDelta FlingingRenderer::GetMediaTime() {
return controller_->GetApproximateCurrentTime(); return controller_->GetApproximateCurrentTime();
} }
void FlingingRenderer::SetTargetPlayState(PlayState state) { void FlingingRenderer::SetExpectedPlayState(PlayState state) {
DVLOG(3) << __func__ << " : state " << static_cast<int>(state); DVLOG(3) << __func__ << " : state " << static_cast<int>(state);
DCHECK(state == PlayState::PLAYING || state == PlayState::PAUSED); DCHECK(state == PlayState::PLAYING || state == PlayState::PAUSED);
reached_target_play_state_ = false;
target_play_state_ = state; expected_play_state_ = state;
play_state_is_stable_ = (expected_play_state_ == last_play_state_received_);
} }
void FlingingRenderer::OnMediaStatusUpdated(const media::MediaStatus& status) { void FlingingRenderer::OnMediaStatusUpdated(const media::MediaStatus& status) {
const auto& current_state = status.state; const auto& current_state = status.state;
if (current_state == target_play_state_) if (current_state == expected_play_state_)
reached_target_play_state_ = true; play_state_is_stable_ = true;
// Because we can get a MediaStatus update at any time from the device, only // Because we can get a MediaStatus update at any time from the device, only
// handle state updates after we have reached the target state. // handle state updates after we have reached the target state.
...@@ -146,7 +147,7 @@ void FlingingRenderer::OnMediaStatusUpdated(const media::MediaStatus& status) { ...@@ -146,7 +147,7 @@ void FlingingRenderer::OnMediaStatusUpdated(const media::MediaStatus& status) {
// queue a new PLAYING. // queue a new PLAYING.
// - The local device enters a tick/tock feedback loop of constantly // - The local device enters a tick/tock feedback loop of constantly
// requesting the wrong state of PLAYING/PAUSED. // requesting the wrong state of PLAYING/PAUSED.
if (!reached_target_play_state_) if (!play_state_is_stable_)
return; return;
// Ignore all non PLAYING/PAUSED states. // Ignore all non PLAYING/PAUSED states.
...@@ -160,10 +161,12 @@ void FlingingRenderer::OnMediaStatusUpdated(const media::MediaStatus& status) { ...@@ -160,10 +161,12 @@ void FlingingRenderer::OnMediaStatusUpdated(const media::MediaStatus& status) {
return; return;
} }
// We previously reached a stable target PlayState, and the cast device has // Save whether the remote device is currently playing or paused.
// reached a new stable PlayState without WMPI having asked for it. last_play_state_received_ = current_state;
// Let WMPI know it should update itself.
if (current_state != target_play_state_) // If the remote device's play state has toggled and we didn't initiate it,
// notify WMPI to update it's own play/pause state.
if (last_play_state_received_ != expected_play_state_)
client_extension_->OnRemotePlayStateChange(current_state); client_extension_->OnRemotePlayStateChange(current_state);
} }
......
...@@ -64,13 +64,19 @@ class CONTENT_EXPORT FlingingRenderer : public media::Renderer, ...@@ -64,13 +64,19 @@ class CONTENT_EXPORT FlingingRenderer : public media::Renderer,
std::unique_ptr<media::FlingingController> controller, std::unique_ptr<media::FlingingController> controller,
ClientExtensionPtr client_extension); ClientExtensionPtr client_extension);
void SetTargetPlayState(PlayState state); void SetExpectedPlayState(PlayState state);
// The state that we expect the remotely playing video to transition into. // The play state that we expect the remote device to reach.
// This is used to differentiate between state changes that originated from // Updated whenever WMPI sends play/pause commands.
// this device versus external devices. PlayState expected_play_state_ = PlayState::UNKNOWN;
PlayState target_play_state_ = PlayState::UNKNOWN;
bool reached_target_play_state_ = false; // True when the remote device has reached the expected play state.
// False when it is transitioning.
bool play_state_is_stable_ = false;
// The last "stable" play state received from the cast device.
// Only updated when |play_state_is_stable_| is true.
PlayState last_play_state_received_ = PlayState::UNKNOWN;
media::RendererClient* client_; media::RendererClient* client_;
......
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