Skip to content
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

Improve color accuracy of BT709 frames on CUDA #372

Merged
merged 2 commits into from
Nov 13, 2024

Conversation

ahmadsharif1
Copy link
Contributor

@ahmadsharif1 ahmadsharif1 commented Nov 13, 2024

libnpp actually has a specialized function to handle images with BT709 colorspace. So we use that instead of the generic one when appropriate. This improves the color accuracy of the image slightly.

Hat tip to @fmassa

Drive-by: fixed a test that doesn't make sense on CUDA -- we can only expect exactly equal frames on CPU.

Visual inspection. Before on the left and after on the right:

image

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Meta Open Source bot. label Nov 13, 2024
Copy link
Member

@fmassa fmassa left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@@ -44,7 +44,7 @@ def assert_tensor_equal(*args, **kwargs):

# Asserts that at least `percentage`% of the values are within the absolute tolerance.
def assert_tensor_close_on_at_least(
actual_tensor, ref_tensor, percentage=90, abs_tolerance=20
actual_tensor, ref_tensor, percentage=90, abs_tolerance=19
Copy link
Member

Choose a reason for hiding this comment

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

This tolerance is still a bit high I think, so might be good to see if it is due to limited color range vs full color range.

I had some good results with limited color range more closely matching the output of some videos.

The code I used was in https://github.com/fairinternal/amaia/blob/fmassa/video_reader/amaia_video/nv12_to_rgb.py

Copy link

@pjs102793 pjs102793 Nov 28, 2024

Choose a reason for hiding this comment

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

@fmassa

In the torchvision decoder, this function was used. According to the NPP Documentation, it is noted that HDTV conversion assumes a full color range of 0 - 255; use the CSC version for limited range color.

Maybe using this function seems to significantly improve issues with color discrepancies.

Choose a reason for hiding this comment

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

When I directly replaced the function with the CSC function, the difference in values between the CPU and CUDA results was only 2 across all values. While further validation is necessary, it seems much more accurate compared to a difference of around 20. I will provide more details in the issue.

@ahmadsharif1 ahmadsharif1 merged commit c446e72 into pytorch:main Nov 13, 2024
38 of 43 checks passed
NicolasHug pushed a commit to NicolasHug/torchcodec that referenced this pull request Nov 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Meta Open Source bot.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants