Closed Bug 1163076 Opened 9 years ago Closed 9 years ago

add Dailymotion domain name in media.mediasource.whitelist

Categories

(Core :: Audio/Video, defect)

39 Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla41
Tracking Status
firefox38 --- wontfix
firefox38.0.5 --- wontfix
firefox39 --- fixed
firefox40 --- fixed
firefox41 --- fixed

People

(Reporter: g.du.pontavice, Assigned: ajones)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:37.0) Gecko/20100101 Firefox/37.0
Build ID: 20150415140819

Steps to reproduce:

we @ Dailymotion have developed a MSE based media player (using fragmented MP4). we are satisfied with Firefox behavior (with version 39), playback/seeking/quality switching/..., everything is working fine and we have tested with a bunch of videos.
we would like the following domain name to be added in Firefox whitelist so that we can extend and start a beta testing phase, without end-user having to tweak their about:config

 *.dailymotion.com
 *.dmcdn.net



Expected results:

 *.dailymotion.com
 *.dmcdn.net

should be allowed to use MediaSource API if media.mediasource.whitelist is set to true
Component: Untriaged → Video/Audio
Product: Firefox → Core
Assignee: nobody → jyavenard
Assignee: jyavenard → ajones
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #8606113 - Flags: review?(jyavenard) → review+
https://hg.mozilla.org/mozilla-central/rev/2c864aa71b98
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
thanks for pushing those changes.
regarding the target milestone,
does it mean that it will be released in Firefox 41 ? not in release 39/40 ?
thanks in advance for your feedback
Anthony, do you want to uplift the Dailymotion change or let it ride the trains?
Flags: needinfo?(ajones)
Comment on attachment 8606113 [details] [diff] [review]
Whitelist Daily Motion

Approval Request Comment
[Feature/regressing bug #]: not a regression
[User impact if declined]: It will delay Daily Motion from being able to switch away from Flash
[Describe test coverage new/current, TreeHerder]: none
[Risks and why]: A simple change and is low risk. The advantage of uplifting is that Daily Motion can potentially move away from Flash sooner.
[String/UUID change made/needed]: None
Flags: needinfo?(ajones)
Attachment #8606113 - Flags: approval-mozilla-beta?
Attachment #8606113 - Flags: approval-mozilla-aurora?
don't worry we also have a white/black list on our side, if we detect any issues with FF39 or 40 we can disable it easily and fallback on Flash/progressive MP4.
having our hostnames enabled early would help for the beta testing phase, stabilizing both our MSE implem and FF MSE.
Is a beta version of your MSE-based media player available for us to help test?
Hi Chris,
you can check our beta MSE player here: 
http://streambox.fr/mse/hls.js/demo/
basically this is a simple video element with a lib on top transmuxing HLS/MPEG2-TS into fMP4, and feeding MSE SourceBuffer.
this test video is not from Dailymotion, I can give you an access to more videos but protected with login/password.

basic things are working well,
however we recently implemented instant quality switching (in order to accomplish this, we are pausing the video, then flushing the whole sourcebuffer, before fetching/transmuxing/appending fragments of new quality level), and we are observing that sourceBuffer.remove(start,end) is not always flushing the buffer. you can reproduce this by clicking on current level buttons. but this deserve a separate ticket. 
I will create a separate ticket to track this. (this flushing is working fine on Chrome and IE11)
The current range removal implementation is currently incomplete and only support two configurations.
Sourcebuffer.remove(start, end)

Where either start is <= sourcebuffer.buffered.start(0) or end >= sourcebuffer.buffered.end(sourcebuffer.buffered.length-1)

As per spec, after any calls to sourcebuffer.appendBuffer() or sourcebuffer.remove() it's up to the JS client to verify that the buffered range got updated, as data may have been evicted, and often more so that required.
If you call sourcebuffer.remove(0, mediasource.duration) you are guaranteed that all the data would have been evicted and that your sourcebuffer is now empty.
When eviction occurs when adding data to one sourcebuffer (memory threshold is 75MB), all other source buffers will be truncated so that their start time are aligned.

This is how YouTube always calls it, and (unfortunately) our implementation has been mostly tailored toward getting YouTube to work perfectly.
our current flush implementation is the following :
flushAll :
  for each sb
  for(i = 0 ; i < sb.buffered.length ; i++) {
    sb.remove(sb.buffered.start(i),sb.buffered.end(i));
  }

but indeed this seems to be broken with FF if sb.buffered.length > 1

we were initially flushing everything using sb.remove(0, mediasource.duration)
but this was not working with IE11 ... which is expecting real buffer range.
(In reply to g.du.pontavice from comment #11)
> our current flush implementation is the following :
> flushAll :
>   for each sb
>   for(i = 0 ; i < sb.buffered.length ; i++) {
>     sb.remove(sb.buffered.start(i),sb.buffered.end(i));
>   }
> 
> but indeed this seems to be broken with FF if sb.buffered.length > 1

Yes, this is a collection of partial removal, which we don't support.
Change it into
If (sb.buffered.length > 0) {
sb.remove(sb.buffered.start(0), sb.buffered.end(sb.buffered.length-1));
}

This will work everywhere.

> 
> we were initially flushing everything using sb.remove(0,
> mediasource.duration)
> but this was not working with IE11 ... which is expecting real buffer range.

Well, it's perfectly okay to do so (per spec at least)

Surprising it doesn't work, as that's what YouTube does (you can verify with YouTube MSE/EME conformance test) and I doubt IE11 wouldn't work with YouTube.
yes, this seems to work as expected. we will use this way.
http://streambox.fr/mse/hls.js/demo/ has been updated.

partial buffer range removal seems to also work somewhat ?
click on "next level" buttons.this will flush a partial range of buffer (in the future).
this behavior will be used to perform smooth level switching (without interrupting the playback), 
this could be triggered when users move to full screen, we want to uncap the quality and smoothly switch to a better quality level... without having to wait too long.
Recheck my comment #10.
We can do partial removal so long that either condition is true.
Or in more layman term: we support removing from the beginning to anywhere, or from anywhere to the end.

With MP4 we can only remove from/to a complete moof as our sourcebuffer works on binary data, not on frames.
Fwiw, rather than using remove, you could adopt YouTube's approach. When the video quality is set to auto, they only feed about 5s of video ahead only (10s audio), it allows for smooth transition without too much wait time. Obviously they control bandwidth and dynamically adjust.
thanks :jya,
according to comment #14, flushing from a future position (aligned on a complete moof) to the end (Number.POSITIVE_INFINITY) should work; this is what we are currently doing.
(In reply to g.du.pontavice from comment #16)
> thanks :jya,
> according to comment #14, flushing from a future position (aligned on a
> complete moof) to the end (Number.POSITIVE_INFINITY) should work; this is
> what we are currently doing.

My suggestion would be to not bother removing anything and just re-append at the new rate. The more recently appended data will take precedence over the older data.
(In reply to Anthony Jones (:kentuckyfriedtakahe, :k17e) from comment #17)
> (In reply to g.du.pontavice from comment #16)
> > thanks :jya,
> > according to comment #14, flushing from a future position (aligned on a
> > complete moof) to the end (Number.POSITIVE_INFINITY) should work; this is
> > what we are currently doing.
> 
> My suggestion would be to not bother removing anything and just re-append at
> the new rate. The more recently appended data will take precedence over the
> older data.

This may be unique to our implementation: We always favour the latest appended data. Not sure how well that would work with other browsers.
Comment on attachment 8606113 [details] [diff] [review]
Whitelist Daily Motion

Approved for uplift to aurora and beta since Anthony's team is working with dailymotion for beta testing this feature.
Attachment #8606113 - Flags: approval-mozilla-beta?
Attachment #8606113 - Flags: approval-mozilla-beta+
Attachment #8606113 - Flags: approval-mozilla-aurora?
Attachment #8606113 - Flags: approval-mozilla-aurora+
Flags: qe-verify-
please discard my previous comment, there are a couple of typos, which are fixed below:

thanks for the merge into FF39/40.

I have another question:
we have some adaptive streams for which :
audio at lowest quality level is HE-AAC (SBR) (signaled as mp4a.40.5, sampling frequency is 22.05 kHz (real freq is double), 2 channels)
audio at higher quality level is standard AAC (signaled as mp4a.40.2, sampling frequency is also 22.05 kHz, 2 channels)

playback starts at low quality, and our audio SourceBuffer is created with codec type : mp4a.40.5.

AudioSpecificConfig (filled in esds box, available in moov) is as follow:

    AudioSpecificConfig[0] = adtsObjectType << 3;
    // samplingFrequencyIndex
    AudioSpecificConfig[0] |= (adtsSampleingIndex & 0x0E) >> 1;
    AudioSpecificConfig[1] |= (adtsSampleingIndex & 0x01) << 7;
    // channelConfiguration
    AudioSpecificConfig[1] |= adtsChanelConfig << 3;
    if(adtsObjectType === 5) {
      // adtsExtensionSampleingIndex
      AudioSpecificConfig[1] |= (adtsExtensionSampleingIndex & 0x0E) >> 1;
      AudioSpecificConfig[2] = (adtsExtensionSampleingIndex & 0x01) << 7;
      AudioSpecificConfig[2] |= 2 << 2;
      AudioSpecificConfig[3] = 0;
    }


when selecting HE-AAC, we are appending a moov with AudioSpecificConfig constructed with the following values:
    adtsObjectType = 5;
    adtsSampleingIndex = 7; // 22.05 kHz
    adtsExtensionSampleingIndex = 4; // 44.1 kHz


when switching to AAC, we are re-appending a new moov with AudioSpecificConfig constructed with the following values:
    adtsObjectType = 2;
    adtsSampleingIndex = 7; // 22.05 kHz


the append sequence looks like this:
append moov (HE-AAC)
append moof+mdat (HE-AAC samples)
append moov (AAC)
append moof+mdat (AAC samples)

we are able to seamlessly switch between HE-AAC and AAC on Chrome/Desktop and Mobile, without re instantiating a SourceBuffer.


However on FF39-40, we got a buffer append error on the last append

append moov (HE-AAC)
append moof+mdat (HE-AAC samples)
append moov (AAC)
append moof+mdat (AAC samples)    <=========== buffer append Error

is there anyway to get this scenario working in FF ?

thanks in advance for your feedback,

Guillaume
We do not support change of audio format in a source buffer (or any other type of sources for that matter)

The only way we could somehow handle it, is if the sampling rate, channel numbers and bit depth is the same across all streams. 

That you have a 44.1kHz followed by 22.05, isn't going to work with our current media architecture, at best the 2nd stream will be played at 44.1kHz

Audio Bandwidth is usually insignificant compare to video. I wouldn't worry about it and just output the same. 

But please provide a test page and we'll have a look for future versions.
this is great news...
Watching NASA TV on a mac:
http://dailymotion.github.io/hls.js/demo/?src=http%3A%2F%2Fnasatv-lh.akamaihd.net%2Fi%2FNASA_101%40319270%2Fmaster.m3u8

I got a green picture following the change of resolution; this typically happens when the init segment isn't sent or if the SPS do not match the content being decoded.

Are the init segment properly extracted and sent when changing resolutions?
can be easily reproduced... when seeking back and forth between resolutions ; will investigate tomorrow.
yes initsegment are sent when switching resolution.
i also checked this stream with flashls
http://www.flashls.org/latest/examples/chromeless/?src=http%3A%2F%2Fnasatv-lh.akamaihd.net%2Fi%2FNASA_101%40319270%2Fmaster.m3u8
and I got the following warning msg printed in the console:
WARN:TS: discarding video tag, as AVC HEADER not found yet, fragment not starting with I-Frame ?

most probably the fragment is not starting with a keyframe, leading to this behaviour.
an option would be to filter out video mp4 samples until the first keyframe is found...
with FF 42 and later this can't happen as we only ever store contiguous data between keyframes.
And when we detect a new resolution (or any data following an init segment) ; we internally seek (that is searching the next keyframe)

great one thank you so much <a herf ="https://lyricsace.com/category/romantic-whatsapp-status/">status video </a>

You need to log in before you can comment on or make changes to this bug.