New issue
Advanced search Search tips

Issue 695734 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Nov 2017
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 715244



Sign in to add a comment

Allow media::Pipeline Stop()/Start() and Demuxer replacement

Project Member Reported by tguilbert@chromium.org, Feb 24 2017

Issue description

This bug covers the work necessary to allow safely restarting media::Pipeline, by properly handling state and pending callbacks through a Stop()/Start() cycle.

This will allow us to change demuxers after the initial Pipeline::Start() call.
 
Blocking: 715244
Project Member

Comment 2 by bugdroid1@chromium.org, Aug 11 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2ff16cbf883246bb6131412930538bce240125f4

commit 2ff16cbf883246bb6131412930538bce240125f4
Author: Thomas Guilbert <tguilbert@chromium.org>
Date: Fri Aug 11 02:32:34 2017

Move internal Pipeline::Stop() call to WMPI

Currently, almost all calls to Pipeline's public interface first pass
through PipelineController. The one exception is that Pipeline calls its
own Stop() in OnError().

This CL moves the internal Pipeline's calls to Stop() out to WMPI. This
gives PipelineController an opportunity to adjust its internal state
appropriately. It also makes Pipeline easier to reason about, since it
doesn't change its own state.

Bug:  695734 
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: Ice20a4e93e0d1a2068f195ad9da15763353f8d5d
Reviewed-on: https://chromium-review.googlesource.com/607176
Commit-Queue: Thomas Guilbert <tguilbert@chromium.org>
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#493649}
[modify] https://crrev.com/2ff16cbf883246bb6131412930538bce240125f4/media/base/pipeline.h
[modify] https://crrev.com/2ff16cbf883246bb6131412930538bce240125f4/media/base/pipeline_impl.cc
[modify] https://crrev.com/2ff16cbf883246bb6131412930538bce240125f4/media/base/pipeline_impl.h
[modify] https://crrev.com/2ff16cbf883246bb6131412930538bce240125f4/media/base/pipeline_impl_unittest.cc
[modify] https://crrev.com/2ff16cbf883246bb6131412930538bce240125f4/media/blink/webmediaplayer_impl.cc
[modify] https://crrev.com/2ff16cbf883246bb6131412930538bce240125f4/media/filters/pipeline_controller.cc
[modify] https://crrev.com/2ff16cbf883246bb6131412930538bce240125f4/media/filters/pipeline_controller.h

Project Member

Comment 3 by bugdroid1@chromium.org, Aug 11 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/42c3d2a9e44b422c0ef4c9516a4011aca229a862

commit 42c3d2a9e44b422c0ef4c9516a4011aca229a862
Author: Olga Sharonova <olka@chromium.org>
Date: Fri Aug 11 08:01:15 2017

Revert "Move internal Pipeline::Stop() call to WMPI"

This reverts commit 2ff16cbf883246bb6131412930538bce240125f4.

Reason for revert: suspected of breaking PipelineIntegrationTest.MediaSource_ConfigChange_EncryptedThenClear_MP4_CENC on ChrOS
BUG= 754596 

Original change's description:
> Move internal Pipeline::Stop() call to WMPI
> 
> Currently, almost all calls to Pipeline's public interface first pass
> through PipelineController. The one exception is that Pipeline calls its
> own Stop() in OnError().
> 
> This CL moves the internal Pipeline's calls to Stop() out to WMPI. This
> gives PipelineController an opportunity to adjust its internal state
> appropriately. It also makes Pipeline easier to reason about, since it
> doesn't change its own state.
> 
> Bug:  695734 
> 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: Ice20a4e93e0d1a2068f195ad9da15763353f8d5d
> Reviewed-on: https://chromium-review.googlesource.com/607176
> Commit-Queue: Thomas Guilbert <tguilbert@chromium.org>
> Reviewed-by: Dan Sanders <sandersd@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#493649}

TBR=sandersd@chromium.org,tguilbert@chromium.org

Change-Id: If08486601d576060b029c056841adc418de0c712
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  695734 
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
Reviewed-on: https://chromium-review.googlesource.com/612080
Reviewed-by: Olga Sharonova <olka@chromium.org>
Commit-Queue: Olga Sharonova <olka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#493683}
[modify] https://crrev.com/42c3d2a9e44b422c0ef4c9516a4011aca229a862/media/base/pipeline.h
[modify] https://crrev.com/42c3d2a9e44b422c0ef4c9516a4011aca229a862/media/base/pipeline_impl.cc
[modify] https://crrev.com/42c3d2a9e44b422c0ef4c9516a4011aca229a862/media/base/pipeline_impl.h
[modify] https://crrev.com/42c3d2a9e44b422c0ef4c9516a4011aca229a862/media/base/pipeline_impl_unittest.cc
[modify] https://crrev.com/42c3d2a9e44b422c0ef4c9516a4011aca229a862/media/blink/webmediaplayer_impl.cc
[modify] https://crrev.com/42c3d2a9e44b422c0ef4c9516a4011aca229a862/media/filters/pipeline_controller.cc
[modify] https://crrev.com/42c3d2a9e44b422c0ef4c9516a4011aca229a862/media/filters/pipeline_controller.h

Project Member

Comment 4 by bugdroid1@chromium.org, Aug 12 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2e591393c960659617b4907c5142f691b3578cce

commit 2e591393c960659617b4907c5142f691b3578cce
Author: Thomas Guilbert <tguilbert@chromium.org>
Date: Sat Aug 12 00:56:38 2017

Reland "Move internal Pipeline::Stop() call to WMPI"

This is a reland of 2ff16cbf883246bb6131412930538bce240125f4
Original change's description:
> Move internal Pipeline::Stop() call to WMPI
> 
> Currently, almost all calls to Pipeline's public interface first pass
> through PipelineController. The one exception is that Pipeline calls its
> own Stop() in OnError().
> 
> This CL moves the internal Pipeline's calls to Stop() out to WMPI. This
> gives PipelineController an opportunity to adjust its internal state
> appropriately. It also makes Pipeline easier to reason about, since it
> doesn't change its own state.
> 
> Bug:  695734 
> 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: Ice20a4e93e0d1a2068f195ad9da15763353f8d5d
> Reviewed-on: https://chromium-review.googlesource.com/607176
> Commit-Queue: Thomas Guilbert <tguilbert@chromium.org>
> Reviewed-by: Dan Sanders <sandersd@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#493649}

Bug:  695734 
Change-Id: I119494df12063b9c9141dac3a0b432ba5478cbcf
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
Reviewed-on: https://chromium-review.googlesource.com/612204
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Commit-Queue: Thomas Guilbert <tguilbert@chromium.org>
Cr-Commit-Position: refs/heads/master@{#493914}
[modify] https://crrev.com/2e591393c960659617b4907c5142f691b3578cce/media/base/pipeline.h
[modify] https://crrev.com/2e591393c960659617b4907c5142f691b3578cce/media/base/pipeline_impl.cc
[modify] https://crrev.com/2e591393c960659617b4907c5142f691b3578cce/media/base/pipeline_impl.h
[modify] https://crrev.com/2e591393c960659617b4907c5142f691b3578cce/media/base/pipeline_impl_unittest.cc
[modify] https://crrev.com/2e591393c960659617b4907c5142f691b3578cce/media/blink/webmediaplayer_impl.cc
[modify] https://crrev.com/2e591393c960659617b4907c5142f691b3578cce/media/filters/pipeline_controller.cc
[modify] https://crrev.com/2e591393c960659617b4907c5142f691b3578cce/media/filters/pipeline_controller.h
[modify] https://crrev.com/2e591393c960659617b4907c5142f691b3578cce/media/test/pipeline_integration_test_base.cc

Status: Verified (was: Assigned)
Demuxer replacement in no longer planned ATM.

Sign in to add a comment