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

Support changing of the video source #51

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

Conversation

philipwatson
Copy link
Owner

@philipwatson philipwatson commented Oct 28, 2024

In progress.

When the video source is changed, the plugin will request the vast (or use the provided vast xml) and start the ad workflow again.

You shouldn't change the source during the ad break because contrib-ads doesn't support this case. For example, should do something like this:

if (!player.ads.isInAdMode()) {
   // then should be ok to change source
} 

But not sure if this will be enough. So added support for changing source during ad playback - might disable this by default (have to enable via config)... though not sure about this yet.

Other things I need to think about or do:

  • ability to change the vast XML
  • ability to change the vast URL
  • ability to change the ad schedule
  • ability to cancel upcoming ads (ad schedule) so user can safely change source
  • is it safe to change content source in ad mode (when player.ads.inAdBreak() is false)?
  • test live stream and vpaid

@nachoaguirre
Copy link
Contributor

cool! great news

I've been fighting some bugs too, but my case is worse, since I'm integrating a plugin for Peertube (a YouTube clone, more or less), and it works as a SPA, so it's harder to intercept events... all the page changes, video started, finished, etc, everything is asynchronous... Terrible for me hehe

I have some improvements that you could try if you like and integrate them (I'd love to do it but I don't have time)

  • some browsers (like FF) are throwing buffer problems, i fixed this adding the following in videojs.ads.min.js
player.on('loadstart', checkSrc);
window.setTimeout(checkSrc, 1);

image

I invite you to try it and if you see that it improves something, it could be integrated hehe


I noticed that in some cases when an ad is playing, the play icon was not being displayed... I fixed it like this in ui.mjs


updateState: (button, player) => {
    button.removeClass('vjs-icon-play');
    button.removeClass('vjs-icon-pause');
    button.addClass(player.paused() ? 'vjs-icon-play' : 'vjs-icon-pause');
}

image


A little further down, in the destructuring of config[] added updateState
const { className, action, toggleClasses, events, initialState, updateState } = config[type];


further down the toggleIcon was changed to:

const toggleIcon = () => {
        if (updateState) {
          updateState(button, player);
        } else {
          toggleClasses.forEach(cls => button.toggleClass(cls));
        }
      };

image

@philipwatson
Copy link
Owner Author

philipwatson commented Feb 27, 2025

@nachoaguirre Thank you for your feedback. Will look into them when I find the time. Currently addressing VPAID issues at the moment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants