Closed Bug 1322587 Opened 3 years ago Closed 3 years ago

[MSE] video no longer playing

Categories

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

52 Branch
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
firefox50 --- unaffected
firefox51 --- unaffected
firefox52 --- fixed
firefox53 --- fixed

People

(Reporter: jya, Assigned: jya)

References

(Blocks 1 open bug)

Details

(Keywords: regression)

Attachments

(2 files)

This no longer plays:
http://rdmedia.bbc.co.uk/dash/ondemand/testcard/1/client_manifest-events.mpd

This no longer plays.
The reason for this is that the 2nd segment contains a "gsme" box.

Per spec: https://w3c.github.io/media-source/index.html#sourcebuffer-segment-parser-loop

"If the input buffer contains bytes that violate the SourceBuffer byte stream format specification, then run the append error algorithm and abort this algorithm."

https://w3c.github.io/media-source/isobmff-byte-stream-format.html
"Valid top-level boxes defined in ISO/IEC 14496-12 other than ftyp, moov, styp, moof, and mdat are allowed to appear between the end of an initialization segment or media segment and before the beginning of a new media segment. These boxes MUST be accepted and ignored by the user agent and are not considered part of the media segment in this specification. "

So only valid top-level boxes defined in 14496-12 are allowed to appear in a stream.

"gsme" isn't a valid top-level box as defined in ISO 14496-12

valid top-level boxes are:
      static const mp4_demuxer::AtomType validBoxes[] = {
        "ftyp", "moov", // init segment
        "pdin", "free", "sidx", // optional prior moov box
        "styp", "moof", "mdat", // media segment
        "mfra", "skip", "meta", "meco", "ssix", "prft" // others.
        "pssh", // optional with encrypted EME, though ignored.
      };

Looking at the MediaError.message attribute would have shown exactly what the error was.

So IMHO, this should be closed as invalid.
But it's very likely we're going to encounter those types of bugs in the future (and this was expected as we made it strictly per spec).
Blocks: MSE
This is an emsg box as defined in ISO23009-1:2014 Section 5.10.3.3, which is used to carry in-band metadata in MPEG-DASH streams and whilst, technically, it violates the MSE byte stream format spec we will definitely continue to use this mechanism in our streams and I expect uptake across the industry as solutions mature.

This used to be a problem in Chrome also: https://bugs.chromium.org/p/chromium/issues/detail?id=276303, who ultimately added the box type to the valid boxes list.

I suppose a longer term solution is to get the byte stream spec modified but I don't know how realistic that is at this stage.

Not sure how your parser is returning gsme - that seems like a bug? Apologies for missing the MediaError.message!
I've raised an issue with MSE: https://github.com/w3c/media-source/issues/174
Thank you for the chrome link.
I've added support for the same top-level boxes as chrome supports.
this adds "emsg", "bloc", "uuid" to the list
Comment on attachment 8817650 [details]
Bug 1322587: [MSE] P1. Fix error message.

https://reviewboard.mozilla.org/r/97872/#review98206
Attachment #8817650 - Flags: review?(gsquelart) → review+
Comment on attachment 8817651 [details]
Bug 1322587: [MSE] P2. Allow additional top-level boxes.

https://reviewboard.mozilla.org/r/97874/#review98210
Attachment #8817651 - Flags: review?(gsquelart) → review+
Pushed by jyavenard@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9dfd492a1bf7
[MSE] P1. Fix error message. r=gerald
https://hg.mozilla.org/integration/autoland/rev/0d0995d09911
[MSE] P2. Allow additional top-level boxes. r=gerald
Keywords: regression
Comment on attachment 8817650 [details]
Bug 1322587: [MSE] P1. Fix error message.

bug request is for both patches.

Approval Request Comment
[Feature/Bug causing the regression]: 1314533
[User impact if declined]: DASH/MSE streams not working (including BBC ones)
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: not yet, but will be
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: self-contained to this bug
[Is the change risky?]: no
[Why is the change risky/not risky?]: we add to an existing list of box types allowed. that mechanism has been in nightly for over 60 days.
[String changes made/needed]: none
Attachment #8817650 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/9dfd492a1bf7
https://hg.mozilla.org/mozilla-central/rev/0d0995d09911
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Comment on attachment 8817650 [details]
Bug 1322587: [MSE] P1. Fix error message.

media fix for aurora52
Attachment #8817650 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment on attachment 8817651 [details]
Bug 1322587: [MSE] P2. Allow additional top-level boxes.

media fix for aurora52
Attachment #8817651 - Flags: approval-mozilla-aurora+
Version: unspecified → 52 Branch
qe-verify- per Comment 9
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.