Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(386)

Side by Side Diff: media/blink/webmediaplayer_impl.cc

Issue 1164343003: Fix WMPI::currentTime/seek race causing flaky Blink mediasource-redundant-seek.html on desktop (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Address nits Created 5 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
« no previous file with comments | « media/blink/webmediaplayer_impl.h ('k') | no next file » | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 // Copyright 2013 The Chromium Authors. All rights reserved. 1 // Copyright 2013 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "media/blink/webmediaplayer_impl.h" 5 #include "media/blink/webmediaplayer_impl.h"
6 6
7 #include <algorithm> 7 #include <algorithm>
8 #include <cmath> 8 #include <cmath>
9 #include <limits> 9 #include <limits>
10 10
(...skipping 97 matching lines...) Expand 10 before | Expand all | Expand 10 after
108 : frame_(frame), 108 : frame_(frame),
109 network_state_(WebMediaPlayer::NetworkStateEmpty), 109 network_state_(WebMediaPlayer::NetworkStateEmpty),
110 ready_state_(WebMediaPlayer::ReadyStateHaveNothing), 110 ready_state_(WebMediaPlayer::ReadyStateHaveNothing),
111 preload_(BufferedDataSource::AUTO), 111 preload_(BufferedDataSource::AUTO),
112 main_task_runner_(base::ThreadTaskRunnerHandle::Get()), 112 main_task_runner_(base::ThreadTaskRunnerHandle::Get()),
113 media_task_runner_(params.media_task_runner()), 113 media_task_runner_(params.media_task_runner()),
114 media_log_(params.media_log()), 114 media_log_(params.media_log()),
115 pipeline_(media_task_runner_, media_log_.get()), 115 pipeline_(media_task_runner_, media_log_.get()),
116 load_type_(LoadTypeURL), 116 load_type_(LoadTypeURL),
117 opaque_(false), 117 opaque_(false),
118 playback_rate_(0.0),
118 paused_(true), 119 paused_(true),
119 seeking_(false), 120 seeking_(false),
120 playback_rate_(0.0),
121 ended_(false), 121 ended_(false),
122 pending_seek_(false), 122 pending_seek_(false),
123 pending_seek_seconds_(0.0f),
124 should_notify_time_changed_(false), 123 should_notify_time_changed_(false),
125 client_(client), 124 client_(client),
126 delegate_(delegate), 125 delegate_(delegate),
127 defer_load_cb_(params.defer_load_cb()), 126 defer_load_cb_(params.defer_load_cb()),
128 context_3d_cb_(params.context_3d_cb()), 127 context_3d_cb_(params.context_3d_cb()),
129 supports_save_(true), 128 supports_save_(true),
130 chunk_demuxer_(NULL), 129 chunk_demuxer_(NULL),
131 // Threaded compositing isn't enabled universally yet. 130 // Threaded compositing isn't enabled universally yet.
132 compositor_task_runner_( 131 compositor_task_runner_(
133 params.compositor_task_runner() 132 params.compositor_task_runner()
(...skipping 149 matching lines...) Expand 10 before | Expand all | Expand 10 after
283 void WebMediaPlayerImpl::seek(double seconds) { 282 void WebMediaPlayerImpl::seek(double seconds) {
284 DVLOG(1) << __FUNCTION__ << "(" << seconds << "s)"; 283 DVLOG(1) << __FUNCTION__ << "(" << seconds << "s)";
285 DCHECK(main_task_runner_->BelongsToCurrentThread()); 284 DCHECK(main_task_runner_->BelongsToCurrentThread());
286 285
287 ended_ = false; 286 ended_ = false;
288 287
289 ReadyState old_state = ready_state_; 288 ReadyState old_state = ready_state_;
290 if (ready_state_ > WebMediaPlayer::ReadyStateHaveMetadata) 289 if (ready_state_ > WebMediaPlayer::ReadyStateHaveMetadata)
291 SetReadyState(WebMediaPlayer::ReadyStateHaveMetadata); 290 SetReadyState(WebMediaPlayer::ReadyStateHaveMetadata);
292 291
293 base::TimeDelta seek_time = ConvertSecondsToTimestamp(seconds); 292 base::TimeDelta new_seek_time = ConvertSecondsToTimestamp(seconds);
294 293
295 if (seeking_) { 294 if (seeking_) {
295 if (new_seek_time == seek_time_) {
296 if (chunk_demuxer_) {
297 if (!pending_seek_) {
298 // If using media source demuxer, only suppress redundant seeks if
299 // there is no pending seek. This enforces that any pending seek that
300 // results in a demuxer seek is preceded by matching
301 // CancelPendingSeek() and StartWaitingForSeek() calls.
302 return;
303 }
304 } else {
305 // Suppress all redundant seeks if unrestricted by media source demuxer
306 // API.
307 pending_seek_ = false;
308 pending_seek_time_ = base::TimeDelta();
309 return;
310 }
311 }
312
296 pending_seek_ = true; 313 pending_seek_ = true;
297 pending_seek_seconds_ = seconds; 314 pending_seek_time_ = new_seek_time;
298 if (chunk_demuxer_) 315 if (chunk_demuxer_)
299 chunk_demuxer_->CancelPendingSeek(seek_time); 316 chunk_demuxer_->CancelPendingSeek(pending_seek_time_);
300 return; 317 return;
301 } 318 }
302 319
303 media_log_->AddEvent(media_log_->CreateSeekEvent(seconds)); 320 media_log_->AddEvent(media_log_->CreateSeekEvent(seconds));
304 321
305 // Update our paused time. 322 // Update our paused time.
306 // In paused state ignore the seek operations to current time if the loading 323 // In paused state ignore the seek operations to current time if the loading
307 // is completed and generate OnPipelineBufferingStateChanged event to 324 // is completed and generate OnPipelineBufferingStateChanged event to
308 // eventually fire seeking and seeked events 325 // eventually fire seeking and seeked events
309 if (paused_) { 326 if (paused_) {
310 if (paused_time_ != seek_time) { 327 if (paused_time_ != new_seek_time) {
311 paused_time_ = seek_time; 328 paused_time_ = new_seek_time;
312 } else if (old_state == ReadyStateHaveEnoughData) { 329 } else if (old_state == ReadyStateHaveEnoughData) {
313 main_task_runner_->PostTask( 330 main_task_runner_->PostTask(
314 FROM_HERE, 331 FROM_HERE,
315 base::Bind(&WebMediaPlayerImpl::OnPipelineBufferingStateChanged, 332 base::Bind(&WebMediaPlayerImpl::OnPipelineBufferingStateChanged,
316 AsWeakPtr(), BUFFERING_HAVE_ENOUGH)); 333 AsWeakPtr(), BUFFERING_HAVE_ENOUGH));
317 return; 334 return;
318 } 335 }
319 } 336 }
320 337
321 seeking_ = true; 338 seeking_ = true;
339 seek_time_ = new_seek_time;
322 340
323 if (chunk_demuxer_) 341 if (chunk_demuxer_)
324 chunk_demuxer_->StartWaitingForSeek(seek_time); 342 chunk_demuxer_->StartWaitingForSeek(seek_time_);
325 343
326 // Kick off the asynchronous seek! 344 // Kick off the asynchronous seek!
327 pipeline_.Seek( 345 pipeline_.Seek(seek_time_, BIND_TO_RENDER_LOOP1(
328 seek_time, 346 &WebMediaPlayerImpl::OnPipelineSeeked, true));
329 BIND_TO_RENDER_LOOP1(&WebMediaPlayerImpl::OnPipelineSeeked, true));
330 } 347 }
331 348
332 void WebMediaPlayerImpl::setRate(double rate) { 349 void WebMediaPlayerImpl::setRate(double rate) {
333 DVLOG(1) << __FUNCTION__ << "(" << rate << ")"; 350 DVLOG(1) << __FUNCTION__ << "(" << rate << ")";
334 DCHECK(main_task_runner_->BelongsToCurrentThread()); 351 DCHECK(main_task_runner_->BelongsToCurrentThread());
335 352
336 // TODO(kylep): Remove when support for negatives is added. Also, modify the 353 // TODO(kylep): Remove when support for negatives is added. Also, modify the
337 // following checks so rewind uses reasonable values also. 354 // following checks so rewind uses reasonable values also.
338 if (rate < 0.0) 355 if (rate < 0.0)
339 return; 356 return;
(...skipping 96 matching lines...) Expand 10 before | Expand all | Expand 10 after
436 453
437 double WebMediaPlayerImpl::currentTime() const { 454 double WebMediaPlayerImpl::currentTime() const {
438 DCHECK(main_task_runner_->BelongsToCurrentThread()); 455 DCHECK(main_task_runner_->BelongsToCurrentThread());
439 DCHECK_NE(ready_state_, WebMediaPlayer::ReadyStateHaveNothing); 456 DCHECK_NE(ready_state_, WebMediaPlayer::ReadyStateHaveNothing);
440 457
441 // TODO(scherkus): Replace with an explicit ended signal to HTMLMediaElement, 458 // TODO(scherkus): Replace with an explicit ended signal to HTMLMediaElement,
442 // see http://crbug.com/409280 459 // see http://crbug.com/409280
443 if (ended_) 460 if (ended_)
444 return duration(); 461 return duration();
445 462
463 // We know the current seek time better than pipeline: pipeline may processing
464 // an earlier seek before a pending seek has been started, or it might not yet
465 // have the current seek time returnable via GetMediaTime().
466 if (seeking()) {
467 return pending_seek_ ? pending_seek_time_.InSecondsF()
468 : seek_time_.InSecondsF();
469 }
470
446 return (paused_ ? paused_time_ : pipeline_.GetMediaTime()).InSecondsF(); 471 return (paused_ ? paused_time_ : pipeline_.GetMediaTime()).InSecondsF();
447 } 472 }
448 473
449 WebMediaPlayer::NetworkState WebMediaPlayerImpl::networkState() const { 474 WebMediaPlayer::NetworkState WebMediaPlayerImpl::networkState() const {
450 DCHECK(main_task_runner_->BelongsToCurrentThread()); 475 DCHECK(main_task_runner_->BelongsToCurrentThread());
451 return network_state_; 476 return network_state_;
452 } 477 }
453 478
454 WebMediaPlayer::ReadyState WebMediaPlayerImpl::readyState() const { 479 WebMediaPlayer::ReadyState WebMediaPlayerImpl::readyState() const {
455 DCHECK(main_task_runner_->BelongsToCurrentThread()); 480 DCHECK(main_task_runner_->BelongsToCurrentThread());
(...skipping 251 matching lines...) Expand 10 before | Expand all | Expand 10 after
707 result.completeWithError( 732 result.completeWithError(
708 blink::WebContentDecryptionModuleExceptionNotSupportedError, 0, 733 blink::WebContentDecryptionModuleExceptionNotSupportedError, 0,
709 "Unable to set MediaKeys object"); 734 "Unable to set MediaKeys object");
710 } 735 }
711 736
712 void WebMediaPlayerImpl::OnPipelineSeeked(bool time_changed, 737 void WebMediaPlayerImpl::OnPipelineSeeked(bool time_changed,
713 PipelineStatus status) { 738 PipelineStatus status) {
714 DVLOG(1) << __FUNCTION__ << "(" << time_changed << ", " << status << ")"; 739 DVLOG(1) << __FUNCTION__ << "(" << time_changed << ", " << status << ")";
715 DCHECK(main_task_runner_->BelongsToCurrentThread()); 740 DCHECK(main_task_runner_->BelongsToCurrentThread());
716 seeking_ = false; 741 seeking_ = false;
742 seek_time_ = base::TimeDelta();
717 if (pending_seek_) { 743 if (pending_seek_) {
744 double pending_seek_seconds = pending_seek_time_.InSecondsF();
718 pending_seek_ = false; 745 pending_seek_ = false;
719 seek(pending_seek_seconds_); 746 pending_seek_time_ = base::TimeDelta();
747 seek(pending_seek_seconds);
720 return; 748 return;
721 } 749 }
722 750
723 if (status != PIPELINE_OK) { 751 if (status != PIPELINE_OK) {
724 OnPipelineError(status); 752 OnPipelineError(status);
725 return; 753 return;
726 } 754 }
727 755
728 // Update our paused time. 756 // Update our paused time.
729 if (paused_) 757 if (paused_)
(...skipping 275 matching lines...) Expand 10 before | Expand all | Expand 10 after
1005 1033
1006 // pause() may be called after playback has ended and the HTMLMediaElement 1034 // pause() may be called after playback has ended and the HTMLMediaElement
1007 // requires that currentTime() == duration() after ending. We want to ensure 1035 // requires that currentTime() == duration() after ending. We want to ensure
1008 // |paused_time_| matches currentTime() in this case or a future seek() may 1036 // |paused_time_| matches currentTime() in this case or a future seek() may
1009 // incorrectly discard what it thinks is a seek to the existing time. 1037 // incorrectly discard what it thinks is a seek to the existing time.
1010 paused_time_ = 1038 paused_time_ =
1011 ended_ ? pipeline_.GetMediaDuration() : pipeline_.GetMediaTime(); 1039 ended_ ? pipeline_.GetMediaDuration() : pipeline_.GetMediaTime();
1012 } 1040 }
1013 1041
1014 } // namespace media 1042 } // namespace media
OLDNEW
« no previous file with comments | « media/blink/webmediaplayer_impl.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698