-
Notifications
You must be signed in to change notification settings - Fork 77
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
App 2Extended : add HLG support #268
base: master
Are you sure you want to change the base?
Conversation
Codecov Report
@@ Coverage Diff @@
## master #268 +/- ##
============================================
- Coverage 67.86% 66.54% -1.32%
Complexity 1609 1609
============================================
Files 158 160 +2
Lines 11226 11458 +232
Branches 1701 1742 +41
============================================
+ Hits 7618 7625 +7
- Misses 2798 3023 +225
Partials 810 810
Continue to review full report at Codecov.
|
@@ -22,6 +22,7 @@ | |||
Color6(ColorPrimaries.P3D65, TransferCharacteristic.SMPTEST2084, CodingEquation.None), | |||
Color7(ColorPrimaries.ITU2020, TransferCharacteristic.SMPTEST2084, CodingEquation.ITU2020NCL), | |||
Color_App5_AP0(ColorPrimaries.ACES, TransferCharacteristic.Linear, CodingEquation.None), | |||
Color8DPP(ColorPrimaries.ITU2020, TransferCharacteristic.ITU2020HLG, CodingEquation.ITU2020NCL), |
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.
Can we use the name as defined in https://github.com/SMPTE/st2067-21-2016-am1/blob/master/35PM-CD-ST-2067-21-2016-AM1.pdf, i.e. simply Color8?
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.
I'll change, it's related to firsts DPP documents which references like that. Made sense to update it.
@@ -0,0 +1,268 @@ | |||
/* |
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.
Can you avoid adding TSP 2121 code in the 2067-2 folder? Can you use a separate folder? Also can you clarify why this is not in the other PR #263 ?
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.
Right for a new folder !
This PR is only for the HLG support in App2e, but it requires updates from PR #263 that's why it's a new branch from the TSP 2121, and it includes all commits. If we merge the TSP 2121 first, this PR will contains only the latest commit.
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.
And what about the App5 stored also in the App2 folder ?
IMFErrorLogger.IMFErrors.ErrorLevels.NON_FATAL, | ||
String.format("EssenceDescriptor with ID %s is missing SubDescriptors", imageEssencedescriptorID.toString())); | ||
|
||
Integer componentDepth = getComponentDepth(); |
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.
Section 6.1.2.4.1 of ST-2067-20:2016 indicates:
"The Top-Level File Package of the Image Track File shall reference a JPEG 2000 Picture Sub Descriptor
SMPTE ST 422 as constrained by Table 13."
So it is an error in App2 and App2e (in the current published editions) to not have a subdescriptor. If this is relaxed, there should be a new namespace, etc.
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.
does it means I need to create also a CompositionImageEssenceDescriptorModel for the TSP 2121-1 ?
…pixel bit depth parsing
…Tsp2121DescriptorModel And a little refactoring
Hello @cconcolato and @MarcAntoine-Arnaud , |
Depends on #263