dimsemenov / PhotoSwipe

JavaScript image gallery for mobile and desktop, modular, framework independent
http://photoswipe.com
MIT License
23.99k stars 3.32k forks source link

Videos no longer working in vs 5.2.3 #1883

Closed acwolff closed 2 years ago

acwolff commented 2 years ago

@dimsemenov I did implement Video support as you suggested in issue #1780

This did work in fine in PhotoSwipe 5.1.2 as you can see in this 5.1.2 video test. Click on the 2nd thumb to open a video.

Sadly, it does no longer work in PhotoSwipe 5.2.3 as you can see in this 5.2 video test. If you click on the 2nd thumb the video is not opened and the console shows a Type error. Notice that the video plays correctly if you click on the first image thumb and swipes next to the next item, the first video.

I use this code:

lightbox.on('itemData', (e) => {
   const element = e.itemData.element;
   isImage=true;
   if (element  && element.dataset.pswpIsVideo) {
     isImage=false;
     const videoURL = element.href;
     const imgPoster= element.dataset.pswpIsPoster;
     const pos= videoURL.lastIndexOf("slides") +7;
     const id= videoURL.substr(pos, videoURL.length - pos);
     e.itemData = {
       html: `<video controls data-id="${id}"  poster="${imgPoster}"><source src="${videoURL}" type="video/mp4"> </video>`
     };
   }

lightbox.on('change', () => {
    slide_index=lightbox.pswp.currSlide.index;
    document.querySelector('.pswp__button--arrow--next').style.zIndex = "9999";
    if (getExtension(itemSrc[slide_index]) == 'mp4') {
       startVideo();
    }
});  
 function startVideo() {
     var ContinueSlideShow = playing;
     if (playing) playpause();
     const x = document.querySelector('[data-id="' + itemSrc[slide_index] + '"]') ;
     x.play();  // this is no longer working  
     videoRunning= true;
     x.addEventListener('ended', function() {
           videoRunning= false;
           if (ContinueSlideShow) playpause();
     }, false)
   }

This code is more complicated as you suggested to support videos in slideshows, ( after loading 3 itemdata events for the same click) but it was working fine with version 5.1.2.

Do you have any idea why this is no longer working in version 5.2.3?

dimsemenov commented 2 years ago

Not sure when do you call startVideo method, but document.querySelector('[data-id="' + itemSrc[slide_index] + '"]') ; is null, meaning there is no corresponding element in DOM yet.

I also have a video plugin under development, here https://github.com/dimsemenov/photoswipe-video-plugin , you may check the source if you're interested.

acwolff commented 2 years ago

I added two console messages:

    lightbox.on('itemData', (e) => {
       const element = e.itemData.element;
       isImage=true;
       if (element  && element.dataset.pswpIsVideo) {
         isImage=false;
         const videoURL = element.href;
         const imgPoster= element.dataset.pswpIsPoster;
         const pos= videoURL.lastIndexOf("slides") +7;
         const id= videoURL.substr(pos, videoURL.length - pos);
         console.log('itemData event id: ' + id);
         e.itemData = {
           html: `<video controls data-id="${id}"  poster="${imgPoster}"><source src="${videoURL}" type="video/mp4"> </video>`
         };
       }
    lightbox.on('change', () => {
        slide_index=lightbox.pswp.currSlide.index;
        document.querySelector('.pswp__button--arrow--next').style.zIndex = "9999";
        if (getExtension(itemSrc[slide_index]) == 'mp4') {
           console.log('Change event startVideo');
           startVideo();
        }

In version 5.2.1 the problem version, the console log is:

itemData event id: IMG_3613.mp4 itemData event id: IMG_3613.mp4 itemData event id: IMG_3613.mp4 Change event startVideo Uncaught (in promise) TypeError: Cannot read properties of null (reading 'play')

The error comes from the last line in the startVideo function:

function startVideo() {
  var ContinueSlideShow = playing;
  if (playing) playpause();
  const x = document.querySelector('[data-id="' + itemSrc[slide_index] + '"]') ;
  x.play();

In version 5.1.2 the correct version, the console log is:

itemData event id: IMG_3613.mp4 itemData event id: IMG_3613.mp4 itemData event id: IMG_3613.mp4 Change event startVideo itemData event id: Test_2MB.mp4 itemData event id: IMG_3613.mp4 itemData event id: Test_2MB.mp4 itemData event id: IMG_1043.mp4

You can try it with the two links given above.

Thanks for the video plugin, I will test it later.

acwolff commented 2 years ago

I added another console message in function strartVideo:

function startVideo() {
  var ContinueSlideShow = playing;
  if (playing) playpause();
  const x = document.querySelector('[data-id="' + itemSrc[slide_index] + '"]') ;
  console.log('In function startVideo: ' + '[data-id="' + itemSrc[slide_index] + '"]');
  x.play();
  videoRunning= true;
  x.addEventListener('ended', function() {
        videoRunning= false;
        if (ContinueSlideShow) playpause();
  }, false)
}

Notice that the problem album 5.2.1 works correctly ia a slideshow: Open this album, click on the F12 key and click on the title 'Test videos' Next click on the play button in the upper left corner. The slideshow starts and he videos do play now correctly and you see this console log:

itemData event id: IMG_3613.mp4 itemData event id: Test_2MB.mp4 itemData event id: Test_2MB.mp4 itemData event id: IMG_1043.mp4 Change event startVideo In function startVideo: [data-id="IMG_3613.mp4"] itemData event id: IMG_1043.mp4 itemData event id: IMG_0877.mp4 Change event startVideo In function startVideo: [data-id="Test_2MB.mp4"] itemData event id: IMG_0877.mp4 Change event startVideo In function startVideo: [data-id="IMG_1043.mp4"] Change event startVideo In function startVideo: [data-id="IMG_0877.mp4"]

dimsemenov commented 2 years ago

I'd recommend to use contentActivate and contentDeactivate events to control the playback of videos

pswp.on('contentActivate', (e) => {
  console.log('slide activated, element:', e.content.element);
});
pswp.on('contentDeactivate', (e) => {
  console.log('slide deactivated, element:', e.content.element);
});
acwolff commented 2 years ago

@dimsemenov thanks for your suggestion. I am using that now instead of the change event, see this 5.2.1. test page.

But this did not solve the crash. The crash which occured after clicking on a video thumb was caused by an unfinished initialisation. So I solved this by setting a variable initCompleted= true in the asfterInit event. If that variable is false, a video is not automaticlly started, so I do not get the crash.

The test is now running OK I think with the following code:

    lightbox.on('contentActivate', (e) => {
        slide_index=lightbox.pswp.currSlide.index;
        console.log('slide activated, element:', e.content.element);
        if (initCompleted) {
          var element = e.content.element;
          let text = element.innerHTML;
          console.log("element.innerHTML: ", text);
          let startPos = text.indexOf('data-id="' ) ;
          if (startPos>-1)  {
            startPos = startPos + 9;
            let endPos = text.indexOf('"', startPos) ;
            videoId= text.substring(startPos, endPos);
            console.log("videoId: ", videoId);
            startVideo(videoId);
          }
        }
    });
function startVideo(id) {
  var ContinueSlideShow = playing;
      if (playing) playpause();
      const x = document.querySelector('[data-id="' + id + '"]') ;
      console.log('In function startVideo: ' + '[data-id="' + id + '"]');
      x.play();
      videoRunning= true;
      x.addEventListener('ended', function() {
            videoRunning= false;
            if (ContinueSlideShow) playpause();
      }, false)
}

What code do you recommend to start a video under program control?

About the 'contentDeactivate' event, I use already the ''slideDeactivate' event. What is the difference between the two events?

I had a quick look to the Video-plugin. The demo code you shows is for an old version of the PhotoSwipe library. Can I test that plugin with the current PhotoSwipe version? When do you plan an update of that plugin to the current PhotoSwipe version?

Has your video-plagin the possibility to start and stop a video under program control?

Thanks for your great library and your excellent support!

dimsemenov commented 2 years ago

here's basic example:

lightbox.on('contentActivate', (e) => {
  const video = e.content.element.querySelector('video');
  if (video) {
    video.play();
    video.onended = () => lightbox.pswp.next();
  } 
});

lightbox.on('contentDeactivate', (e) => {
  const video = e.content.element.querySelector('video');
  if (video) {
    video.pause();
    video.onended = undefined;
  } 
});

About the 'contentDeactivate' event, I use already the ''slideDeactivate' event. What is the difference between the two events?

There is no difference in your case, slides may have multiple types of content or no content at all.

acwolff commented 2 years ago

Thank you very much for this code, it will simplify my code a lot!

But your solution gives the same crash as discussed above. The crash which occured after clicking on a video thumb was caused by an unfinished initialisation. I solved this by setting a variable initCompleted= true in the asfterInit event and checking that variable in the contentActivate event:

    lightbox.on('contentActivate', (e) => {
        if (!initCompleted) {
            setTimeout(function() {
            console.log("initCompleted after delay: ", initCompleted)
            checkVideo(e.content.element);
          }, 500);
        }
        if (initCompleted) {
            checkVideo(e.content.element);
        }
    });

De video is started in:

    function checkVideo(element) {
        const video = element.querySelector('video');
        if (video) {
            video.play();
            video.onended = () => lightbox.pswp.next();
        }
    }

You can see this code in action in my video testalbum.

Thanks you very much for your help and your great library!