855064 - Exclude DASH from the build by default

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Chris Pearce [:cpearce (Not reading bugmail)]]

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.

Comment 1

[Ralph Giles (:rillian)]

Can you document why we're not going to enable it?

Comment 2

[Chris Pearce [:cpearce (Not reading bugmail)]]

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.

Comment 3

[Steve Workman [:sworkman] (INACTIVE)]

(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.

Comment 4

[Ralph Giles (:rillian)]

Thanks for the explanation. Here's hoping those industry drivers don't change their minds again.

Comment 5

[Steve Workman [:sworkman] (INACTIVE)]

Attachment: 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.

Comment 6

[Chris Pearce [:cpearce (Not reading bugmail)]]

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.

Comment 7

[Steve Workman [:sworkman] (INACTIVE)]

Attachment: 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

Comment 8

[Steve Workman [:sworkman] (INACTIVE)]

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

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

Comment 9

[Steve Workman [:sworkman] (INACTIVE)]

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

Comment 10

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/7a9087d86d45
https://hg.mozilla.org/mozilla-central/rev/81d4bd15053d