1314552 - MDSM::mMediaSeekableOnlyInBufferedRanges should be updated by metadata instead of MediaDecoder

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect, P3)

Description

[JW Wang [:jwwang] [:jw_wang]]

http://searchfox.org/mozilla-central/rev/f5c9e9a249637c9abd88754c8963ecb3838475cb/dom/media/MediaInfo.h#503

The flag can be read from metadata.

Comment 1

[JW Wang [:jwwang] [:jw_wang]]

Attachment: Bug 1314552. Part 1 - update mMediaSeekableOnlyInBufferedRanges in OnMetadataRead().

Review commit: https://reviewboard.mozilla.org/r/91264/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/91264/

Comment 2

[JW Wang [:jwwang] [:jw_wang]]

Attachment: Bug 1314552. Part 2 - remove unused canonical.

Review commit: https://reviewboard.mozilla.org/r/91266/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/91266/

Comment 3

[Jean-Yves Avenard [:jya]]

Comment on attachment 8808454 [details]
Bug 1314552. Part 1 - update mMediaSeekableOnlyInBufferedRanges in OnMetadataRead().

https://reviewboard.mozilla.org/r/91264/#review91098

As I mentioned last week, you are here assuming that the seekability is known when metadata is being read.
This is not always the case and is an incorrect assumption.

With chained ogg, the first ogg file will play normally, and this one is seekable.
Only once you've reached the end of the ogg file and go to a chained one, will the media becomes unseekable.

As such, you can't do what you are doing in this change. As you will lose the ability to transition from normal ogg to a chained ogg.

Comment 4

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8808454 [details]
Bug 1314552. Part 1 - update mMediaSeekableOnlyInBufferedRanges in OnMetadataRead().

https://reviewboard.mozilla.org/r/91264/#review91098

http://searchfox.org/mozilla-central/rev/f5c9e9a249637c9abd88754c8963ecb3838475cb/dom/media/MediaDecoderStateMachine.h#819
Isn't that what mMediaSeekable is for?

MDSM::mMediaSeekableOnlyInBufferedRanges used to be mirrored from MediaDecoder::mMediaSeekableOnlyInBufferedRanges which is updated by the metadata. This bug basically changes nothing but removing canonical/mirror and update MDSM::mMediaSeekableOnlyInBufferedRanges directly from the metadata. The logic is basically the same.

Comment 5

[Jean-Yves Avenard [:jya]]

Comment on attachment 8808454 [details]
Bug 1314552. Part 1 - update mMediaSeekableOnlyInBufferedRanges in OnMetadataRead().

https://reviewboard.mozilla.org/r/91264/#review91098

yes and no. mMediaSeekableOnlyInBufferedRanges is currently only used with WebM, and is used when playing webm with no seek cues (like those generated when recording a webrtc session). This one can be determined during AsyncReadMetadata as we can immediately know if the webm container has clues or not.

Then you have the asynchronous call, that can happen at anytime during the lifetime of the reader/demuxer.

https://dxr.mozilla.org/mozilla-central/source/dom/media/ogg/OggDemuxer.cpp#760

This uses the MediaDecoderReader::MediaNotSeekableProducer() mechanism
https://dxr.mozilla.org/mozilla-central/source/dom/media/MediaDecoderReader.h#287

Comment 6

[Jean-Yves Avenard [:jya]]

Comment on attachment 8808454 [details]
Bug 1314552. Part 1 - update mMediaSeekableOnlyInBufferedRanges in OnMetadataRead().

https://reviewboard.mozilla.org/r/91264/#review91108

Oh my bad...

This removal of the mirror of mMediaSeekableOnlyInBufferedRanges is okay.
However, it appears that MDSM::OnMediaNotSeekable() is no longer connected, that will break chained ogg

Comment 7

[Jean-Yves Avenard [:jya]]

Comment on attachment 8808454 [details]
Bug 1314552. Part 1 - update mMediaSeekableOnlyInBufferedRanges in OnMetadataRead().

https://reviewboard.mozilla.org/r/91264/#review91110

Comment 8

[Jean-Yves Avenard [:jya]]

Comment on attachment 8808455 [details]
Bug 1314552. Part 2 - remove unused canonical.

https://reviewboard.mozilla.org/r/91266/#review91112

Comment 9

[JW Wang [:jwwang] [:jw_wang]]

Thanks!

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8b82e11c3127
Part 1 - update mMediaSeekableOnlyInBufferedRanges in OnMetadataRead(). r=jya
https://hg.mozilla.org/integration/autoland/rev/deb2dcbb0ec0
Part 2 - remove unused canonical. r=jya

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/8b82e11c3127
https://hg.mozilla.org/mozilla-central/rev/deb2dcbb0ec0