Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restore auto-revoking behavior of createObjectURL(MediaSource) to revert breaking change introducing memory leaks #156

Open
karlt opened this issue Aug 31, 2016 · 7 comments

Comments

@karlt
Copy link

commented Aug 31, 2016

#10 (comment)
pointed out that removing the auto-revoking behavior would be a breaking
change.

#10 (comment)
used two arguments to say this would not be a breaking change:

  1. That clients already needed to revoke explicitly anyway, due to a bug in
    the File API spec.
  2. That three existing UAs did not implement the auto-revoking behavior.

Firefox does auto-revoke, and so 2 was incorrect. This is tested in
web-platform-tests/wpt@7ca2c25

I am also not convinced by 1. Whether the precise time of revocation was
clear or not, it was clear that the intention was that the client need not
revoke.

MSE does not need object URLs of infinite lifetime because these merely
provide a mechanism of associating the MediaSource with a media element.

As referenced in
#10 (comment)
removing the auto revocation leads to memory leaks.
These leaks extend to objects affecting and affected by the MediaSource object.

The second sentence of
#10 (comment)
is also incorrect because Firefox, at least, revokes the object URL on the
next stable state, and so there is nothing to keep the objects alive.

I don't know the reasons for requiring explicit revocation of File URLs, but I
doubt they apply to MSE.

Keeping track of strings for object URLs created, or immediately explicitly
revoking, is an unnecessary burden for clients, one that is unexpected in a
garbage collected world. This outweighs benefits of a breaking change to
be consistent with the File API.

@wolenetz

This comment has been minimized.

Copy link
Contributor

commented Sep 2, 2016

While I generally agree with the sentiment that auto-revocation is best for web authors, the time for changing the spec in this way is pretty much past at this point for V1.
For VNext, a createFor() method for creating auto-revoking URLs (akin to FILE API method of same semantic) and/or just HTMLMediaElement.srcObject = a MediaSource object would be good to consider.
Blink and WebKit have had the non-autorevoking behavior for a long time, and there is a note in the current V1 editor's draft of the MSE spec encouraging web authors to explicitly revoke these URLs.

@jdsmith3000 @mwatson2 Please chime in on triage of this issue.

@jdsmith3000

This comment has been minimized.

Copy link
Contributor

commented Sep 6, 2016

I agree with @wolenetz's comments. It would be extremely difficult to revisit his issue, and adding a createFor() method in VNext seems like the only practical solution now.

@karlt

This comment has been minimized.

Copy link
Author

commented Sep 6, 2016

For vnext, rather than adding more object URL API, I'd suggest following the example of getUserMedia, removing createObjectURL and requiring srcObject support.

srcObject doesn't yet exist on source elements. I don't know that it would be necessary, but I guess it could be added if required.

I can't comment on how easy it would be to undo damage that slipped in just before the cut-off time.

@wolenetz wolenetz added this to the VNext milestone Sep 7, 2016
@wolenetz

This comment has been minimized.

Copy link
Contributor

commented Sep 7, 2016

@karlt thank you for your comments. @jdsmith3000 thanks for assisting triage. This sounds unfortunately like VNext at this point. Implementations may want to rapidly incubate some form of createFor() and/or srcObject MSE support (perhaps in WICG) to give web authors less "leaky" ways to attach a MediaSource to an HTMLMediaElement.

@wolenetz

This comment has been minimized.

Copy link
Contributor

commented Mar 19, 2019

An update and a proposed route forward for this issue:

Now that we're in vNext and incubating various new features for MSE (like codec switching and exposing the MSE API for use from worker context), this issue is again a priority.

(@jyavenard @jernoble)
From discussions this month at FOMS 2019 in NYC, Mozilla folks confirmed they still have the original (pre-MSE-REC) version of auto-revoking MediaSource objectUrls. This is confirmed by inspection of a related MSE web-platform-test: https://wpt.fyi/results/media-source/URL-createObjectURL-revoke.html?label=master&product=chrome%5Bexperimental%5D&product=edge&product=firefox%5Bexperimental%5D&product=safari%5Bexperimental%5D&aligned

Since Firefox is still autorevoking MediaSource object URLs, restoring such behavior as official MSE specified behavior is less likely to be a breaking change.

Problem / Motivation:
In Chrome (and likely in other MSE-REC implementations that do not currently auto-revoke MediaSource object URLs) there is an unnecessary burden on web app developers to explicitly revoke their MediaSource object URLs as soon as they are no longer necessary, so as to avoid memory bloat. Further, Chrome currently requires the objectUrl to not be revoked until after the attachment to the Media Element is completed as signaled by "sourceopen", since it does not dereference it until the middle of the resource selection algorithm. Also, the current MSE-in-Workers incubation (#175 - with current prototype Chromium CL: https://chromium-review.googlesource.com/c/chromium/src/+/1405697) uses a MediaSource object URL as the reference postMessage'd by the worker to the main thread, which then uses it to attach the worker-thread-constructed MediaSource object. This means that an immediately enqueued revocation task in the worker context's createObjectUrl(mediaSource) would race the main thread's attachment scheduling.

Proposed solution:
For MediaSource object URLs created on the main thread: upon successful URL.createObjectUrl(mediaSourceObject) execution, a task should be queued to revoke the created object URL. Alternatively, we could change this revocation's timing to occur upon successful attachment by a media element to the MediaSource object (as seen below, this would more align with the behavior needed to support MSE-in-Workers attachment using object URLs).
For MediaSource object URLs created in a worker context (#175): upon successful attachment by the main thread's media element to the MediaSource object, the object URL should be revoked.

I intend to begin incubation of this proposed solution soon (in the WICG MSE vNext repo, and in Chromium), because:

  1. the MSE REC API forces ongoing burden on web developers to ensure they explicitly revoke, and
  2. the potential impact on users of memory bloat needs to be fixed due to suspected lack of explicit revocation by web apps running on MSE implementations that comply with current MSE REC spec.

Feedback on this proposed solution would be very much appreciated.

Update Sep 19, 2019: I plan to experiment in Chrome soon with one of the proposed solutions, above: "Alternatively, we could change this revocation's timing to occur upon successful attachment by a media element to the MediaSource object (as seen below, this would more align with the behavior needed to support MSE-in-Workers attachment using object URLs)."

@karlt

This comment has been minimized.

Copy link
Author

commented Mar 20, 2019

Thanks for the helpful summary.

Is the MediaSourceHandle from https://github.com/wicg/media-source/blob/mse-in-workers-using-handle/mse-in-workers-using-handle-explainer.md still the intended direction there, or has this been replaced by object url strings?

I'm having trouble imagining the timing of the effect of URL.revokeObjectURL() if the url can concurrently be dereferenced in both a Window and Worker (or multiple Worklers). Is this defined somewhere?

@wolenetz

This comment has been minimized.

Copy link
Contributor

commented May 9, 2019

@karlt (#156 (comment)):

Is the MediaSourceHandle from https://github.com/wicg/media-source/blob/mse-in-workers-using-handle/mse-in-workers-using-handle-explainer.md still the intended direction there, or has this been replaced by object url strings?

the likely route forward for now with MSE-in-Workers will be to use object URL strings for MSE attachment rather than that MediaSourceHandle object.

I'm having trouble imagining the timing of the effect of URL.revokeObjectURL() if the url can concurrently be dereferenced in both a Window and Worker (or multiple Worklers). Is this defined somewhere?

That is a good general concern. However, MediaSource object URLs are only useful for attachment to HTML media elements, which live only on the window/main thread. There is no way these MediaSource object URLs are dereference-able in any other context, if I understand correctly. The current closest definition to this is in the proposal, above (#156 (comment)).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.