TagStudio icon indicating copy to clipboard operation
TagStudio copied to clipboard

Video Player

Open DrRetro2033 opened this issue 1 year ago • 14 comments

Did a redo on the video player, as it was in GitHub conflict hell. So it should work now, plus I even added some rounded corners to it as well.

DrRetro2033 avatar May 09 '24 02:05 DrRetro2033

Awesome!! The persistent volume settings seem to work fine, but the autoplay toggle seems to be inconsistent. If I toggle autoplay off followed by clicking on another entry and then clicking back to the first video, then it will autoplay regardless of the setting.

I'm also still having this same autoplay issue where it has inconsistency following the setting.

CyanVoxel avatar May 10 '24 01:05 CyanVoxel

Awesome!! The persistent volume settings seem to work fine, but the autoplay toggle seems to be inconsistent. If I toggle autoplay off followed by clicking on another entry and then clicking back to the first video, then it will autoplay regardless of the setting.

I'm also still having this same autoplay issue where it has inconsistency following the setting.

I can't seem to recreate the autoplay issue. It is working just fine on my end after multiple tries.

DrRetro2033 avatar May 10 '24 12:05 DrRetro2033

I can't seem to recreate the autoplay issue. It is working just fine on my end after multiple tries.

Here's a clip of me recreating the issue on my end:

https://github.com/TagStudioDev/TagStudio/assets/46939827/2e534205-969c-4331-8200-ac1603e10192

It usually takes the form of the video autoplaying regardless of the setting when clicking off then back to the file. It seems to also be affected by whether or not the last file clicked on was also a video.

CyanVoxel avatar May 10 '24 20:05 CyanVoxel

I can't seem to recreate the autoplay issue. It is working just fine on my end after multiple tries.

Here's a clip of me recreating the issue on my end: 2024-05-10.12-57-17-1.mp4

It usually takes the form of the video autoplaying regardless of the setting when clicking off then back to the file. It seems to also be affected by whether or not the last file clicked on was also a video.

Alright, I see why I did not get the bug, I was switching between video to video and not video to image. It was because of code that was checking to see if the new file is the same as the last video it played (for old logic I forgot to remove.) So, I had the check for autoplay, but the logic was skipping over it. Now it works just fine. ;)

DrRetro2033 avatar May 10 '24 22:05 DrRetro2033

Looking good! Besides the suggested changes above, there's only a couple issues I'm still running into. One is that the play/pause button seems to get desynced and doesn't update to the correct "pause" state when the videos start autoplaying.

The other issue might be tougher to track down, and seems to only affect certain video files I've tested it on. This is the issue I mentioned where only the center part of the video frame updates, unless there's mouse activity over the video where that'll force a frame update. So far I've only found 3 video files that do this, and they're all .mov files.

https://github.com/TagStudioDev/TagStudio/assets/46939827/154c9210-ca06-4d84-a197-d60aa81a0d63

CyanVoxel avatar May 11 '24 02:05 CyanVoxel

Looking good! Besides the suggested changes above, there's only a couple issues I'm still running into. One is that the play/pause button seems to get desynced and doesn't update to the correct "pause" state when the videos start autoplaying.

The other issue might be tougher to track down, and seems to only affect certain video files I've tested it on. This is the issue I mentioned where only the center part of the video frame updates, unless there's mouse activity over the video where that'll force a frame update. So far I've only found 3 video files that do this, and they're all .mov files. 2024-05-10.17-58-05-1.mp4

Alright, So I do not have any vertical videos in the mov format to test; BUT, I have a hunch it has something to do with how Qt is rendering the video. I noticed in the video you provided, the screen was glitching the most with videos that were higher than 1440p. So I tried playing the GTA 6 trailer at 2160p in the video player and it had artifacts and freezes the program. I may have a solution, but I do not know how long it will take to implement it. Hopefully I will have the solution in a day or two. But I am not holding my breath.

DrRetro2033 avatar May 11 '24 13:05 DrRetro2033

Alright, So I do not have any vertical videos in the mov format to test; BUT, I have a hunch it has something to do with how Qt is rendering the video. I noticed in the video you provided, the screen was glitching the most with videos that were higher than 1440p. So I tried playing the GTA 6 trailer at 2160p in the video player and it had artifacts and freezes the program. I may have a solution, but I do not know how long it will take to implement it. Hopefully I will have the solution in a day or two. But I am not holding my breath.

Here are the videos I've found that result in the issue for me. One of them isn't even vertical and is pretty low resolution (it's also a bizarre video so I'm sorry in advance), so that may (or may not) narrow down the issue... Videos for PR 149.zip

CyanVoxel avatar May 13 '24 04:05 CyanVoxel

Also for the current merge conflict: In commit b00dbf95487616a5e5123d428e86b3fd6e6e789a the resources_rc.py file was recompiled with fewer resources from the resources.qrc file. I assumed that you added your assets to the resources.qrc file and recompiled, but now that I'm looking at it closer you don't appear to have touched it at all and Qt may have updated the resources_rc.py on it's own, or maybe that's just an older version of the file that was present when you made your branch.

Basically, if you didn't touch these resource files then I would recommend removing/reverting the changes to the resources_rc.py file to solve the merge conflict.

CyanVoxel avatar May 13 '24 04:05 CyanVoxel

Alright, So I do not have any vertical videos in the mov format to test; BUT, I have a hunch it has something to do with how Qt is rendering the video. I noticed in the video you provided, the screen was glitching the most with videos that were higher than 1440p. So I tried playing the GTA 6 trailer at 2160p in the video player and it had artifacts and freezes the program. I may have a solution, but I do not know how long it will take to implement it. Hopefully I will have the solution in a day or two. But I am not holding my breath.

Here are the videos I've found that result in the issue for me. One of them isn't even vertical and is pretty low resolution (it's also a bizarre video so I'm sorry in advance), so that may (or may not) narrow down the issue... Videos for PR 149.zip

Thanks for the videos, they are extremely helpful. It appears that I am getting a slightly different, more stranger bug now. image It appears that the video is rotated 90 degrees, and it is for every video you sent. Extremely weird. I am starting to think PySide6 hates me.

DrRetro2033 avatar May 14 '24 14:05 DrRetro2033

Thanks for the videos, they are extremely helpful. It appears that I am getting a slightly different, more stranger bug now. ! It appears that the video is rotated 90 degrees, and it is for every video you sent. Extremely weird. I am starting to think PySide6 hates me.

Seeing how they're playing for you has potentially led me to the solution: These videos are actually encoded as having their height and widths swapped, with an EXIF rotation flag being set to correct for them. This is something I already have to manually account for when loading in image previews. I'll dig around the video player code to see where I can account for this flag and correct the resolution. Actual dimensions for the low-res landscape video: image image

CyanVoxel avatar May 14 '24 19:05 CyanVoxel

Upon further testing this issue may lay deeper within Qt or possibly FFmpeg. That fact that we already had different behaviors should've tipped me off. While I'm certain that the rotation flag is the root of this behavior (also it wasn't EXIF but rather just QuickTime metadata), I'm not sure if it's easily fixable on our end.

CyanVoxel avatar May 14 '24 21:05 CyanVoxel

I did checkout the branch to see why is the CI failing and I noticed one unrelated bug:

  • select any video in library
  • video starts playing in the thumbnail
  • hover over the video and it shows play icon (▶️ ), but the video is playing already so it should show pause icon instead.

yedpodtrzitko avatar May 16 '24 16:05 yedpodtrzitko

you can check this commit how to fix the mypy issues: https://github.com/TagStudioDev/TagStudio/commit/8e62147e24b4685271e69ea27bb1a79175651f89

yedpodtrzitko avatar May 17 '24 02:05 yedpodtrzitko

The play/pause indicator not updating after autoplaying seems to be mostly fixed, except in the case of when the video autoplays while the user's mouse is hovered over the indicator.

CyanVoxel avatar May 18 '24 04:05 CyanVoxel

This seems like a nice feature to add to this project, looking forward to it!

Shirkit avatar May 29 '24 16:05 Shirkit

@DrRetro2033 Thank you again for all your work on this!

CyanVoxel avatar May 29 '24 20:05 CyanVoxel