WebMediaPlayerImpl: err if no A/V in ::OnMetadata #26177
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
MSE worker thread termination or worker removal of SourceBuffers can
race the thread hopping of the OnMetadata signal. If MSE track removals
win the race such that pipeline's reported metadata contains no audio or
video, then when handling that metadata in WMPI, this change issues
player load error instead of transitioning to HAVE_METADATA. This fixes
a crash resulting otherwise because transition to HAVE_METADATA without
MSE pipeline metadata having any A/V previously enabled beginning
playback (in WMPI::Play()) without having a WatchTimeReporter.
I tried reproducing similar failure first using MSE from the main
thread (not MSE-in-Workers) and trying to get removeSourceBuffer to
occur precisely in time to win the race, but was unable to get a repro
with a bit of trying. However, I still suspect this issue could have
pre-existed MSE-in-Workers.
The change is not specific to MSE, since WMPI should prevent successful
playback start if the resource has neither audio nor video metadata.
This change also includes a test note to add a feature to MSE-in-Workers
(feature-detection of MSE-in-Workers support from main/Window context)
and to use it to deflake the test itself which could flakily fail (not
crash as is fixed by this change) on implementations that do not support
MSE-in-Workers yet.
BUG=878133,1139854
TEST=Updated WebMediaPlayerImplTest.NoStreams (blink_media_unittests),
and manually verified on linux locally: flaky crash appears fixed for
.../dedicated-worker/mediasource-worker-play-terminate-worker.html
Change-Id: I8a9d8428417555089c8b09f4bc1e19849bbe0162
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2486074
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Commit-Queue: Matthew Wolenetz <wolenetz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#818731}