1092996 - MSE/WebM: loadedmetadata event not fired under some circumstances

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Jean-Yves Avenard [:jya]]

With bug 1065827, the loadedmetadata event should be fired as soon as the webm init segment is appended.
However, this doesn't happen, TrackBuffer::InitializeDecoder fails to properly initialize the decoder with just the webm init segment.

WebMReader::ReadMetadata will fail.

This is due nestegg_init that reads until the first cluster element, an MSE init segment will be just short of that, so nestegg_init will be blocked until another append.

Potential solution would be to provide nestegg_init the size of the data we have already received as maximum offset,

Comment 1

[Jean-Yves Avenard [:jya]]

Attachment: WebM: limit size read to what is available

Limit the size read to what is available. This approach turned out to work nicely with all my tests.

Comment 2

[Matthew Gregan [:kinetik]]

Comment on attachment 8516252 [details] [diff] [review]
WebM: limit size read to what is available

Review of attachment 8516252 [details] [diff] [review]:
-----------------------------------------------------------------

I think this needs to be MSE-only, a regular media stream should be permitted to block waiting for more data here (and without it, I'd expect to see random media load failures).

Comment 3

[Jean-Yves Avenard [:jya]]

FWIW: it has passed all mochitests...

Where WebMReader::ReadMetadata is called is during initialisation, and where the MediaDecoderStateMachine is in DECODING_METADATA state, if there's not enough data there and WebMReader::ReadMetadata fails, the state machine will go back to WAIT_FOR_RESOURCES state, and once new data is received will once again change to DECODING_METADATA and try again.

As such, I don't believe not blocking for more data is a problem, the system will retry automatically.

Comment 4

[Matthew Gregan [:kinetik]]

(In reply to Jean-Yves Avenard [:jya] from comment #3)
> FWIW: it has passed all mochitests...

They aren't likely to trigger the bug as they're mostly running with local files, and those that aren't are using a web server on the same machine that has no transfer rate limits and transfers 64kB chunks.

> Where WebMReader::ReadMetadata is called is during initialisation, and where
> the MediaDecoderStateMachine is in DECODING_METADATA state, if there's not
> enough data there and WebMReader::ReadMetadata fails, the state machine will
> go back to WAIT_FOR_RESOURCES state, and once new data is received will once
> again change to DECODING_METADATA and try again.

That mechanism relies on the reader returning NS_OK despite not having completed initialization, right?

So if nestegg_init returns failure (e.g. if it hasn't parsed far enough to see the Tracks element or some other element required for initialization), the media will fail to load and end up firing DecodeError().  As far as I understand, it's not safe to call nestegg_init with a specified maxOffset unless you are either certain sufficient data is already available or you're prepared for the reader to permanently fail to initialize.  A call originated by MSE has the information available to satisfy the first of those requirements, but a regular media stream does not.

Comment 5

[Jean-Yves Avenard [:jya]]

Attachment: MSE/WebM: limit size read to what is available

Limiting new behaviour to MSE. Had trouble finding an appropriate function name, happy to be corrected there.

Comment 6

[Matthew Gregan [:kinetik]]

Comment on attachment 8516333 [details] [diff] [review]
MSE/WebM: limit size read to what is available

Review of attachment 8516333 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!

::: dom/media/AbstractMediaDecoder.h
@@ +129,5 @@
>    virtual void SetElementVisibility(bool aIsVisible) {}
>  
> +  // Called by some MediaDecoderReader to determine if we can rely
> +  // on the resource length to limit reads.
> +  virtual bool IsResourceLengthUsable() { return false; }

I can't think of a great name for this either.  Maybe HasInitializationData(), since that's effectively the question we want to answer?

Comment 7

[Jean-Yves Avenard [:jya]]

Attachment: MSE/WebM: limit size read to what is available

Rename new member function to HasInitializationData as suggested

Comment 8

[Jean-Yves Avenard [:jya]]

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=c4db27d2d075

Comment 9

[Jean-Yves Avenard [:jya]]

Attachment: MSE/WebM: limit size read to what is available

rebase

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/fd98b545a4b9

Comment 11

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

https://hg.mozilla.org/mozilla-central/rev/fd98b545a4b9