search

TagStudioDev / TagStudio

A User-Focused Photo & File Management System
GNU General Public License v3.0
2.84k stars 265 forks source link

Video Player #149

Closed DrRetro2033 closed 2 weeks ago

DrRetro2033 commented 1 month ago

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.

CyanVoxel commented 1 month ago

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.

DrRetro2033 commented 1 month ago

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.

CyanVoxel commented 1 month ago

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.

DrRetro2033 commented 1 month ago

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. ;)

CyanVoxel commented 1 month ago

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

DrRetro2033 commented 1 month ago

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.

CyanVoxel commented 1 month ago

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 commented 1 month ago

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.

DrRetro2033 commented 1 month ago

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.

CyanVoxel commented 1 month ago

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 commented 1 month ago

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.

yedpodtrzitko commented 1 month ago

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

yedpodtrzitko commented 1 month ago

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

CyanVoxel commented 4 weeks ago

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.

Shirkit commented 2 weeks ago

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

CyanVoxel commented 2 weeks ago

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