Closed
Bug 1288329
Opened 8 years ago
Closed 8 years ago
When transitioning over chained ogg; new metadataloaded event should be fired.
Categories
(Core :: Audio/Video: Playback, defect)
Core
Audio/Video: Playback
Tracking
()
RESOLVED
FIXED
mozilla50
Tracking | Status | |
---|---|---|
firefox50 | --- | fixed |
People
(Reporter: jya, Assigned: jya)
References
(Depends on 1 open bug)
Details
Attachments
(8 files)
58 bytes,
text/x-review-board-request
|
Details | |
58 bytes,
text/x-review-board-request
|
Details | |
58 bytes,
text/x-review-board-request
|
Details | |
58 bytes,
text/x-review-board-request
|
Details | |
58 bytes,
text/x-review-board-request
|
Details | |
58 bytes,
text/x-review-board-request
|
mozbugz
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
mozbugz
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
mozbugz
:
review+
|
Details |
see dom/media/test/test_chaining.html Need to amend MediaFormatReader to handle chaining
Chris - do we need to continue chaining support?
Flags: needinfo?(cpearce)
Assignee | ||
Comment 2•8 years ago
|
||
Decided that we will only support chaining audio files, however the new metadataloaded won't be fired at each transition (which is against the spec anyway)
Assignee | ||
Updated•8 years ago
|
Flags: needinfo?(cpearce)
Comment 3•8 years ago
|
||
Is this something where we should try to get the spec changed? Seems like this is needed for song title changes in web radio. I guess we probably don't support ICY metadata updates for MP3, either?
Flags: needinfo?(jyavenard)
Comment 4•8 years ago
|
||
(In reply to Anthony Jones (:kentuckyfriedtakahe, :k17e) from comment #1) > Chris - do we need to continue chaining support? In Firefox 47, 0.02% of all media elements used report that they were used for chained ogg playback: https://mzl.la/2ahsCBE It's pretty easy in the new media format reader to keep supporting the minimal subset of chaining that we support. The only part that requires effort to support is re-firing loaded metadata when changing links. I'd be happy to lose that. We shouldn't remove chaining without warning. We should add webconsole logging when Ogg chaining is encountered to encourage webdevs to switch from chained oggs to WebM+MSE. (In reply to Timothy B. Terriberry (:derf) from comment #3) > Is this something where we should try to get the spec changed? Seems like > this is needed for song title changes in web radio. Streaming radio should be using MSE. Ogg chaining never took off, and we shouldn't be encouraging it to. > I guess we probably don't support ICY metadata updates for MP3, either? Yes, and I'm not aware of anyone asking for it. Are you aware of anyone asking for it?
Comment 5•8 years ago
|
||
> > I guess we probably don't support ICY metadata updates for MP3, either?
>
> Yes, and I'm not aware of anyone asking for it. Are you aware of anyone asking for it?
I'm not. Everything you say makes sense. Thanks!
Flags: needinfo?(jyavenard)
Assignee | ||
Comment 6•8 years ago
|
||
So I added the metadata support to the ogg demuxer yesterday, I kind of like the support of MozMetadata, too bad it didn't take off. On second thought, replicating the behaviour of the OggReader when it comes to metadata could be easily hacked off by having the demuxer itself firing the notification event to the decoder (JW made that very easy with his EventNotifier class). Extending to do the same in MP3 would be trivial too, just saying.
Comment 7•8 years ago
|
||
If it's cheap, go for it.
Assignee | ||
Comment 8•8 years ago
|
||
This is not the cleanest approach, but ensures identical behavior with the OggReader when it comes to firing loadedmetadata event and handling the change of seekability. A more universal solution could be considered involving the MediaFormatReader and changing the MediaDataDemuxer API, of interest would be adding support for a new event fired whenever we have a change of content or metadata (useful with MSE or recorded webm of a WebRTC session Review commit: https://reviewboard.mozilla.org/r/67614/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/67614/
Attachment #8775430 -
Flags: review?(cpearce)
Assignee | ||
Comment 9•8 years ago
|
||
Now passes all media mochitests. Review commit: https://reviewboard.mozilla.org/r/67724/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/67724/
Attachment #8775430 -
Attachment description: Bug 1288329: [ogg] Add support for metadata chaining in OggDemuxer. → Bug 1288329: [ogg] P1. Add support for metadata chaining in OggDemuxer.
Attachment #8775540 -
Flags: review?(cpearce)
Assignee | ||
Comment 10•8 years ago
|
||
Comment on attachment 8775430 [details] Bug 1288329: [ogg] P1. Add support for metadata chaining in OggDemuxer. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/67614/diff/1-2/
Assignee | ||
Comment 11•8 years ago
|
||
The OggReader always passed a complete ogg_packet to the vorbis decoder, ensuring that the right number of frames was be returned. In the conversion to the new architecture, this information got lost making the vorbis decoder always return more frames than normal on the last packet. Review commit: https://reviewboard.mozilla.org/r/67786/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/67786/
Attachment #8775540 -
Attachment description: Bug 1288329: [ogg] P2. Enable new OggDemuxer by default. → Bug 1288329: [ogg] P4. Enable new OggDemuxer by default.
Attachment #8775636 -
Flags: review?(cpearce)
Attachment #8775637 -
Flags: review?(cpearce)
Assignee | ||
Comment 12•8 years ago
|
||
The old OggReader only use the end time as duration of the video, ignoring the start time of the first sample. This leads to incorrect duration calculation. Review commit: https://reviewboard.mozilla.org/r/67788/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/67788/
Assignee | ||
Comment 13•8 years ago
|
||
Comment on attachment 8775430 [details] Bug 1288329: [ogg] P1. Add support for metadata chaining in OggDemuxer. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/67614/diff/2-3/
Assignee | ||
Comment 14•8 years ago
|
||
Comment on attachment 8775540 [details] Bug 1288329: [ogg] P8. Enable new OggDemuxer by default. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/67724/diff/1-2/
Assignee | ||
Comment 15•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/67886/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/67886/
Attachment #8775637 -
Attachment description: Bug 1288329: [ogg] P3. Fix mochitest. → Bug 1288329: [ogg] P5. Adjust mochitests.
Attachment #8775540 -
Attachment description: Bug 1288329: [ogg] P4. Enable new OggDemuxer by default. → Bug 1288329: [ogg] P6. Enable new OggDemuxer by default.
Attachment #8775838 -
Flags: review?(cpearce)
Attachment #8775839 -
Flags: review?(gsquelart)
Assignee | ||
Comment 16•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/67888/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/67888/
Assignee | ||
Comment 17•8 years ago
|
||
Comment on attachment 8775430 [details] Bug 1288329: [ogg] P1. Add support for metadata chaining in OggDemuxer. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/67614/diff/3-4/
Assignee | ||
Comment 18•8 years ago
|
||
Comment on attachment 8775636 [details] Bug 1288329: [ogg/vorbis] P2. Pass extra information to the decoder so that it can perform proper trimmer. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/67786/diff/1-2/
Assignee | ||
Comment 19•8 years ago
|
||
Comment on attachment 8775637 [details] Bug 1288329: [ogg] P6. Adjust mochitests. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/67788/diff/1-2/
Assignee | ||
Comment 20•8 years ago
|
||
Comment on attachment 8775540 [details] Bug 1288329: [ogg] P8. Enable new OggDemuxer by default. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/67724/diff/2-3/
Comment on attachment 8775839 [details] Bug 1288329: [ogg] P5. Fix coding style. https://reviewboard.mozilla.org/r/67888/#review64946
Attachment #8775839 -
Flags: review?(gsquelart) → review+
Assignee | ||
Comment 22•8 years ago
|
||
Comment on attachment 8775430 [details] Bug 1288329: [ogg] P1. Add support for metadata chaining in OggDemuxer. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/67614/diff/4-5/
Assignee | ||
Comment 23•8 years ago
|
||
Comment on attachment 8775636 [details] Bug 1288329: [ogg/vorbis] P2. Pass extra information to the decoder so that it can perform proper trimmer. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/67786/diff/2-3/
Assignee | ||
Comment 24•8 years ago
|
||
Comment on attachment 8775838 [details] Bug 1288329: [ogg] P3. Never take into considerations frames prior the first keyframe. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/67886/diff/1-2/
Assignee | ||
Comment 25•8 years ago
|
||
Comment on attachment 8775839 [details] Bug 1288329: [ogg] P5. Fix coding style. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/67888/diff/1-2/
Assignee | ||
Comment 26•8 years ago
|
||
Comment on attachment 8775637 [details] Bug 1288329: [ogg] P6. Adjust mochitests. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/67788/diff/2-3/
Assignee | ||
Comment 27•8 years ago
|
||
Comment on attachment 8775540 [details] Bug 1288329: [ogg] P8. Enable new OggDemuxer by default. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/67724/diff/3-4/
Comment on attachment 8775430 [details] Bug 1288329: [ogg] P1. Add support for metadata chaining in OggDemuxer. https://reviewboard.mozilla.org/r/67614/#review64982 r+ with nits: ::: dom/media/ogg/OggDecoder.cpp:21 (Diff revision 5) > + useFormatDecoder ? new OggDemuxer(GetResource()) : nullptr; > RefPtr<MediaDecoderReader> reader = useFormatDecoder ? > - static_cast<MediaDecoderReader*>(new MediaFormatReader(this, new OggDemuxer(GetResource()), GetVideoFrameContainer())) : > + static_cast<MediaDecoderReader*>(new MediaFormatReader(this, demuxer, GetVideoFrameContainer())) : > new OggReader(this); Style: use 2 spaces indent instead of 4. ::: dom/media/ogg/OggDemuxer.cpp:167 (Diff revision 5) > + > + Double empty lines, is that intended?
Attachment #8775430 -
Flags: review+
Attachment #8775636 -
Flags: review+
Comment on attachment 8775636 [details] Bug 1288329: [ogg/vorbis] P2. Pass extra information to the decoder so that it can perform proper trimmer. https://reviewboard.mozilla.org/r/67786/#review64986
Comment on attachment 8775838 [details] Bug 1288329: [ogg] P3. Never take into considerations frames prior the first keyframe. https://reviewboard.mozilla.org/r/67886/#review64988 r+ with nit: In commit description: 'prior the' -> 'prior to the'.
Attachment #8775838 -
Flags: review+
Comment on attachment 8775637 [details] Bug 1288329: [ogg] P6. Adjust mochitests. https://reviewboard.mozilla.org/r/67788/#review64992
Attachment #8775637 -
Flags: review+
Comment on attachment 8775540 [details] Bug 1288329: [ogg] P8. Enable new OggDemuxer by default. https://reviewboard.mozilla.org/r/67724/#review64996
Attachment #8775540 -
Flags: review+
Comment 33•8 years ago
|
||
Comment on attachment 8775430 [details] Bug 1288329: [ogg] P1. Add support for metadata chaining in OggDemuxer. https://reviewboard.mozilla.org/r/67614/#review65006 ::: dom/media/MediaDecoderReader.h:294 (Diff revision 5) > } > > // Notified by the OggReader during playback when chained ogg is detected. > MediaEventSource<void>& OnMediaNotSeekable() { return mOnMediaNotSeekable; } > > + TimedMetadataEventProducer& TimedMetadataProducer() Return a raw ponter so you don't need NotNull<> below. ::: dom/media/ogg/OggDecoder.cpp:26 (Diff revision 5) > + useFormatDecoder ? new OggDemuxer(GetResource()) : nullptr; > RefPtr<MediaDecoderReader> reader = useFormatDecoder ? > - static_cast<MediaDecoderReader*>(new MediaFormatReader(this, new OggDemuxer(GetResource()), GetVideoFrameContainer())) : > + static_cast<MediaDecoderReader*>(new MediaFormatReader(this, demuxer, GetVideoFrameContainer())) : > new OggReader(this); > + if (useFormatDecoder) { > + demuxer->SetChainingEvents(WrapNotNull(&reader->TimedMetadataProducer()), SetChainingEvents() is regarded as part of the initialization of the demuxer. Please move the call above |new MediaFormatReader| so MFR has no chance to use a half-baked object. ::: dom/media/ogg/OggDecoder.cpp:27 (Diff revision 5) > RefPtr<MediaDecoderReader> reader = useFormatDecoder ? > - static_cast<MediaDecoderReader*>(new MediaFormatReader(this, new OggDemuxer(GetResource()), GetVideoFrameContainer())) : > + static_cast<MediaDecoderReader*>(new MediaFormatReader(this, demuxer, GetVideoFrameContainer())) : > new OggReader(this); > + if (useFormatDecoder) { > + demuxer->SetChainingEvents(WrapNotNull(&reader->TimedMetadataProducer()), > + WrapNotNull(&reader->MediaNotSeekableProducer())); Why not just passing raw pointers? I can't see how NotNull<> helps here.
Attachment #8775430 -
Flags: review+
Comment 34•8 years ago
|
||
Comment on attachment 8775636 [details] Bug 1288329: [ogg/vorbis] P2. Pass extra information to the decoder so that it can perform proper trimmer. https://reviewboard.mozilla.org/r/67786/#review65026 ::: dom/media/MediaData.h:661 (Diff revision 3) > const CryptoSample& mCrypto; > RefPtr<MediaByteBuffer> mExtraData; > > + // Used by the Vorbis decoder and Ogg demuxer. > + // Indicates that this is the last packet of the stream. > + bool mEOS; I would prefer in-class initialization so you don't need to change every constructor when adding a new member.
Attachment #8775636 -
Flags: review+
Comment 35•8 years ago
|
||
Comment on attachment 8775839 [details] Bug 1288329: [ogg] P5. Fix coding style. https://reviewboard.mozilla.org/r/67888/#review65028 ::: dom/media/ogg/OggDemuxer.h:94 (Diff revision 2) > SeekRange(int64_t aOffsetStart, > int64_t aOffsetEnd, > int64_t aTimeStart, > int64_t aTimeEnd) > - : mOffsetStart(aOffsetStart), > - mOffsetEnd(aOffsetEnd), > + : mOffsetStart(aOffsetStart) > + , mOffsetEnd(aOffsetEnd) 2 spaces between "," and "mOffsetEnd".
Attachment #8775839 -
Flags: review+
Assignee | ||
Comment 36•8 years ago
|
||
https://reviewboard.mozilla.org/r/67614/#review65006 > SetChainingEvents() is regarded as part of the initialization of the demuxer. Please move the call above |new MediaFormatReader| so MFR has no chance to use a half-baked object. How would that be possible as prior new MediaFormatReader the objects we want to pass to the OggDemuxer do not yet exist ? > Why not just passing raw pointers? I can't see how NotNull<> helps here. I felt like using something new and so I don't have to put assert in the OggDemuxer :)
Assignee | ||
Comment 37•8 years ago
|
||
https://reviewboard.mozilla.org/r/67786/#review65026 > I would prefer in-class initialization so you don't need to change every constructor when adding a new member. not sure I follow. could you provide an example? MediaRawData isn't supposed to change... it's unfortunate that I found no other way to pass that required data.
Comment 38•8 years ago
|
||
https://reviewboard.mozilla.org/r/67614/#review65006 > How would that be possible as prior new MediaFormatReader the objects we want to pass to the OggDemuxer do not yet exist ? I mean: 1. RefPtr<OggDemuxer> demuxer = new OggDemuxex(...); 2. demuxer->SetChainingEvents(...); 3. reader = new MediaFormatReader(this, demuxer, ...);
Comment 39•8 years ago
|
||
https://reviewboard.mozilla.org/r/67786/#review65026 > not sure I follow. could you provide an example? > > MediaRawData isn't supposed to change... it's unfortunate that I found no other way to pass that required data. bool mEOS = false; So you don't need to init the member in every constructor.
Comment 40•8 years ago
|
||
https://reviewboard.mozilla.org/r/67614/#review65048 ::: dom/media/ogg/OggDemuxer.cpp:714 (Diff revision 5) > if (mIsChained) { > return; > } > mIsChained = true; > } > - // @FIXME how can MediaDataDemuxer / MediaTrackDemuxer notify this has changed? > + if (mOnSeekableEvent) { You won't need null-check if the member is declared as NotNull<T>.
Assignee | ||
Comment 41•8 years ago
|
||
https://reviewboard.mozilla.org/r/67614/#review65006 > I mean: > 1. RefPtr<OggDemuxer> demuxer = new OggDemuxex(...); > 2. demuxer->SetChainingEvents(...); > 3. reader = new MediaFormatReader(this, demuxer, ...); How can you call the demuxer with parameters found in the reader if the reader hasn't been created yet...
Comment 42•8 years ago
|
||
https://reviewboard.mozilla.org/r/67614/#review65006 > How can you call the demuxer with parameters found in the reader if the reader hasn't been created yet... Oops, I see what you mean. Forget about this non-sense.
Assignee | ||
Updated•8 years ago
|
Attachment #8775430 -
Flags: review?(cpearce)
Attachment #8775636 -
Flags: review?(cpearce)
Attachment #8775838 -
Flags: review?(cpearce)
Attachment #8775637 -
Flags: review?(cpearce)
Attachment #8775540 -
Flags: review?(cpearce)
Assignee | ||
Comment 43•8 years ago
|
||
Comment on attachment 8775430 [details] Bug 1288329: [ogg] P1. Add support for metadata chaining in OggDemuxer. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/67614/diff/5-6/
Attachment #8775430 -
Flags: review?(cpearce)
Attachment #8775430 -
Flags: review+
Attachment #8775636 -
Flags: review?(cpearce)
Attachment #8775636 -
Flags: review+
Attachment #8775838 -
Flags: review+ → review?(cpearce)
Attachment #8775839 -
Flags: review+
Attachment #8775637 -
Flags: review+ → review?(cpearce)
Attachment #8775540 -
Flags: review+ → review?(cpearce)
Assignee | ||
Comment 44•8 years ago
|
||
Comment on attachment 8775636 [details] Bug 1288329: [ogg/vorbis] P2. Pass extra information to the decoder so that it can perform proper trimmer. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/67786/diff/3-4/
Assignee | ||
Comment 45•8 years ago
|
||
Comment on attachment 8775838 [details] Bug 1288329: [ogg] P3. Never take into considerations frames prior the first keyframe. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/67886/diff/2-3/
Assignee | ||
Comment 46•8 years ago
|
||
Comment on attachment 8775839 [details] Bug 1288329: [ogg] P5. Fix coding style. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/67888/diff/2-3/
Assignee | ||
Comment 47•8 years ago
|
||
Comment on attachment 8775637 [details] Bug 1288329: [ogg] P6. Adjust mochitests. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/67788/diff/3-4/
Assignee | ||
Comment 48•8 years ago
|
||
Comment on attachment 8775540 [details] Bug 1288329: [ogg] P8. Enable new OggDemuxer by default. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/67724/diff/4-5/
Assignee | ||
Updated•8 years ago
|
Attachment #8775430 -
Flags: review?(cpearce)
Attachment #8775636 -
Flags: review?(cpearce)
Attachment #8775838 -
Flags: review?(cpearce) → review?(jwwang)
Attachment #8775637 -
Flags: review?(cpearce) → review?(jwwang)
Attachment #8775540 -
Flags: review?(cpearce) → review?(jwwang)
Assignee | ||
Updated•8 years ago
|
Attachment #8775838 -
Flags: review?(jwwang)
Attachment #8775637 -
Flags: review?(jwwang)
Attachment #8775540 -
Flags: review?(jwwang)
Assignee | ||
Comment 49•8 years ago
|
||
Comment on attachment 8775430 [details] Bug 1288329: [ogg] P1. Add support for metadata chaining in OggDemuxer. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/67614/diff/6-7/
Assignee | ||
Comment 50•8 years ago
|
||
Comment on attachment 8775636 [details] Bug 1288329: [ogg/vorbis] P2. Pass extra information to the decoder so that it can perform proper trimmer. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/67786/diff/4-5/
Assignee | ||
Comment 51•8 years ago
|
||
Comment on attachment 8775838 [details] Bug 1288329: [ogg] P3. Never take into considerations frames prior the first keyframe. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/67886/diff/3-4/
Assignee | ||
Comment 52•8 years ago
|
||
Comment on attachment 8775839 [details] Bug 1288329: [ogg] P5. Fix coding style. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/67888/diff/3-4/
Assignee | ||
Comment 53•8 years ago
|
||
Comment on attachment 8775637 [details] Bug 1288329: [ogg] P6. Adjust mochitests. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/67788/diff/4-5/
Assignee | ||
Comment 54•8 years ago
|
||
Comment on attachment 8775540 [details] Bug 1288329: [ogg] P8. Enable new OggDemuxer by default. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/67724/diff/5-6/
Comment 55•8 years ago
|
||
Pushed by jyavenard@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/a76b6e559777 [ogg] P1. Add support for metadata chaining in OggDemuxer. r=gerald,jwwang https://hg.mozilla.org/integration/autoland/rev/a1c4d9b9de32 [ogg/vorbis] P2. Pass extra information to the decoder so that it can perform proper trimmer. r=gerald,jwwang https://hg.mozilla.org/integration/autoland/rev/936b73eb37c6 [ogg] P3. Never take into considerations frames prior the first keyframe. r=gerald https://hg.mozilla.org/integration/autoland/rev/1406c14098f1 [ogg] P4. Fix coding style. r=gerald,jwwang https://hg.mozilla.org/integration/autoland/rev/49ea83d00a77 [ogg] P5. Adjust mochitests. r=gerald https://hg.mozilla.org/integration/autoland/rev/f33cb032cc9f [ogg] P6. Enable new OggDemuxer by default. r=gerald
I had to back these out in https://hg.mozilla.org/integration/autoland/rev/3f66f98ebf11 for failures like https://treeherder.mozilla.org/logviewer.html#?job_id=1162009&repo=autoland
Flags: needinfo?(jyavenard)
Other failures include https://treeherder.mozilla.org/logviewer.html#?job_id=1161044&repo=autoland
Assignee | ||
Comment 58•8 years ago
|
||
We can seek in cached data, we will rely on the seek operation to fail instead to determine if we can't or not Review commit: https://reviewboard.mozilla.org/r/68160/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/68160/
Attachment #8775839 -
Attachment description: Bug 1288329: [ogg] P4. Fix coding style. → Bug 1288329: [ogg] P5. Fix coding style.
Attachment #8775637 -
Attachment description: Bug 1288329: [ogg] P5. Adjust mochitests. → Bug 1288329: [ogg] P6. Adjust mochitests.
Attachment #8775540 -
Attachment description: Bug 1288329: [ogg] P6. Enable new OggDemuxer by default. → Bug 1288329: [ogg] P8. Enable new OggDemuxer by default.
Attachment #8776363 -
Flags: review?(gsquelart)
Attachment #8776364 -
Flags: review?(gsquelart)
Assignee | ||
Comment 59•8 years ago
|
||
The ogg file contains a theora video track with a flac audio track, not vorbis. The new OggDemuxer properly ignore the tracks it knows nothing about. This will cause the tests to use MP4 with h264/aac instead which isn't available on Windows XP, so we mark those tests are expected to fail. Review commit: https://reviewboard.mozilla.org/r/68162/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/68162/
Assignee | ||
Comment 60•8 years ago
|
||
Comment on attachment 8775430 [details] Bug 1288329: [ogg] P1. Add support for metadata chaining in OggDemuxer. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/67614/diff/7-8/
Assignee | ||
Comment 61•8 years ago
|
||
Comment on attachment 8775636 [details] Bug 1288329: [ogg/vorbis] P2. Pass extra information to the decoder so that it can perform proper trimmer. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/67786/diff/5-6/
Assignee | ||
Comment 62•8 years ago
|
||
Comment on attachment 8775838 [details] Bug 1288329: [ogg] P3. Never take into considerations frames prior the first keyframe. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/67886/diff/4-5/
Assignee | ||
Comment 63•8 years ago
|
||
Comment on attachment 8775839 [details] Bug 1288329: [ogg] P5. Fix coding style. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/67888/diff/4-5/
Assignee | ||
Comment 64•8 years ago
|
||
Comment on attachment 8775637 [details] Bug 1288329: [ogg] P6. Adjust mochitests. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/67788/diff/5-6/
Assignee | ||
Comment 65•8 years ago
|
||
Comment on attachment 8775540 [details] Bug 1288329: [ogg] P8. Enable new OggDemuxer by default. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/67724/diff/6-7/
Comment on attachment 8776363 [details] Bug 1288329: [ogg] P4. Don't assume we can't seek in resource if transport isn't seekable. https://reviewboard.mozilla.org/r/68160/#review65260
Attachment #8776363 -
Flags: review?(gsquelart) → review+
Comment on attachment 8776364 [details] Bug 1288329: [webref] P7. Fix mimetype. https://reviewboard.mozilla.org/r/68162/#review65262 r+
Attachment #8776364 -
Flags: review?(gsquelart) → review+
Comment 68•8 years ago
|
||
Pushed by jyavenard@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/725b6c6ff7f6 [ogg] P1. Add support for metadata chaining in OggDemuxer. r=gerald,jwwang https://hg.mozilla.org/integration/autoland/rev/b1f32a906af2 [ogg/vorbis] P2. Pass extra information to the decoder so that it can perform proper trimmer. r=gerald,jwwang https://hg.mozilla.org/integration/autoland/rev/f9dec37d375c [ogg] P3. Never take into considerations frames prior the first keyframe. r=gerald https://hg.mozilla.org/integration/autoland/rev/e49d5a02100e [ogg] P4. Don't assume we can't seek in resource if transport isn't seekable. r=gerald https://hg.mozilla.org/integration/autoland/rev/aced758afc8a [ogg] P5. Fix coding style. r=gerald,jwwang https://hg.mozilla.org/integration/autoland/rev/6e1765d4c1c1 [ogg] P6. Adjust mochitests. r=gerald https://hg.mozilla.org/integration/autoland/rev/f3e86b019fbf [webref] P7. Fix mimetype. r=gerald https://hg.mozilla.org/integration/autoland/rev/de5f049f1029 [ogg] P8. Enable new OggDemuxer by default. r=gerald
Comment 69•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/725b6c6ff7f6 https://hg.mozilla.org/mozilla-central/rev/b1f32a906af2 https://hg.mozilla.org/mozilla-central/rev/f9dec37d375c https://hg.mozilla.org/mozilla-central/rev/e49d5a02100e https://hg.mozilla.org/mozilla-central/rev/aced758afc8a https://hg.mozilla.org/mozilla-central/rev/6e1765d4c1c1 https://hg.mozilla.org/mozilla-central/rev/f3e86b019fbf https://hg.mozilla.org/mozilla-central/rev/de5f049f1029
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
Assignee | ||
Updated•8 years ago
|
Flags: needinfo?(jyavenard)
Updated•7 years ago
|
Updated•2 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•