[Video] Fix convert_to_rgb channel slicing and alpha blending for RGBA videos - #48053
Conversation
guarin
left a comment
guarin
left a comment
There was a problem hiding this comment.
Thanks for the PR! Looks like a legit issue, cc @zucchini-nlp
| rgb_video = video_processor.convert_to_rgb(torch.cat([video, video[:, :1]], dim=1)) | ||
| # Test torch tensor with alpha channel | ||
| rgba_torch = torch.cat([video, torch.full_like(video[:, :1], 128)], dim=1) | ||
| rgb_video = video_processor.convert_to_rgb(rgba_torch) | ||
| self.assertEqual(rgb_video.shape, (8, 3, 20, 20)) |
There was a problem hiding this comment.
I think we can leave these lines unchanged from the original. The changed test doesn't really test anything different no?
| ], | ||
| dtype=np.uint8, | ||
| ) | ||
| rgb_np = convert_to_rgb(video_np_transparent, input_data_format="channels_last") |
There was a problem hiding this comment.
Please call video_processor.convert_to_rgb here and pass the enum as input_data_format. Let's call the output also rgb_video as further above.
| video_np_opaque = np.array( | ||
| [ | ||
| [[[255, 0, 0, 255]]], | ||
| [[[0, 255, 0, 255]]], | ||
| ], | ||
| dtype=np.uint8, | ||
| ) | ||
| rgb_np_opaque = convert_to_rgb(video_np_opaque, input_data_format="channels_last") | ||
| self.assertEqual(rgb_np_opaque.shape, (2, 3, 1, 1)) |
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
|
Thanks for the fix! I'm the reporter of #48051 — confirmed the new tests cover my original repro, and the output shape/pixel values match what I expected. One small gap: the tests cover alpha=128 (semi-transparent) and alpha=255 (opaque), but not alpha=0 (fully transparent). With alpha=0 the blended result should be pure white [255, 255, 255] — might be worth adding a case to lock in the behavior at the other end of the range. Also noticed the torch RGBA test only asserts shape, while the numpy one uses |
zucchini-nlp
left a comment
zucchini-nlp
left a comment
There was a problem hiding this comment.
Agreed with Guarin on tests that we need to check video_processor.convert_rbg as well. TBH i need to make video-processor call the existing helper, and do some general maintenance
For now we can keep as is and only fix the bug
| if video.shape[-3] == 3 or not (video[..., 3, :, :] < 255).any(): | ||
| if video.shape[-3] == 3: | ||
| return video | ||
|
|
||
| if not (video[..., 3, :, :] < 255).any(): | ||
| return video[..., :3, :, :] | ||
|
|
There was a problem hiding this comment.
nit:
if video.shape[-3] == 3 or not (video[..., 3, :, :] < 255).any():
return video[..., :3, :, :]
…m, add alpha=0 and torch value tests
|
Thanks @guarin, @zucchini-nlp, and @carlszk for the review and suggestions! Pushed an update addressing all points:
|
CI recapDashboard: View test results in Grafana |
5 participants
What does this PR do?
Fixes #48051
Problem
src/transformers/video_utils.py::convert_to_rgb,infer_channel_dimension_format(video)was called withoutnum_channels=(1, 3, 4), which raisedValueError: Unable to infer channel dimension formatfor 4-channel RGBA videos.video_utils.py::convert_to_rgb,video[..., 3, :, :](the 1-channel alpha tensor) was mistakenly multiplied as the foreground colors instead ofvideo[..., :3, :, :](the 3 RGB channels). This caused incorrect shapes (e.g.(2, 2, 1, 1)instead of(2, 3, 1, 1)) and corrupted pixel values.video_utils.py::convert_to_rgbandvideo_processing_utils.py::BaseVideoProcessor.convert_to_rgb, fully opaque RGBA inputs returnedvideowith 4 channels rather than stripping the alpha channel to return 3 RGB channels (video[..., :3, :, :]).Solution
infer_channel_dimension_format(video, num_channels=(1, 3, 4))invideo_utils.py::convert_to_rgb.video[..., :3, :, :]as foreground.video[..., :3, :, :]for opaque 4-channel RGBA inputs across both numpy (video_utils.py) and torch (video_processing_utils.py) implementations.tests/utils/test_video_utils.pycovering transparent and opaque RGBA numpy and torch inputs.Before submitting