feat: SG-42241: RV: Implement the ability to fetch filmstrips and thumbnails for sources in coming into RV's Session Manager#1200
Conversation
Signed-off-by: chenl1 <ling.jie.chen@autodesk.com>
Signed-off-by: chenl1 <ling.jie.chen@autodesk.com>
Signed-off-by: chenl1 <ling.jie.chen@autodesk.com>
Signed-off-by: chenl1 <ling.jie.chen@autodesk.com>
Signed-off-by: chenl1 <ling.jie.chen@autodesk.com>
Signed-off-by: chenl1 <ling.jie.chen@autodesk.com>
Signed-off-by: chenl1 <ling.jie.chen@autodesk.com>
| { | ||
| let widget = QWidget(nil, 0), | ||
| layout = QHBoxLayout(widget); | ||
| widget.setStyleSheet("background: transparent;"); |
There was a problem hiding this comment.
I see you added several setStyleSheet calls, and I believe we should be using our .qss files to set properties based on themes instead. Also, I know users can set a specific font size in the RV preferences tab. You might want to validate how that works with the metalabel font size property you added below
| string meta = ""; | ||
| try | ||
| { | ||
| let sn = sourceNodeOfGroup(node), |
There was a problem hiding this comment.
I would rename sn to something more meaningful here. I know there is a lot of mu code using abbreviations or one or two letters as a variable name, but there is no reason to do that for newer code. This is an obvious one, but I saw a few other like nat_w and mprop
|
|
||
| try | ||
| { | ||
| let sn = sourceNodeOfGroup(node); |
There was a problem hiding this comment.
You are declaring twice sn in this method (i.e. here and at line 1841). You should reuse the variable instead of calling sourceNodeOfGroup twice.
| preview.setFixedSize(QSize(PREVIEW_WIDTH, PREVIEW_HEIGHT)); | ||
| preview.setFallback(_fallbackSourceIcon.pixmap(QSize(PREVIEW_WIDTH, PREVIEW_HEIGHT))); | ||
|
|
||
| try |
There was a problem hiding this comment.
I would avoid doing try catch blocks around several lines, and with a catch-all. I would keep it specifically for the line that could throw:
| try | |
| try | |
| { | |
| let sourceNode = sourceNodeOfGroup(node); | |
| } | |
| catch (exception exc) | |
| { | |
| print("WARNING: [...]); | |
| } |
Since sourceNodeOfGroup can return a string or nil, you should then add a statement around the lines that get the paths and load the thumbnail and filmstrip.
| textLayout.addWidget(nameLabel); | ||
|
|
||
| string meta = ""; | ||
| try |
There was a problem hiding this comment.
I think you can remove the try catch statement here, and simply guard what is inside of it by checking that the source node is not nil before proceeding. For the media property, You can simply check if propertyExists() on the string before calling getStringProperty which would prevent it from throwing.
SG-42241: RV: Implement the ability to fetch filmstrips for sources in coming from FPT into RV's Session Manager
Summarize your change.
Added ThumbnailWidget, FilmstripWidget, and SourcePreviewWidget to session_manager_mu.in so that we can display a thumbnail and a scrubbable filmstrip to the session manager UI. Currently I added a fallback icon that will be used when no other is available.
Added two new events fetch-version-filmstrip and fetch-version-thumbnail which gets sent when building the session manager. This is so that we can then use a custom python plugin to pick up these events and process them
Describe the reason for the change.
We want to add a new UI to the source section of the session manager so that we can have a visual of what each clip is instead of just having a title.
Describe what you have tested and on which operating system.
Tested on MacOS to see if the fallback icon is properly displayed when no plugins are available.