Skip to content

Conversation

@FuzzkingCool
Copy link
Contributor

This pull request enhances the thumbnail preview functionality in the Loader tool, introducing support for video thumbnails (particularly h264_* and other common video formats) and enabling video playback directly within the thumbnail widget. It also allows users to open thumbnails with the system's default viewer via double-click.

Video thumbnail and playback support:

  • Added logic to detect video representations (h264_*, mp4, mov, etc.) for selected versions and display their first frame as a thumbnail. If a video is detected, the widget shows a play overlay and supports playback using QtMultimedia if available. (client/ayon_core/tools/loader/ui/window.py, client/ayon_core/tools/utils/thumbnail_paint_widget.py) [1] [2] [3]
  • Implemented extraction of the first frame from video files using ffmpeg for preview purposes. (client/ayon_core/tools/utils/thumbnail_paint_widget.py)

User interaction improvements:

  • Added double-click support on thumbnails to open the image or video with the system's default viewer, handling cross-platform compatibility. (client/ayon_core/tools/loader/ui/window.py, client/ayon_core/tools/utils/thumbnail_paint_widget.py) [1] [2] [3]

UI and code refactoring:

  • Improved painting logic and code formatting for thumbnail rendering, including overlay drawing and aspect ratio handling. (client/ayon_core/tools/utils/thumbnail_paint_widget.py) [1] [2] [3] [4] [5] [6]

… LoaderWindow

- Implemented video representation path retrieval in LoaderWindow.
- Enhanced ThumbnailPainterWidget to detect video files and manage playback.
- Added functionality to extract the first frame from videos for thumbnail previews.
- Updated mouse event handling to toggle video playback on single click.
@ynbot ynbot added the size/S label Oct 30, 2025
@BigRoy
Copy link
Collaborator

BigRoy commented Oct 30, 2025

This sounds neat! Any chance you can attach some previews of what it looks like in action?

Side note: Your code has some style changes that I'm sure are from some linter to doing it, but they don't adhere to AYON's code style if I'm correctly looking at it. Any chance you can keep the 'style changes' out of the PR?

@BigRoy BigRoy requested a review from iLLiCiTiT October 30, 2025 16:31
@BigRoy BigRoy added community Issues and PRs coming from the community members type: enhancement Improvement of existing functionality or minor addition labels Oct 30, 2025
@MustafaJafar
Copy link
Member

This sounds neat! Any chance you can attach some previews of what it looks like in action?

2025-10-30.21-29-47.mp4

There are few things to consider,

  1. Empty thumbnails follow the type of the last working thumbnail. and I believe they all should default to look like an empty image not empty video.
  2. The video thumbnail doesn't work in place inside the loader.

@FuzzkingCool
Copy link
Contributor Author

Thanks @MustafaJafar for the video and feedback. These are definitely important issues to fix before this can get merged. Odd that the single-click play isn't working but I will try and recreate and get back to you. Cheers!

@FuzzkingCool
Copy link
Contributor Author

Upon reviewing the "thumbnail lag", video overlay and thumbnail type issues aside, there is a complication with how this was designed before my pull request:
When you first go into an entity's published files, it shows the latest published item's thumbnail it can find. This makes sense to immediately see an image on first dip in, but this also confuses things after the initial load and different selections are made because when all items are deselected it goes back to this image and this makes for a janky or misleading user experience. Instead after initial entry into the items for an entity, subsequent deselection of items should show empty imho. Thoughts?

@BigRoy BigRoy requested a review from mkolar October 30, 2025 22:19
…nail handling in LoaderWindow

- Added a method to select the latest published item in the ProductsWidget.
- Enabled automatic selection of the first item when no versions or folders are selected in LoaderWindow.
- Improved thumbnail management by clearing thumbnails when no entities are selected or when clearing current thumbnails.
self._selected_version_ids = set(event["version_ids"])
self._update_thumbnails()

def _get_video_representation_path(self, project_name, version_ids):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, no pipeline or ayon api usage in UI. All backend to do stuff should be moved to controller (and models).

and not self._selected_folder_ids
):
self._auto_select_first_item = False
self._products_widget.select_latest_item()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So if I select different version and then hit refresh, it will also change what I had selected in browser?

self._auto_select_first_item = False
self._products_widget.select_latest_item()

def _on_thumbnail_double_clicked(self, thumbnail_path):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please move to controller.

"""

# Signal emitted when thumbnail is double-clicked
thumbnail_double_clicked = QtCore.Signal(str) # Emits the thumbnail path
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doubled information

Suggested change
thumbnail_double_clicked = QtCore.Signal(str) # Emits the thumbnail path
thumbnail_double_clicked = QtCore.Signal(str)

self._on_media_status_changed
)
except (AttributeError, TypeError):
# Qt5 fallback
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What exactly is different? And why it crashes? The only difference I see is QMediaPlayer initialization (one line) but whole logic is duplicated. If it's because VideoSurface should be used then do

if hasattr(QtMultimedia.QMediaPlayer, "VideoSurface")"
    self._media_player = QtMultimedia.QMediaPlayer(
        self, QtMultimedia.QMediaPlayer.VideoSurface
    )
else:
    self._media_player = QtMultimedia.QMediaPlayer(self)

Comment on lines +122 to +126
try:
end_of_media = QtMultimedia.QMediaPlayer.MediaStatus.EndOfMedia
except AttributeError:
# Qt5 uses QMediaPlayer.EndOfMedia
end_of_media = QtMultimedia.QMediaPlayer.EndOfMedia
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
try:
end_of_media = QtMultimedia.QMediaPlayer.MediaStatus.EndOfMedia
except AttributeError:
# Qt5 uses QMediaPlayer.EndOfMedia
end_of_media = QtMultimedia.QMediaPlayer.EndOfMedia
# Qt5 uses QMediaPlayer.EndOfMedia
end_of_media = getattr(
QtMultimedia.QMediaPlayer,
"EndOfMedia",
QtMultimedia.QMediaPlayer.MediaStatus.EndOfMedia
)

if not filepath:
return False
basename = os.path.basename(filepath).lower()
# Check for h264_* naming convention
Copy link
Member

@iLLiCiTiT iLLiCiTiT Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why? There is not a convention.

if "h264_" in basename or "h264" in basename:
return True
# Check for common video extensions
video_exts = (".mp4", ".mov", ".avi", ".mkv", ".webm", ".m4v")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be constant and can be set.

Suggested change
video_exts = (".mp4", ".mov", ".avi", ".mkv", ".webm", ".m4v")
video_exts = {".mp4", ".mov", ".avi", ".mkv", ".webm", ".m4v"}

return True
# Check for common video extensions
video_exts = (".mp4", ".mov", ".avi", ".mkv", ".webm", ".m4v")
return any(filepath.lower().endswith(ext) for ext in video_exts)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return any(filepath.lower().endswith(ext) for ext in video_exts)
ext = os.path.splitext(filpath)[1].lower()
return ext in video_exts

video_exts = (".mp4", ".mov", ".avi", ".mkv", ".webm", ".m4v")
return any(filepath.lower().endswith(ext) for ext in video_exts)

def _extract_first_frame(self, video_path):
Copy link
Member

@iLLiCiTiT iLLiCiTiT Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eh, why?

Running ffmpeg on each selection change can be very heavy... And when are the temp files cleaned up?

(Should be in controller.)

from .lib import paint_image_with_color


class ThumbnailPainterWidget(QtWidgets.QWidget):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With changes made in this PR the widget is loader tool specific and should be moved there.

@iLLiCiTiT
Copy link
Member

iLLiCiTiT commented Nov 4, 2025

I might be wrong, but I believe most of studios don't integrate reviewables as representation anymore as they're uploaded to AYON server as activity. The code is very dirty and requires changes to be approved (at least for me), but the question is if this approach is the way to go with video player feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community Issues and PRs coming from the community members size/S type: enhancement Improvement of existing functionality or minor addition

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants