Skip to content

cudacodec::VideoWriter add flag for writing full range luma chroma video#3868

Merged
asmorkalov merged 1 commit intoopencv:4.xfrom
cudawarped:cudacodec_videowriter_add_full_range_flag
Jan 16, 2025
Merged

cudacodec::VideoWriter add flag for writing full range luma chroma video#3868
asmorkalov merged 1 commit intoopencv:4.xfrom
cudawarped:cudacodec_videowriter_add_full_range_flag

Conversation

@cudawarped
Copy link
Copy Markdown
Contributor

@cudawarped cudawarped commented Jan 9, 2025 •
edited
Loading

Currently the meta data in all encoded video's is not updated to reflect the range (full of limited as defined in Annex E of the ITU-T Specification) of encoded luma/chroma values. With all video's indicating that they use the limited range. This means that when a full range YUV source is encoded the decoder will incorrectly decode it to a limited range resulting in poor color reproduction.

This PR correctly sets this meta data field for all encoded video's.

Pull Request Readiness Checklist

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

  • I agree to contribute to the project under Apache 2 License.
  • To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV
  • The PR is proposed to the proper branch
  • There is a reference to the original bug report and related work
  • There is accuracy test, performance test and test data in opencv_extra repository, if applicable
    Patch to opencv_extra has the same branch name.
  • The feature is well documented and sample code can be built with the project CMake

asmorkalov
asmorkalov reviewed Jan 15, 2025
View reviewed changes
@cudawarped cudawarped force-pushed the cudacodec_videowriter_add_full_range_flag branch from dacc035 to c8ff1a7 Compare January 15, 2025 11:25
@cudawarped
Copy link
Copy Markdown
Contributor Author

cudawarped commented Jan 15, 2025

@asmorkalov I've added a warning when the codec is not h264 or h265.

Note: Because I can't test AV1 encoding (I don't have access to a card with 8th Gen NVDEC) the cudacodec module does not currently support AV1 encoding. This is why I haven't also added AV1 support for the videoFullRangeFlag (NV_ENC_CONFIG::encodeConfig->encodeCodecConfig.av1Config.colorRange) flag in this PR.

@asmorkalov
Copy link
Copy Markdown
Contributor

asmorkalov commented Jan 15, 2025

You actually can submit the PR with GF 4x support. I have a device for test.

@cudawarped
Copy link
Copy Markdown
Contributor Author

cudawarped commented Jan 15, 2025

That's good to know although I'll need access to one first to test everything locally first before submitting a PR.

@asmorkalov asmorkalov self-assigned this Jan 15, 2025
@cudawarped
cudacodec: Add flag to signal whether the luma and chroma samples in…
b4a91cd 
… the yuv frames passed to VideoWriter occupy the full or limited range as defined by the Annex E of the ITU-T Specification.
@asmorkalov
Copy link
Copy Markdown
Contributor

asmorkalov commented Jan 15, 2025

@cudawarped There is conflict with the previous PR please rebase. Also videoFullRangeFlag looks ugly for me. It's bool and it's obviously flag. I propose to rename it to something like encodeVideoFullRange.

@cudawarped
Copy link
Copy Markdown
Contributor Author

cudawarped commented Jan 15, 2025

@asmorkalov Correct, I copied the name from the SDK where its a flag (unsigned char video_full_range_flag in NVDEC and uint32_t videoFullRangeFlag in NVENC). Additionaly as I mentioned above this is refered to as uint32_t colorRange for AV1 encoding so a differently named bool here would make sense. I tried to keep the naming the same as the SDK for transparency but I can change this if its less confusing?

For consistency should I also rename it in VideoReader to videoFullRangeSource or similar?

@asmorkalov
Copy link
Copy Markdown
Contributor

asmorkalov commented Jan 15, 2025

videoFullRangeSource looks good.

@asmorkalov
Copy link
Copy Markdown
Contributor

asmorkalov commented Jan 15, 2025

Oh, got the issue. Let it be videoFullRangeFlag everywhere to not break code and confuse users.

@cudawarped cudawarped force-pushed the cudacodec_videowriter_add_full_range_flag branch from c8ff1a7 to b4a91cd Compare January 15, 2025 13:41
@asmorkalov asmorkalov merged commit 2af5458 into opencv:4.x Jan 16, 2025
@tvercaut tvercaut mentioned this pull request Feb 4, 2025
Closed
@asmorkalov asmorkalov mentioned this pull request Feb 18, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@asmorkalov asmorkalov asmorkalov approved these changes

Assignees

@asmorkalov asmorkalov

Labels

category: cuda

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@cudawarped @asmorkalov