Closed Bug 855064 Opened 11 years ago Closed 11 years ago

Exclude DASH from the build by default


(Core :: Audio/Video, defect)

Not set





(Reporter: cpearce, Assigned: sworkman)



(2 files)

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?
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.
(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.
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)
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+
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.
Assignee: nobody → sworkman
Attachment #732394 - Flags: review?(cpearce)
Attachment #732394 - Flags: review?(cpearce) → review+
Comment on attachment 732076 [details] [diff] [review]
v1.0 Disables DASH build by default; excludes more code via MOZ_DASH
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
You need to log in before you can comment on or make changes to this bug.