Feature/Qt6 - Rebase to main (Jan 21 2025) #664
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cedrik-fuoco-adsk
requested review from
bernie-laberge and
eloisebrosseau
as code owners
eloisebrosseau
approved these changes
Jan 21, 2025
…SoftwareFoundation#643) ### 616: Fix MediaLibrary crash when prog source loading enabled ### Linked issues Fixes AcademySoftwareFoundation#616 ### Summarize your change. #### Note about progressive source loading in Open RV Note: Progressive source loading is NOT enabled by default in Open RV as it significantly complexifies custom scripting since many previously synchronous operations become asynchronous. For reference: https://aswf-openrv.readthedocs.io/en/latest/rv-manuals/rv-reference-manual/rv-reference-manual-chapter-two.html#progressive-source-loading Progressive source loading in RV can be enabled by setting the following environment variable: `export RV_PROGRESSIVE_SOURCE_LOADING=1` #### Problem Open RV sometimes crashes when executing the following script repeatedly: ```python from rv import commands as rvc rvc.setProgressiveSourceLoading(True) srcA = { "Streaming": "https://commondatastorage.googleapis.com/gtv-videos-bucket/sample/ForBiggerBlazes.mp4", "Frames": "https://commondatastorage.googleapis.com/gtv-videos-bucket/sample/ForBiggerBlazes.mp4" } srcAp = { "Streaming": "https://commondatastorage.googleapis.com/gtv-videos-bucket/sample/ForBiggerEscapes.mp4", "Frames": "https://commondatastorage.googleapis.com/gtv-videos-bucket/sample/ForBiggerEscapes.mp4" } rvc.addSourcesVerbose([[srcA["Streaming"], "+mediaRepName", "Small"]]) rvc.addSourceMediaRep("last", "Large", [srcA["Frames"]]) rvc.addSourcesVerbose([[srcAp["Streaming"], "+mediaRepName", "Small"]]) rvc.addSourceMediaRep("last", "Large", [srcAp["Frames"]]) ``` #### Cause When progressive source loading is enabled (which is not the default in RV), the actual loading of the media is done in worker threads in order to parallelize and thus optimize multiple source load as much as possible. The actual code executed is here: `FileSourceIPNode::openMovieTask()` This code contains MediaLibrary accesses. When RV was open sourced, the MediaLibrary code was refactored to be Python plugin based to make the MediaLibrary functionality more generic. The original implementation was c++ based and thread safe. The new implementation should not be called outside of the main thread. This is what was causing the random crash when progressive source loading was enabled. #### Solution I have extracted the MediaLibrary code and moved it into its own dedicated method named `FileSourceIPNode::lookupFilenameInMediaLibrary()`, which is now invoked from the main thread prior to launch the openMovieTasks . Note that the code is exactly like before. It is just invoked differently, that's all. ### Describe the reason for the change. Open RV sometimes crashes when adding media with progressive source loading enabled ### Describe what you have tested and on which operating system. Successfully tested on macOS ### Add a list of changes, and note any that might need special attention during the review. ### If possible, provide screenshots. Signed-off-by: Bernard Laberge <[email protected]> Signed-off-by: Cédrik Fuoco <[email protected]>
…ftwareFoundation#642) ### Fix default Python locale and now honour PYTHONUTF8 env var ### Linked issues NA ### Summarize your change. Now pre-initializing the Python interpreter in RV to ensure that the PyConfig.use_environment is set to 1 (default). Without this pre-initialization, PyConfig.use_environment defaults to 0 which means that the currently set locale gets ignored as well as the PYTHONUTF8 env var if set by the user. ### Describe the reason for the change. Since commercial RV 2023 (Python 3.9.13), the default Python locale was no longer respecting the currently set environment variables (including the PYTHONUTF8 env var) and the script described below, was always returning: US-ASCII 0 Prior to this release, such as commercial RV 2022.3.1 (Python 3.7.13), the default Python locale was respecting both the currently set environment variables, including PYTHONUTF8 env var. So we would generally have UTF-8 1 ### Describe what you have tested and on which operating system. I used this python script to validate the fix: ``` import locale import sys print(locale.getpreferredencoding(False)) print(sys.flags.utf8_mode) ``` It now returns: UTF-8 1 and now honours the PYTHONUTF8 env var export PYTHONUTF8=0 It now returns: US-ASCII 0 ### Add a list of changes, and note any that might need special attention during the review. ### If possible, provide screenshots. Signed-off-by: Bernard Laberge <[email protected]> Signed-off-by: Cédrik Fuoco <[email protected]>
…areFoundation#650) ### Add CMake custom build to the Rocky9 build instructions ### Linked issues NA ### Summarize your change. Add CMake custom build to the Rocky9 build instructions ### Describe the reason for the change. A newer version of CMake than the one coming standard with Rocky 9 is required to build an RV dependency. ### Describe what you have tested and on which operating system. Validated on Rocky 9 ### Add a list of changes, and note any that might need special attention during the review. ### If possible, provide screenshots. Signed-off-by: Bernard Laberge <[email protected]> Signed-off-by: Cédrik Fuoco <[email protected]>
…undation#652) ### Add BMD SDI output plugin support for 47.95+48 fps ### Linked issues NA ### Summarize your change. Adds the 47.95fps + 48fps support for 1080P, 2KDCI, UHD and 4KDCI to the BMD SDI output plugin. *** Note that I intentionally added the formats at the end of the list so that it does not affect any existing user preferences: video output formats are persisted via an index in the video output formats array. If the formats get reordered, this invalidates any existing video format preference which is a real annoyance to users. Adding the new formats at the end of the list prevents this. ### Describe the reason for the change. These missing formats were supported by the Blackmagic DeckLink SDK but had never beed added to RV until today. ### Describe what you have tested and on which operating system. Successfully tested the new 47.95fps + 48fps formats on macOS using a Blackmagic Decklink 8K Pro and verified the output via a Blackmagic Smartview 4K monitor. ### Add a list of changes, and note any that might need special attention during the review. ### If possible, provide screenshots. Signed-off-by: Bernard Laberge <[email protected]> Signed-off-by: Cédrik Fuoco <[email protected]>
- [x] Update the SDK version in aja.cmake and fix issues related with OS specific header files that are no longer copied and the addition of the mbedtls library that we need to link against - [x] Replace code related to `ScanHardware()` and `GetDeviceInfoList()` since both functions have been deprecated The SDK needs to be at the latest version. Presentation mode with the AJA Io 4K Plus device with HDMI and SDI on MacOS. --------- Signed-off-by: Éloïse Brosseau <[email protected]> Signed-off-by: Cédrik Fuoco <[email protected]>
AcademySoftwareFoundation#644) I think using the toolchain argument is a much simpler and preferred solution to changing the system configuration for the VC tools. This is only relevant for Windows. Signed-off-by: mcoms <[email protected]> Co-authored-by: Bernard Laberge <[email protected]> Signed-off-by: Cédrik Fuoco <[email protected]>
Initialize Python in rvls main function to match the behaviour of rvio inside the ShotGrid license validation. The license validation was failing on Linux when using rvls, but the same code was working fine with rvio because of the Python initialization in main(). The Python interpreter from rvio had more capabilities that were needed to properly import Python modules on Linux. This change was tested on Rocky8, MacOS Sonoma and Windows 11. Co-authored-by: Bernard Laberge <[email protected]> Signed-off-by: Cédrik Fuoco <[email protected]>
…demySoftwareFoundation#647) ### Add default global scale value if missing ### Summarize your change. A default global scale value is calculated based on the aspect ratio of the first source if it couldn't be obtained from the OTIO. ### Describe the reason for the change. When no `available_image_bounds` is present in the OTIO, no global scale value was being calculated. This was causing an issue where the annotations couldn't be displayed in RV during a Live Review Session. ### Describe what you have tested and on which operating system. Successfully test on MacOS --------- Signed-off-by: Éloïse Brosseau <[email protected]> Signed-off-by: Cédrik Fuoco <[email protected]>
### Update ClangFormat ### Summarize your change. The .clang-format was updated to the latest version and added to the .pre-commit-config.yaml. I also modernized the formatting style based on LLVM. Every options related to the include statements formatting was removed since it might have some side effects on some legacy parts of the codebase if they were to be reorder. Once this PR is merged, the same formatting should be applied to the commercial RV repository for consistency. Moreover, I suggest to make another PR to format all the files afterwards and to add a `.git-blame-ignore-revs` to avoid having the formatting commit show up when using `git blame`. Ideally, once this is done, we should add a formatting validation in our CI pipeline to avoid relying only on users installing the pre-commit hook to follow the formatting convention. ### Describe the reason for the change. The .clang-format had a lot of deprecated options and not all the files were formatted the same way. ### If possible, provide screenshots. Here's an example of what a formatted file would look like with the new .clang-format. Please let me know if you don't agree with any of the chosen options.     --------- Signed-off-by: Éloïse Brosseau <[email protected]> Signed-off-by: Cédrik Fuoco <[email protected]>
### Add a .clang-tidy to lint the C++ code ### Summarize your change. A .clang-tidy was added at the root of the project to lint our C++ code. We should do the same for commercial RV if everyone is happy with this one. The formatStyle is set to 'file' so that the changes suggested by the linter will match the rules in our .clang-format. WarningAsErrors could be changed to match all or any of the checks if, at one point, we would want to enforce the respect of a category. HeaderFilterRegex could be changed if we need to exclude some header files from being lint by clang-tidy. ### Describe the reason for the change. Just like we are formatting our code with the .clang-format, we should also lint it using a .clang-tidy. This is the one I've been using personally but I think it could be beneficial to others, especially for code reviews since it would help to avoid having to ask for small changes that could have been easily catch by a linter. Ideally, we would also add it to our CI pipeline so that we don't only rely on people generating a compile_commands.json to use it and making the changes suggested to the code by the linter. Here's the documentation ### If possible, provide screenshots. Here's an example of what the linter messages could look like: <img width="1255" alt="Screenshot 2024-12-04 at 3 42 17 PM" src="https://github.com/user-attachments/assets/7eab6730-f46e-4535-8b0a-62c083e5857e"> Signed-off-by: Éloïse Brosseau <[email protected]> Signed-off-by: Cédrik Fuoco <[email protected]>
…ation#655) ### Fix Open RV not launching on macOS Sequoia 15.2 ### Linked issues NA ### Summarize your change. Increased the Garbage Collector's maximum number of roots from 2048 to 4096 by enabling the large config option in the garbage collector config. ### Describe the reason for the change. RV+Open RV could no longer launch since Apple released macOS Sequoia 15.2 (2 days ago). The following fatal error was reported by the garbage collector component of RV: "Too many root sets" ### Describe what you have tested and on which operating system. Successfully tested on macOS Sequoia 15.2. ### Add a list of changes, and note any that might need special attention during the review. ### If possible, provide screenshots. Signed-off-by: Bernard Laberge <[email protected]> Signed-off-by: Cédrik Fuoco <[email protected]>
### Clean up the OTIO reader annotation This PR aims to clean up a bit the code related to the OTIO reader for annotations in Live Review. ### Question I see that some files for other hooks and schemas are in camel case instead of snake case. Does anyone knows if there is a reason? Is everyone ok with me renaming the files inside `otio_reader` to use snake case instead? --------- Signed-off-by: Éloïse Brosseau <[email protected]> Signed-off-by: Cédrik Fuoco <[email protected]>
### Fix AJA not working on Windows ### Summarize your change. The GLEW library was not initialized before using other GLEW extension ### Describe the reason for the change. The AJA output module was having the same issue as the one mention in the following PR with BlackMagic: [[AcademySoftwareFoundation#116](https://git.autodesk.com/media-and-entertainment/rv/pull/116)](https://git.autodesk.com/media-and-entertainment/rv/pull/116). ### Describe what you have tested and on which operating system. Starting the presentation mode on Windows 11 with an AJA Io 4K Plus device does not make RV crash anymore. Signed-off-by: Éloïse Brosseau <[email protected]> Signed-off-by: Cédrik Fuoco <[email protected]>
…ation#662) <!-- Thanks for your contribution! Please read this comment in its entirety. It's quite important. When a contributor merges the pull request, the title and the description will be used to build the merge commit! ### Pull Request TITLE It should be in the following format: [ 12345: Summary of the changes made ] Where 12345 is the corresponding Github Issue OR [ Summary of the changes made ] If it's solving something trivial, like fixing a typo. --> ### Linked issues <!-- Link the Issue(s) this Pull Request is related to. Each PR should link to at least one issue, in the form: Use one line for each Issue. This allows auto-closing the related issue when the fix is merged. Fixes #12345 Fixes #54345 --> Fixes AcademySoftwareFoundation#600 ### Summarize your change. add fix for mov container files, using avg_frame_rate instead of time_base ### Describe the reason for the change. The ffmpeg update causes this issue. ### Describe what you have tested and on which operating system. Tested on: Windows 10, Centos7, AlmaLinux9 Tested for .mov, .mp4, .mxf (not affected by change) --------- Signed-off-by: Mirco Tornow <[email protected]> Signed-off-by: Cédrik Fuoco <[email protected]>
…eFoundation#661) ### Fix OCIO SYNDisplay/SYNLinearize nodes causing freeze ### Linked issues NA ### Summarize your change. Make the mutex (QMutex) used by OCIOIPNode recursive. ### Describe the reason for the change. Prior to this fix, using an OCIO SYNDisplay or SYNLinearize nodes was freezing OpenRV. This was an unexpected regression of a previous fix : AcademySoftwareFoundation@3ec54d2 The problem was that the OCIO SYNDisplay/SYNLinearize were locking the same mutex twice by design, thus leading to the freeze. ### Describe what you have tested and on which operating system. Successfully tested on macOS ### Add a list of changes, and note any that might need special attention during the review. ### If possible, provide screenshots. Signed-off-by: Bernard Laberge <[email protected]> Signed-off-by: Cédrik Fuoco <[email protected]>
Signed-off-by: Cédrik Fuoco <[email protected]>
Signed-off-by: Cédrik Fuoco <[email protected]>
cedrik-fuoco-adsk
force-pushed
the
qt6-rebase-to-main-Jan21-2025
branch
from
a392f36 to
dff0311
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Feature/Qt6 - Rebase to main (Jan 21 2025)
Linked issues
n/a
Describe the reason for the change.
Rebase to main