What specific section or headline is this issue about?
Multiple cross-cutting areas of the MSE MDN documentation updated by PR #22242 (MSE-in-Workers content additions) contained items I found during my PR review unfortunately after the PR had already been approved and merged.
What information was incorrect, unhelpful, or incomplete?
Below is copied verbatim from the PR #22242 conversations where I originally noted the issues on my eventual review of the PR:
So what's the deal here, and what do you think I should say on the srcObject page?
In Chrome, srcObject on HTMLMediaElement is only currently usable for associating a MediaStream or a Dedicated Worker MediaSource's MediaSourceHandle. Main thread MediaSources cannot provide handles yet, nor does srcObject work for plain MediaSource objects in Chrome.
srcOjbect: Should probably get some kind of note mentioning MediaSourceHandle (from a Dedicated Worker MediaSource's handle getter, transferred to the main thread) is the way to attach a Dedicated Worker MediaSource to a main thread HTMLMediaElement.
canConstructInDedicatedWorker: LGTM
MediaSource.handle: mostly LGTM, though the worker portion of the example script should probably indicate "// In a dedicated worker" near the top of that example snippet, and indicate something like "// Transfer the handle to the main context" before the postMessage line in that snippet (perhaps also with some clarification, since that postMessage sends to whichever context created the dedicated worker, and it might not be the main thread; MediaSourceHandles cannot be successfully transferred into or via a Shared Worker or Service Worker, so it could assist readers to point out the intended target is the main context in the snippet), and also indicate something like "// Await sourceopen on the MediaSource before creating SourceBuffers and populating them with fetched media" because the dedicated worker MediaSource won't accept creation of SourceBuffers until its readyState is "open" -- which can only happen once the MediaSource (in this case via the MediaSource's handle) is attached to the HTMLMediaElement.
MediaSourceHandle: mostly LGTM, though MediaSourceHandle is currently only retrievable from a Dedicated Worker MediaSource, so technically the statement "Each MediaSource object has its own distinct MediaSourceHandle" is false, since there is no MediaSourceHandle for a main thread MediaSource object.
MediaSource: mostly LGTM, though similar comments around example snippet of usage of dedicated worker MediaSource as noted above. Also would help to clarify that the handle getter is visible only on Dedicated Worker MediaSource instances. Also noticed a minor mispelling: "constructuring" should be "constructing".
Media_Source_Extensions_API: worker part LGTM. I did notice above that, "MSE allows us to replace the usual single track src value fed to media elements..." would be more accurate if it instead said "MSE allows us to replace the usual single progressive src URI fed to media elements..." (this is because such progressive src URI can indeed have multiple tracks muxed inside them, they just provide no ability for the web app to do things like bitrate adaptation, splicing, codec switching, or advanced telemetry reporting).
Functions and classes available to workers: LGTM
Finally, on at least one page (MediaSource.handle), the "Note: This feature is available in Web Workers" link is broken. Maybe this is due to the preview linkage or perhaps it's a real broken linkage.
What did you expect to see?
See the detailed list of questions and items I noted as possibly incorrect/needing clarification/incomplete, above.
Do you have any supporting links, references, or citations?
The MSE spec and my mse-in-workers-demo are already both linked appropriately and contain the best information.
This issue is meant to fix some post-merge items I found in my eventual PR review of #22242. See also the related data update PR (https://github.com/mdn/browser-compat-data/pull/18189) and similar issue I filed today on the post-merge items I found with it, too (https://github.com/mdn/browser-compat-data/issues/18220).
Do you have anything more you want to share?
Thank you for doing this content update! I apologize for not getting my review comments entered more rapidly on this, necessitating this issue as a follow-up of the already-merged PR #22242
sideshowbarker
commented
1 year ago
MDN URL
https://developer.mozilla.org/en-US/docs/Web/API/Media_Source_Extensions_API
What specific section or headline is this issue about?
Multiple cross-cutting areas of the MSE MDN documentation updated by PR #22242 (MSE-in-Workers content additions) contained items I found during my PR review unfortunately after the PR had already been approved and merged.
What information was incorrect, unhelpful, or incomplete?
Below is copied verbatim from the PR #22242 conversations where I originally noted the issues on my eventual review of the PR:
https://github.com/mdn/content/pull/22242#issuecomment-1315902153:
In Chrome, srcObject on HTMLMediaElement is only currently usable for associating a MediaStream or a Dedicated Worker MediaSource's MediaSourceHandle. Main thread MediaSources cannot provide handles yet, nor does srcObject work for plain MediaSource objects in Chrome.
https://github.com/mdn/content/pull/22242#issuecomment-1315923645: Preview URL verifications:
Finally, on at least one page (MediaSource.handle), the "Note: This feature is available in Web Workers" link is broken. Maybe this is due to the preview linkage or perhaps it's a real broken linkage.
What did you expect to see?
See the detailed list of questions and items I noted as possibly incorrect/needing clarification/incomplete, above.
Do you have any supporting links, references, or citations?
The MSE spec and my mse-in-workers-demo are already both linked appropriately and contain the best information. This issue is meant to fix some post-merge items I found in my eventual PR review of #22242. See also the related data update PR (https://github.com/mdn/browser-compat-data/pull/18189) and similar issue I filed today on the post-merge items I found with it, too (https://github.com/mdn/browser-compat-data/issues/18220).
Do you have anything more you want to share?
Thank you for doing this content update! I apologize for not getting my review comments entered more rapidly on this, necessitating this issue as a follow-up of the already-merged PR #22242