The default bug view has changed. See this FAQ.

Exclude DASH from the build by default

RESOLVED FIXED in mozilla23

Status

()

Core
Audio/Video
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: cpearce, Assigned: sworkman)

Tracking

Trunk
mozilla23
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

4 years ago
Steve has done a lot of great work on adding DASH support to Firefox, and it's a damn shame that we're not going to be enabling it.

We've talked about it amongst the media team here in Auckland, and since we're not going to be turning DASH on, we'd like to disable DASH in the build and let it bitrot.

This is to reduce our maintenance burden; when we do any refactorings we don't want to also have to do them to the DASH backend as well. The DASH backend required quite invasive changes to the MediaDecoder architecture to get working, making MediaDecoder harder for us to refactor and maintain, which we want to do going forward. Disabling DASH will also make our tests run faster, and thus reduce the turn around time of tinderbox and local mochitest runs.

I'm not proposing that we remove the DASH code at this stage, it serves as a good guide for the work on RTSP. However we would like to remove the DASH code after a while if it isn't found to be needed any more.
Can you document why we're not going to enable it?
(Reporter)

Comment 2

4 years ago
I'm not the one who made the decision so I don't have detailed knowledge, as I understand it the industry drivers that were pushing for DASH decided that they'd rather have MediaSource, and there are JS libs out there that can be used to get DASH support anyway.
(Assignee)

Comment 3

4 years ago
(I was writing this comment, but Chris posted before I did - this mostly echoes what he wrote).

Basically, it's a combination of a resource/prioritization issue and momentum growing for Javascript DASH implementations on top of MediaSource. Since development started, the JS style implementations have emerged as more desirable than native DASH (C++ in Gecko) for the big players. We currently don't have the resources to warrant spending more time on having a second approach, so work has been halted.
Thanks for the explanation. Here's hoping those industry drivers don't change their minds again.
(Assignee)

Comment 5

4 years ago
Created attachment 732076 [details] [diff] [review]
v1.0 Disables DASH build by default; excludes more code via MOZ_DASH

I think I got most of the DASH code here. I added some #ifdefs in ChannelMediaResource, one for PrepareToDecode, and WebMReader. Let me know if there's something else you can think of.
Attachment #732076 - Flags: review?(cpearce)
(Reporter)

Comment 6

4 years ago
Comment on attachment 732076 [details] [diff] [review]
v1.0 Disables DASH build by default; excludes more code via MOZ_DASH

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

Thanks Steve.

::: content/media/MediaResource.cpp
@@ +66,5 @@
>      mIsTransportSeekable(true),
>      mSeekOffsetMonitor("media.dashseekmonitor"),
>      mSeekOffset(-1)
> +#else
> +    mIsTransportSeekable(true)

Just move mIsTransportSeekable outside of the MOZ_DASH block, no need to declare it inside and out with the same initializer.
Attachment #732076 - Flags: review?(cpearce) → review+
(Assignee)

Comment 7

4 years ago
Created attachment 732394 [details] [diff] [review]
v1.0 Removes DASH references from media mochitests

Mochitest-1 failed on try yesterday due to references to DASH files that are no longer there. I removed them in this patch and pushed to try again, with M1 passing.

https://tbpl.mozilla.org/?tree=Try&rev=4342ca733525
Assignee: nobody → sworkman
Status: NEW → ASSIGNED
Attachment #732394 - Flags: review?(cpearce)
(Reporter)

Updated

4 years ago
Attachment #732394 - Flags: review?(cpearce) → review+
(Assignee)

Comment 8

4 years ago
Comment on attachment 732394 [details] [diff] [review]
v1.0 Removes DASH references from media mochitests

https://hg.mozilla.org/integration/mozilla-inbound/rev/7a9087d86d45
(Assignee)

Comment 9

4 years ago
Comment on attachment 732076 [details] [diff] [review]
v1.0 Disables DASH build by default; excludes more code via MOZ_DASH

https://hg.mozilla.org/integration/mozilla-inbound/rev/81d4bd15053d
https://hg.mozilla.org/mozilla-central/rev/7a9087d86d45
https://hg.mozilla.org/mozilla-central/rev/81d4bd15053d
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
You need to log in before you can comment on or make changes to this bug.