1086525 - ContainerParser doesn't do a very good job of determining whether it has all of the init data

Status: RESOLVED FIXED

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

Description

[Bobby Holley (:bholley)]

Right now the ContainerParser implementations check for a couple of magic bytes at the start of the chunk, and then permanently store that chunk (whatever its size) as the canonical initialization data. TrackBuffer then assumes that it can create a brand new decoder, pass in this (potentially-incomplete) initialization chunk, and then append arbitrary media data. I don't know a ton about either webm or mp4, but I'd hazard a guess that the decoders will throw if they receive only 4 bytes of initialization data.

In the current behavior, this bug should manifest itself even in the case where  the complete data stream is appended in order, since it's purely an effect of the chunking.

Here's an untested sketch of some tests that I believe will fail with our current implementation:

var webMData = someUInt8Array;
function testChunking(firstChunkBoundary) {
  var ms = new MediaSource();
  var video = document.createElement('video');
  document.body.appendChild(video);
  video.src = window.URL.createObjectURL(ms);
  ms.addEventListener('sourceopen', function(e) {
    var sourceBuffer = ms.addSourceBuffer('video/webm; codecs="vorbis,vp8"');
    sourceBuffer.appendBuffer(webMData.subArray(0, firstChunkBoundary));
    sourceBuffer.appendBuffer(webMData.subArray(firstChunkBoundary, webMData.length));
  }, false);
}
testChunking(2);
testChunking(4);
testChunking(20);

Comment 1

[Bobby Holley (:bholley)]

Anthony - Should I just file stuff like this as blocking bug 778617, or is there anything else I need to do to get it on the radar? Not sure if this is already covered somewhere - feel free to dupe it if it is.

Comment 2

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Yes. Just block that bug and we can move them around as necessary. If you think bugs should block enabling MSE on MP4 then make them block 1086525. We should block that on bugs that break YouTube or the DASHIF player. I don't think this one does.

Comment 3

[Bobby Holley (:bholley)]

(In reply to Anthony Jones (:kentuckyfriedtakahe, :k17e) from comment #2)
> Yes. Just block that bug and we can move them around as necessary. If you
> think bugs should block enabling MSE on MP4

Do you mean in the sense of supporting MP4 when MSE is enabled, or shipping MSE to the web?

> then make them block 1086525.

That's this bug. Did you mean bug 1081817, or something else?

> We
> should block that on bugs that break YouTube or the DASHIF player. I don't
> think this one does.

Discussed this with jst. We both agree that we shouldn't ship a basic spec compliance issue like this without having a serious discussion about it. Is blocking bug 778617 enough to make sure that discussion happens, or is there a different radar I should target?

Comment 4

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

(In reply to Bobby Holley (:bholley) from comment #3)
> Do you mean in the sense of supporting MP4 when MSE is enabled, or shipping
> MSE to the web?

I mean in the sense of blocking us enabling MSE for MP4 on nightly.

> That's this bug. Did you mean bug 1081817, or something else?

Yes.

> Discussed this with jst. We both agree that we shouldn't ship a basic spec
> compliance issue like this without having a serious discussion about it. Is
> blocking bug 778617 enough to make sure that discussion happens, or is there
> a different radar I should target?

778617 is a catch all so it is not likely to insight a discussion. Things that break YouTube and the DASHIF player are higher priority than things that don't. This doesn't break the DASHIF player for sure and I don't believe it breaks YouTube.

Comment 5

[Bobby Holley (:bholley)]

(In reply to Anthony Jones (:kentuckyfriedtakahe, :k17e) from comment #4)
> I mean in the sense of blocking us enabling MSE for MP4 on nightly.

Ok. Shipping an incomplete implementation on Nightly for the purposes of YouTube feedback is probably fine.

> 778617 is a catch all so it is not likely to insight a discussion.

Ok. Is there another bug? Should we make one, or add a bugzilla flag or a whiteboard milestone or something?

> Things
> that break YouTube and the DASHIF player are higher priority than things
> that don't. This doesn't break the DASHIF player for sure and I don't
> believe it breaks YouTube.

Sure, that's why I'm not working on fixing it at the moment. ;-)

IIUC, Our primary motivation for putting resources into MSE stems from a business interest in YouTube and DASHIF. But we also have certain rules to follow and standards to uphold when shipping features to the Web at large, which is why the eventual "Intent To Ship" announcement will face scrutiny beyond simple compatibility with the above. It seems prudent to catalog these issues as we go along so that we can hit the ground running when it comes time to ship.

Comment 6

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

(In reply to Bobby Holley (:bholley) from comment #5)
> Ok. Is there another bug? Should we make one, or add a bugzilla flag or a
> whiteboard milestone or something?

No. Feel free to add a meta-bug for that.

Comment 7

[Jean-Yves Avenard [:jya]]

this has been fixed by bug 1125776, partial init segment are now properly handled