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.

Sign up for GitHub

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

Feat/default duration #779

Merged
merged 5 commits into from
Dec 5, 2023
Merged

Feat/default duration #779

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
bb6c4d5
feat(defaultduration): Add defaultduration to use before media is rea…
cjpillsbury Dec 5, 2023
24ed9b5
chore(defaultduration): Update advanced usecase to use defaultduratio…
cjpillsbury Dec 5, 2023
982bd04
docs(defaultduration): Update docs for defaultduration.
cjpillsbury Dec 5, 2023
feddaf9
feat(defaultduration): Updating impl to rely on duration instead of r…
cjpillsbury Dec 5, 2023
fe30917
chore(defaultduration): dropping comment for infinite duration per PR…
cjpillsbury Dec 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions docs/src/pages/docs/en/components/media-controller.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,21 @@ as `breakpointx` attributes on media-controller and as `breakpointx`

When enabled, this will cause captions or subtitles to be turned on by default, if available.

### defaultduration

`defaultduration` (number, in seconds)

When enabled, this will use the value of `defaultduration` as the `mediaduration` before the media has been loaded. This is useful when you want to avoid preloading the media (e.g. for cost or network usage reasons) but still want the UI to show what the (already known) duration will be.

Example:

```html
<media-controller defaultduration="134"> <!-- aka 2:14 -->
<video slot="media" src="..." preload="none"></video> <!-- don't automatically load the media -->
<media-time-display showduration></media-time-display> <!-- This will show 0:00 / 2:14 before the media is loaded and the actual duration after -->
</media-controller>
```

### defaultstreamtype

`defaultstreamtype` (values: `live`, `on-demand`)
Expand Down
7 changes: 5 additions & 2 deletions docs/src/pages/docs/en/components/media-time-display.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,17 @@ The `<media-time-display>` component is used to show the current or remaining ti

Showing the duration can be configured with the `showduration` attribute.

Media Controller also sets the `defaultduration` to use before the media is loaded.

<SandpackContainer
editorHeight={265}
html={`<media-controller defaultsubtitles>
html={`<media-controller defaultduration="134">
<video
slot="media"
src="https://stream.mux.com/A3VXy02VoUinw01pwyomEO3bHnG4P32xzV7u1j1FSzjNg/low.mp4"
playsinline
muted
preload="none"
>
</video>
<media-control-bar>
Expand All @@ -55,7 +58,7 @@ Showing the remaining time by default can be configured with the `remaining` att

<SandpackContainer
editorHeight={265}
html={`<media-controller defaultsubtitles>
html={`<media-controller>
<video
slot="media"
src="https://stream.mux.com/A3VXy02VoUinw01pwyomEO3bHnG4P32xzV7u1j1FSzjNg/low.mp4"
Expand Down
3 changes: 2 additions & 1 deletion examples/vanilla/advanced.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,14 @@
<body>
<main>
<h1>Media Chrome Advanced Video Usage Example</h1>
<media-controller defaultsubtitles>
<media-controller defaultsubtitles defaultduration="134">
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NOTE: we can revert this if preferred, though I think it might be a good example for advanced.

<video
slot="media"
src="https://stream.mux.com/A3VXy02VoUinw01pwyomEO3bHnG4P32xzV7u1j1FSzjNg/high.mp4"
muted
crossorigin
playsinline
preload="none"
>
<track
label="thumbnails"
Expand Down
10 changes: 9 additions & 1 deletion src/js/controller.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,10 +150,18 @@
MEDIA_DURATION: {
get: function (controller) {
const { media } = controller;
const defaultDuration = controller.hasAttribute('defaultduration')
? +controller.getAttribute('defaultduration')
: NaN;

// TODO: What if duration is infinity/live? (heff)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, ha. Still a future us problem? Or can we remove this note?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't wanna be presumptuous, but we can probably remove it. We expect to strip live infinity in our architecture rn and don't have a story for live + our components that (typically) expect a (finite) duration. I'll drop it before merging.

if (!media || !Number.isFinite(media.duration)) {
return NaN;
return defaultDuration;
}

// Apply defaultDuration if it's set and the media is not yet ready
if (!(media.readyState || Number.isNaN(defaultDuration))) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not rely on media.duration? media.duration >= 0 and not NaN
could backfire with custom media elements that have a limited media API

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting thought! I think readyState better represents the actual conditions, but only relying on duration sounds like it would be good simply because it reduces the barrier of entry for custom slotted media elements' API. technically 0 duration media is valid, but I think we can ignore that wild scenario unless/until it comes up. Sold.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right 0 would be wild :) was thinking current time

return defaultDuration;

Check warning on line 164 in src/js/controller.js

View check run for this annotation

Codecov / codecov/patch

src/js/controller.js#L164 

Added line #L164 was not covered by tests
}

return media.duration;
Expand Down
5 changes: 4 additions & 1 deletion src/js/media-controller.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ const DEFAULT_TIME = 0;
export const Attributes = {
DEFAULT_SUBTITLES: 'defaultsubtitles',
DEFAULT_STREAM_TYPE: 'defaultstreamtype',
DEFAULT_DURATION: 'defaultduration',
FULLSCREEN_ELEMENT: 'fullscreenelement',
HOTKEYS: 'hotkeys',
KEYS_USED: 'keysused',
Expand All @@ -43,6 +44,7 @@ export const Attributes = {
*
* @attr {boolean} defaultsubtitles
* @attr {string} defaultstreamtype
* @attr {string} defaultduration
* @attr {string} fullscreenelement
* @attr {boolean} nohotkeys
* @attr {string} hotkeys
Expand All @@ -56,7 +58,8 @@ class MediaController extends MediaContainer {
Attributes.NO_HOTKEYS,
Attributes.HOTKEYS,
Attributes.DEFAULT_STREAM_TYPE,
Attributes.DEFAULT_SUBTITLES
Attributes.DEFAULT_SUBTITLES,
Attributes.DEFAULT_DURATION,
);
}

Expand Down