Add setSource method #153
Add setSource method #153
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
rtritto
force-pushed
the
add-setSource
branch
from
0009f56 to
09fe160
Compare
src/core/player.ts
Outdated
| * Set the media source | ||
| * @param source New media source URL | ||
| */ | ||
| setSource(source: string) { |
There was a problem hiding this comment.
I'm not sure if the method should be here, because the Player class is used by all providers with inheritance (Youtube, Dailymotion, HTML5, etc.) and this is a feature only for HTML5 video and audio. This is the first time this case has arisen I believe.
This must also be documented. I will check
There was a problem hiding this comment.
There was a problem hiding this comment.
Video.js method is setSource
There was a problem hiding this comment.
Maybe, if this is a feature only for HTML, it can not be added but the doc should at least be updated with related use cases on how can be done.
I wrote below my use case with HTML and where I have some problem.
There was a problem hiding this comment.
It can be added, but the player file is a bridge between all provider. If the method is for only one provider, it should be is the dedicated provider (html5)
There was a problem hiding this comment.
If the method is for only one provider, it should be is the dedicated provider (
html5)
Agree.
Can the providers be used as modular plugin? (new feature)
There was a problem hiding this comment.
I did a commit to move setSource method from player core to html5 provider.
|
In my test, I have the videoReference ( videoReference.src = 'new-src.mp4' // prefer src method over videoReference.setAttribute('src', 'new-src.mp4'); both trigger the DOM but src method also triggers a reload of the video content
videoReference.load() // force video reloadThe issue is that the player doesn't reload time (current time and time duration) and current progress bar. 2025-02-19.012843.mp4There is any method of player that I should call? |
|
Have you tested with audio and video HTML5? |
| @@ -296,6 +296,7 @@ The player instance exposes the following methods, accessible when the player is | |||
| | `mute()` | - | - | Mute the volume | | |||
| | `unMute()` | - | - | Unmute the volume | | |||
| | `seekTo(time)` | `Number` | - | Seek to a current time in seconds | | |||
| | `setSource(source)` | `String` | - | Set the media source | | |||
There was a problem hiding this comment.
It should be added a mention to specify that it is only available for HTML5 video (and audio?)
You can get inspiration from similar examples in the README
https://github.com/vlitejs/vlite/blob/main/README.md?plain=1#L266-L270
There was a problem hiding this comment.
Should it be implemented also for other non-HTML5 providers?
There was a problem hiding this comment.
I've check and other providers have method to dynamically change the video source. Good news! And I prefer have common use case between providers.
- Youtube:
loadVideoById() - Dailymotion:
loadContent() - Vimeo:
loadVideo()
The change can finally be in Player class but the same method needs to work with all providers. You can inspired from other method for the logic. Their method seems to not be a Promise.
The commun parameter seems to be videoId which contains the new video ID.
Example of implementation:
diff --git a/README.md b/README.md
index 669135a..aa3497d 100644
--- a/README.md
+++ b/README.md
@@ -295,6 +295,7 @@ The player instance exposes the following methods, accessible when the player is
| `getDuration()` | - | `Promise` | Get the duration |
| `mute()` | - | - | Mute the volume |
| `unMute()` | - | - | Unmute the volume |
+| `setSource()` | `String` | - | Set the new source from video ID |
| `seekTo(time)` | `Number` | - | Seek to a current time in seconds |
| `requestFullscreen()` | - | - | Request the fullscreen |
| `exitFullscreen()` | - | - | Exit the fullscreen |
diff --git a/src/core/player.ts b/src/core/player.ts
index 82598ab..e715ff5 100644
--- a/src/core/player.ts
+++ b/src/core/player.ts
@@ -168,6 +168,13 @@ export default abstract class Player {
*/
public abstract methodUnMute(): void
+ /**
+ * methodSetSource
+ * Extends by the provider
+ * @abstract
+ */
+ public abstract methodSetSource(videoId: string): void
+
/**
* On the player is ready
*/
@@ -455,6 +462,14 @@ export default abstract class Player {
this.dispatchEvent('volumechange')
}
+ /**
+ * Set the new source of the player
+ * @param videoId Video ID
+ */
+ setSource(videoId: string) {
+ this.methodSetSource(videoId)
+ }
+
/**
* Update the current time of the media element
* @param newTime New current time of the media element
diff --git a/src/providers/vimeo/vimeo.ts b/src/providers/vimeo/vimeo.ts
index ba59476..8c2ea7d 100644
--- a/src/providers/vimeo/vimeo.ts
+++ b/src/providers/vimeo/vimeo.ts
@@ -147,6 +147,10 @@ export default function VimeoProvider(Player: any) {
})
}
+ methodSetSource(id: string) {
+ this.instance.loadVideo(id)
+ }
+
/**
* Play method of the player
*/There was a problem hiding this comment.
videoId parameter name is okay for Youtube, Vimeo and Dailymotion but not for HTML5 which have a video url. Needs to adapt
Yes, the change works also with audio: the source is updated but the player doesn't reload time (current time and time duration) and current progress bar: 2025-02-22.233921.mp4 |
The action to be taken after loading the new media is not so simple. The player needs to be “reset”
|
|
The reset logic of the player should be added as event every time the source is changed by |
Fix #151
This PR contains a:
Note
To be tested and reviewed