silvermine / videojs-quality-selector

MIT License
180 stars 55 forks source link

Remote text track disappears when video quality is changed #82

Closed adis0308 closed 2 years ago

adis0308 commented 2 years ago

If I change the video quality then the remote text track that I added earlier disappears. Here's the js code I'm using.

player.addRemoteTextTrack({
    src: '//localhost/videojs/subtitle.vtt',
    label: 'English',
    kind: 'subtitles',
    srclang: 'en',
    default: true
}, false);
player.controlBar.addChild('QualitySelector');
adis0308 commented 2 years ago

Please anyone help me. I like to use this plugin because it is compatible with the videojs-ima plugin whereas the similar "videojs-resolution-switcher" plugin that I tried is not compatible. Thanks

yokuze commented 2 years ago

@adis0308: If you'd like assistance, you will need to post a minimal, complete, verifiable example. (MCVE) Sometimes just by creating a MCVE you will actually discover the source of the problem yourself - or at least more details to help us reproduce it.

If you want some good examples of MCVE's, see a couple of issues that we have posted elsewhere, e.g:

All of that detail is really important in getting help - especially on an open source plugin where people are donating their time. Without it we can't be sure what version of VideoJS you're using, or what other plugins you're using with it.

If you're able to reproduce the issue in an MVCE, post a comment here and the issue can be reopened.

navidmafi commented 2 years ago

I can confirm this problem. even adding back the button with vjs.controlBar.addChild("SubtitlesButton"); does not work. Also the "answer" to this problem which closed the issue is beyond unhelpful. If you can't help you may not close a valid issue and let others see it.

jthomerson commented 2 years ago

@navidmafi I'm not sure if you have experience maintaining an open source project, but comments like yours are not helpful. We are unpaid volunteers. We've made our work available for free. We have no obligation to debug someone else's problem for them. We are happy to help folks use the free software we've written. But if it's working for us (and this is), then we have no idea why it's not working for you or @adis0308. Asking you to provide an MCVE is very common. It's also the most respectful thing you can do, because it shows that you value our time and the free software you are getting from us, as well as showing that you will appreciate the free support you are expecting us to provide.

If you don't have time to create an MCVE, that's absolutely no problem. But we don't have time to create one for you, either. And even if we did, we couldn't reproduce your problem because it works for us.

As was stated in the comment that you find unhelpful, if someone provides an MCVE that demonstrates the problem, we will reopen the issue. Anyone who wants to can still see the issue despite it being closed, as is proven by you finding the issue so that you voipd leave your comment on it.

Have a great day.

navidmafi commented 2 years ago

I didn't mean to be disrespectful or anything. I meant that I usually see the problem of user reporting the issue without proper information being handled by a needinfo tag or something. which could result in the issue being easier to see by more people and easier to find for the ones who have this problem aswell. I shouldn't have commented at 4am. I hope things are more clear now.

Anyway , the problem is when player.ready is called in changeQuality method, textTracks would be gone and need to be added back. Also the videojs player.textTracks() method was a bit confusing. (and still is). I expected the method to return a Set or an Array but the actual array was not typed and was hidden under player.textTracks().tracks_

After reading more about requirements for a PR and the process, I decided not to create a PR and share my idea directly in this issue

here is what I achieved and it does currently work. I hope this can give some insight as to what might be wrong:

//src/js/index.js
...
videojs.hook('setup', function(player) {
      function changeQuality(event, newSource) {
         var sources = player.currentSources(),
             currentTime = player.currentTime(),
             currentPlaybackRate = player.playbackRate(),
             currentPlayerTextTracks = player.textTracks().tracks_.slice(), // clone the text tracks array
             isPaused = player.paused(),
             selectedSource;

         // Clear out any previously selected sources (see: #11)
         _.each(sources, function(source) {
            source.selected = false;
         });

         selectedSource = _.findWhere(sources, { src: newSource.src });
         // Note: `_.findWhere` returns a reference to an object. Thus the
         // following updates the original object in `sources`.
         selectedSource.selected = true;

         if (player._qualitySelectorSafeSeek) {
            player._qualitySelectorSafeSeek.onQualitySelectionChange();
         }

         player.src(sources);

         player.ready(function() {
            if (!player._qualitySelectorSafeSeek || player._qualitySelectorSafeSeek.hasFinished()) {
               // Either we don't have a pending seek action or the one that we have is no
               // longer applicable. This block must be within a `player.ready` callback
               // because the call to `player.src` above is asynchronous, and so not
               // having it within this `ready` callback would cause the SourceInterceptor
               // to execute after this block instead of before.
               //
               // We save the `currentTime` within the SafeSeek instance because if
               // multiple QUALITY_REQUESTED events are received before the SafeSeek
               // operation finishes, the player's `currentTime` will be `0` if the
               // player's `src` is updated but the player's `currentTime` has not yet
               // been set by the SafeSeek operation.
               player._qualitySelectorSafeSeek = new SafeSeek(player, currentTime);
               player.playbackRate(currentPlaybackRate);
               if (currentPlayerTextTracks && player.textTracks().tracks_[0] === undefined) {
                  //We check for the existence of the text track because the player may have been created already with proper text tracks
                  for (const t of currentPlayerTextTracks) {
                     const trackToAdd = {
                        kind: t.kind,
                        id: t.id,
                        label: t.label,
                        src: t.src,
                        language: t.language,
                        mode: t.mode,
                        loaded: t.loaded,
                     };

                     player.addRemoteTextTrack(trackToAdd);
                  }
               }
            }
            if (!isPaused) {
...