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

RESOLVED FIXED in Firefox 50

Status

()

RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: jya, Assigned: jya)

Tracking

(Blocks: 1 bug)

Trunk
mozilla50
Points:
---

Firefox Tracking Flags

(firefox50 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

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

Comment 1

3 years ago
Created attachment 8771243 [details]
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/
Attachment #8771243 - Flags: review?(jwwang)
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?
(Assignee)

Comment 3

3 years ago
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.
That is a long text to read. Can you give a summary about what should happen in order in the spec?
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?
(Assignee)

Comment 6

3 years ago
yes
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); // <--
  // ...
}
(Assignee)

Comment 8

3 years ago
true... I had assumed that the event could only be fired after the loadstatus had changed, but it doesn't matter
(Assignee)

Comment 9

3 years ago
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/
Attachment #8771243 - Flags: review?(jwwang) → review+
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

3 years ago
Pushed by jyavenard@mozilla.com:
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

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/1b3826196e7e
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox50: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in before you can comment on or make changes to this bug.