1287021 - [MSE] delaying-the-load-event-flag should be set to false when mediasource is attached to element

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect)

Description

[Jean-Yves Avenard [:jya]]

As per new MSE spec made on June 6th 2016 and discussed in https://github.com/w3c/media-source/issues/24

Comment 1

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1287021: [MSE] Set delaying-the-load-event-flag to false when attaching media source.

Review commit: https://reviewboard.mozilla.org/r/64498/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/64498/

Comment 2

[JW Wang [:jwwang] [:jw_wang]]

https://reviewboard.mozilla.org/r/64498/#review61488

::: dom/media/mediasource/MediaSource.cpp:435
(Diff revision 1)
>    }
>    MOZ_ASSERT(!mMediaElement);
>    mMediaElement = aDecoder->GetOwner()->GetMediaElement();
>    MOZ_ASSERT(!mDecoder);
>    mDecoder = aDecoder;
> +  mMediaElement->ChangeDelayLoadStatus(false);

Can you call ChangeDelayLoadStatus() here https://hg.mozilla.org/mozilla-central/file/88bebcaca249aeaca9197382e89d35b02be8292e/dom/html/HTMLMediaElement.cpp#l1516 without making it public?

Comment 3

[Jean-Yves Avenard [:jya]]

https://reviewboard.mozilla.org/r/64498/#review61488

> Can you call ChangeDelayLoadStatus() here https://hg.mozilla.org/mozilla-central/file/88bebcaca249aeaca9197382e89d35b02be8292e/dom/html/HTMLMediaElement.cpp#l1516 without making it public?

but then the order of operations isn't as per spec.

Comment 4

[JW Wang [:jwwang] [:jw_wang]]

That is a long text to read. Can you give a summary about what should happen in order in the spec?

Comment 5

[JW Wang [:jwwang] [:jw_wang]]

Do you mean https://github.com/w3c/media-source/issues/24#issuecomment-221993352
(3) just set the delaying-the-load-event-flag to false immediately upon MediaSource attachment to the HTMLMediaElement?

Comment 6

[Jean-Yves Avenard [:jya]]

yes

Comment 7

[JW Wang [:jwwang] [:jw_wang]]

I can't see how it is not per spec if we say:

if (mMediaSource) {
  RefPtr<MediaSourceDecoder> decoder = new MediaSourceDecoder(this);
  if (!mMediaSource->Attach(decoder)) {
    decoder->Shutdown();
    return NS_ERROR_FAILURE;
  }
  ChangeDelayLoadStatus(false); // <--
  // ...
}

Comment 8

[Jean-Yves Avenard [:jya]]

true... I had assumed that the event could only be fired after the loadstatus had changed, but it doesn't matter

Comment 9

[Jean-Yves Avenard [:jya]]

Comment on attachment 8771243 [details]
Bug 1287021: [MSE] Set delaying-the-load-event-flag to false when attaching media source.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/64498/diff/1-2/

Comment 10

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8771243 [details]
Bug 1287021: [MSE] Set delaying-the-load-event-flag to false when attaching media source.

https://reviewboard.mozilla.org/r/64498/#review61728

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1b3826196e7e
[MSE] Set delaying-the-load-event-flag to false when attaching media source. r=jwwang

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/1b3826196e7e