contently / videojs-annotation-comments

A plugin for video.js to add support for timeline moment/range comments and annotations
https://contently.github.io/videojs-annotation-comments/
Other
171 stars 50 forks source link

Cannot dispose, teardown, or reset plugin without error #51

Closed tylerlaws0n closed 4 years ago

tylerlaws0n commented 4 years ago

Hi I am attempting to use this plugin with videojs v6.2.0 since that is the latest that you have said is supported. I am working on a student project where I am making a video viewing type page. My goal is to be able to switch the video and add a new set of annotations to each video. To do this I would need to reset the annotations each time I set a new source to the videojs object. When I attempt to dispose the plugin I get this error:

TypeError: Cannot read property 'teardown' of undefined at AnnotationComments.dispose

I noticed that there is a "resetData" event in the source code, but this is not available in the event registry. If it is possible to easily reset the plugin, so I can change the source of the videojs object, then add new annotations for the new video source after clearing the old ones, that would be awesome.

jackpope commented 4 years ago

Hi @tylerjlawson

For the error you are hitting specifically, you may be calling dispose too soon after init, or there could be a problem with the video source you are providing (maybe you are leaving it empty in hopes of swapping out later?)

I'm not sure which call of teardown is failing as there are a few in the dispose func, but those objects are mostly set in the postLoadDataConstructor function https://github.com/contently/videojs-annotation-comments/blob/master/src/js/annotation_comments.js#L56

So first I would make sure your videojs instance and the plugin are initializing properly with a valid video before tearing down.

If you do in fact have a race condition (AnnotationComments is not ready and you already want to teardown), it is probably possible to make the dispose a bit smarter. It could check for the video meta data before running teardowns. Would probably cancel this subscription as well https://github.com/contently/videojs-annotation-comments/blob/master/src/js/annotation_comments.js#L51

tylerlaws0n commented 4 years ago

So you were right about disposing too soon. I added a check to see if source has been set before I dispose the plugin and that error has disappeared. However, now I am trying to wait for the player and plugin to be ready before I add new annotations. Here is my attempt:

reset() {
      if (!this.first) {
        this.plugin.dispose();
        this.player.dispose();
      }
      this.first = true;
      return new Promise(resolve => {
        this.player.ready(function() {
          this.plugin.onReady(() => {
            resolve("finished");
          });
        });
      });
    }

this.first is a boolean to check if source has been set. I am getting this error now that I am trying to test for onReady of the plugin: Uncaught TypeError: Cannot read property 'onReady' of undefined Any Ideas? Also, the dispose is not removing annotations when I am changing videos as I was trying to do. When I try to destroy an annotation by id, nothing seems to happen, is there a reliable what to delete annotations?