After a pipeline error, suspend is still triggered |
|||||||
Issue descriptionAfter an error, we expect Blink to pause the video element. Once paused, the 15 second suspend timer will trigger suspend, which will fail because the pipeline is stopped (resulting in a second OnPipielineError()). One of these solutions could work: - WMPI could remove the suspend timer when an error occurs. - WMPI could track the error state, and ignore duplicates. - WMPI could check IsRunning() before calling Suspend(). - PipelineController could silently drop calls after an error. - Pipeline could silently drop calls after an error.
,
Mar 19 2016
,
Mar 19 2016
Pri-2 since this really just affects UMA metrics and leaves a stale media session after a pipeline error.
,
Mar 19 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/60030663a474eb523aa9c958e5fe88fa844d8b79 commit 60030663a474eb523aa9c958e5fe88fa844d8b79 Author: dalecurtis <dalecurtis@chromium.org> Date: Sat Mar 19 00:28:34 2016 Drop media session on playback error; avoids idle suspension. Avoids leaving a stale interactive session when the player is in the error state and also avoids idle suspension from running later and triggering an invalid state error. The pipeline has already released its resources internally when the error occurred so the idle suspension is not necessary. BUG= 596157 Review URL: https://codereview.chromium.org/1815293003 Cr-Commit-Position: refs/heads/master@{#382137} [modify] https://crrev.com/60030663a474eb523aa9c958e5fe88fa844d8b79/media/blink/webmediaplayer_impl.cc
,
Mar 19 2016
,
Mar 19 2016
Your change meets the bar and is auto-approved for M50 (branch: 2661)
,
Mar 21 2016
Please try to merge your change to M50 branch 2661 asap as we're getting closer to M50 beta candidate cut for this week. Thank you.
,
Mar 21 2016
Discovered that issue 591244 was incorrectly marked as merged for some reason while trying to merge this. Merging that change and then this one. Will review merges to see if there were others that were incorrectly marked.
,
Mar 21 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0a7d7379015541ad6356f2cb4d90be4614ca3135 commit 0a7d7379015541ad6356f2cb4d90be4614ca3135 Author: Dale Curtis <dalecurtis@chromium.org> Date: Mon Mar 21 17:54:41 2016 Merge M50: "Drop media session on playback error; avoids idle suspension." Avoids leaving a stale interactive session when the player is in the error state and also avoids idle suspension from running later and triggering an invalid state error. The pipeline has already released its resources internally when the error occurred so the idle suspension is not necessary. BUG= 596157 Review URL: https://codereview.chromium.org/1815293003 Cr-Commit-Position: refs/heads/master@{#382137} (cherry picked from commit 60030663a474eb523aa9c958e5fe88fa844d8b79) Review URL: https://codereview.chromium.org/1816173003 . Cr-Commit-Position: refs/branch-heads/2661@{#317} Cr-Branched-From: ef6f6ae5e4c96622286b563658d5cd62a6cf1197-refs/heads/master@{#378081} [modify] https://crrev.com/0a7d7379015541ad6356f2cb4d90be4614ca3135/media/blink/webmediaplayer_impl.cc
,
Mar 21 2016
Looks like only one other intended merge change was tagged "merged-merged-2666", instead of merged-merged-2661. |
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by dalecur...@chromium.org
, Mar 19 2016Owner: dalecur...@chromium.org
Status: Started (was: Available)