-
Notifications
You must be signed in to change notification settings - Fork 49
Allow review/transcoding of more channels, like "Z", "Y", "XYZ", "AR", "AG" "AB" #1271
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
…e RGBA channels in source media instead of raising error
…iles of the source representation but used for multiple output profiles
…fix/transcode_ignore_conversion_on_unknown_channel # Conflicts: # client/ayon_core/plugins/publish/extract_color_transcode.py
…code-failing-to-transcode-media-blocking-publishes-2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request adds support for transcoding and reviewing additional channel names (e.g. "Z", "Y", "XYZ", "AR", "AG", "AB") in EXR files.
- Updated allowed channel names for detecting reviewable channels.
- Replaced the previous per-layer RGBA extraction with a more flexible channels-by-layer approach and implemented new prioritization/sorting for layer names supporting varied channel configurations.
- Modified error messaging to include the channel names present if no suitable channels are found.
…on_on_unknown_channel' into 989-ay-7315_extract-review-and-oiio-transcode-failing-to-transcode-media-blocking-publishes-2 # Conflicts: # client/ayon_core/lib/transcoding.py
…o-transcode-media-blocking-publishes-2' of https://github.com/BigRoy/ayon-core into 989-ay-7315_extract-review-and-oiio-transcode-failing-to-transcode-media-blocking-publishes-2
…code-failing-to-transcode-media-blocking-publishes-2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enhances EXR transcoding to support additional channel types beyond RGBA (e.g., Z, Y, XYZ groups, AR/AG/AB) by extending channel-detection logic and adding error handling for missing channels.
- Introduces
MissingRGBAChannelsError
and updatesconvert_colorspace
calls to catch and skip on missing reviewable channels. - Extends
get_review_info_by_layer_name
to detect XYZ, colored mattes (AR/AG/AB), luminance (Y), and depth (Z) channel sets, and adds a sort priority for layers. - Updates the extract/transcode plugin to use the new exception and avoid errors when uncommon channel combinations are encountered.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
client/ayon_core/plugins/publish/extract_color_transcode.py | Added exception handling around convert_colorspace to skip representations with missing channels. |
client/ayon_core/lib/transcoding.py | Defined new MissingRGBAChannelsError , enhanced get_review_info_by_layer_name for extra channel types, and raised the new exception in get_oiio_input_and_channel_args . |
Comments suppressed due to low confidence (2)
client/ayon_core/plugins/publish/extract_color_transcode.py:169
- [nitpick] The variable name
unknown_rgba_channels
could be clearer—consider renaming tomissing_rgba_channels
to match theMissingRGBAChannelsError
exception.
unknown_rgba_channels = False
client/ayon_core/lib/transcoding.py:418
- New branching logic handles AR/AG/AB, Y, and Z channel sets; consider adding unit tests to cover these cases and ensure correct behavior.
elif all(channel in channel_info for channel in ("AR", "AG", "AB")):
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not tested but LGTM
"b", "blue", | ||
"a", "alpha" | ||
# Detect RGBA channels | ||
"r", "g", "b", "a", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You removed "red"
, "green"
, "blue"
and "alpha"
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's correct - but if you check the old code.. it'd have never been used. Because at line 389-391 it would only ever have used R, G, B. And since we stored them as .upper()
it should've then also checked RED
, GREEN
, BLUE
keys, right?
So I'm just removing it because it was never used... and I couldn't find any example files that actually had those channel names
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is was using first character to store it for mapping, but this condition is to skip the channel if does not match the items in set -> 2 different things. Put it back please. Because you've changed the logic you broke it, so also resolve that plase...
(e.g. Beauty.red
would be skipped)
…o-transcode-media-blocking-publishes-2' of https://github.com/BigRoy/ayon-core into 989-ay-7315_extract-review-and-oiio-transcode-failing-to-transcode-media-blocking-publishes-2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR expands the transcoding review functionality to support additional channel types (e.g., "Z", "Y", "XYZ", "AR", "AG", "AB") and refines error handling when suitable channels are not detected. Key changes include:
- Addition of tests verifying behavior for new channel types.
- Introduction of the MissingRGBAChannelsError custom exception for signaling missing reviewable channels.
- Modifications to get_review_info_by_layer_name to support extended channel detection and prioritization.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
File | Description |
---|---|
tests/client/ayon_core/lib/test_transcoding.py | Added tests covering new channel scenarios and ensuring RGB priority over Z channels. |
client/ayon_core/plugins/publish/extract_color_transcode.py | Updated file conversion logic and error handling with MissingRGBAChannelsError. |
client/ayon_core/lib/transcoding.py | Extended channel detection in get_review_info_by_layer_name and added a custom exception. |
Comments suppressed due to low confidence (2)
client/ayon_core/lib/transcoding.py:392
- Using only the first character of 'last_part' loses information for composite channel names (e.g., 'XYZ', 'AR'). Consider using last_part.upper() to preserve the full channel identifier.
channel = last_part[0].upper()
tests/client/ayon_core/lib/test_transcoding.py:90
- Consider adding tests for composite channel names (e.g., 'XYZ', 'AR') to verify that the updated channel detection logic handles them correctly.
def test_rgba_priority(self):
Added unittest with some "example channel names". Note that it does not actually go through testing EXR files, only by some channel names. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR extends the EXR transcoding functionality to support additional channel names such as "Z", "Y", "XYZ", "AR", "AG", and "AB". Key changes include:
- Updated tests in tests/client/ayon_core/lib/test_transcoding.py to validate handling of esoteric channels.
- Enhanced channel detection and prioritization logic in get_review_info_by_layer_name within transcoding.py.
- Integrated error handling for missing RGBA channels in extract_color_transcode.py by catching a new custom MissingRGBAChannelsError.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
tests/client/ayon_core/lib/test_transcoding.py | Added tests for handling multiple channel configurations. |
client/ayon_core/plugins/publish/extract_color_transcode.py | Updated file processing logic and error handling with the new error type. |
client/ayon_core/lib/transcoding.py | Introduced MissingRGBAChannelsError and revised channel mapping and prioritization logic. |
Comments suppressed due to low confidence (1)
client/ayon_core/plugins/publish/extract_color_transcode.py:193
- [nitpick] Consider consolidating the two consecutive error log statements into a single log statement to improve log clarity and reduce redundancy.
self.log.error("Skipping OIIO Transcode. Unknown RGBA channels"
Co-authored-by: Copilot <[email protected]>
Changelog Description
Allow review/transcoding of more channels, like "Z", "Y", "XYZ", "AR", "AG" "AB"
Additional info
Do not error if review or transcoding happens on EXRs that only contain e.g. "Z" channel or "Y" channels.
Allow more channel names to pass through.
Fix #989
Testing notes: