Video Player #149
Video Player #149
Conversation
# Conflicts: # tagstudio/src/qt/ts_qt.py
There was a problem hiding this comment.
Love the functionality! And sorry for all the code changes in the meantime, there was a recent formatting refactor that probably messed with stuff pretty bad. Thank you for your patience!
Overall I think this is going in a good direction, there's just a few things I've run into or would like to see done:
- When clicking on a new video file, the preview size is always incorrect until I manually grab the resize bar to "refresh" the panel (pictured below)
- Rarely I'm encountering an issue with vertical videos where the only about the middle part of the video is updating during playback, while the rest of the frame area only updates when the panel size changes. This may be related to the first issue.
- The video playback can be kind of intensive sometimes and slows down the program. This is more so an issue stemming from having to stream a potentially high bitrate video file from disk while the program runs. Because of this, I would really like to see an autoplay toggle to stop videos from automatically playing in the preview window. Maybe this could be included in the right-click context menu for the video preview.
- And lastly I'd like to see the mute/unmute status carry over from video to video instead of always defaulting to mute. This doesn't need to persist between sessions, but I think it makes sense for each next video to remember your audio preference.
Thank you again for working on this and getting a pretty cool feature going!
There was a problem hiding this comment.
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.
Other than that, just a couple general housekeeping things:
- Remove or comment out the testing log messages (under mouse, over mouse, etc.). Ideally the ffmpeg logs would also be removed, but I'm not even sure if it's possible to turn those off, so I wouldn't worry about those.
- On startup I get the following warning logs:
QGraphicsScene::addItem: item has already been added to this scene
QGraphicsProxyWidget::setWidget: cannot embed widget 0x27f44083a30 which is not a toplevel widget, and is not a child of an embedded widget
QGraphicsProxyWidget::setWidget: cannot embed widget 0x27f44083d90 which is not a toplevel widget, and is not a child of an embedded widget
The first warning seems to stem from line 85 in video_player.py, however I'm not sure about the other two warnings. I would suggest investigating the causes of these.
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. |
Here's a clip of me recreating the issue on my end: 2024-05-10.12-57-17-1.mp4It 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, 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... |
|
Also for the current merge conflict: In commit b00dbf9 the Basically, if you didn't touch these resource files then I would recommend removing/reverting the changes to the |
Thanks for the videos, they are extremely helpful. It appears that I am getting a slightly different, more stranger bug now. |
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. |
|
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. |
# Conflicts: # tagstudio/src/qt/resources_rc.py
# Conflicts: # tagstudio/src/qt/widgets/preview_panel.py
|
I did checkout the branch to see why is the CI failing and I noticed one unrelated bug:
|
|
you can check this commit how to fix the mypy issues: 8e62147 |
| radius=12, | ||
| fill=(0, 0, 0, 0), | ||
| ) | ||
| mask = mask.getchannel("A").toqpixmap() |
There was a problem hiding this comment.
mask has originally type PIL.Image, but here it's changed to QPixmap, so it would be really better to use different variable for one of these, so the types wont clash
| # return super().resizeEvent(event)\ | ||
|
|
||
| def inputMethodEvent(self, event: QInputMethodEvent) -> None: | ||
| return |
There was a problem hiding this comment.
is this intentional?
|
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. |
# Conflicts: # tagstudio/src/qt/widgets/preview_panel.py
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.