-
Notifications
You must be signed in to change notification settings - Fork 12
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
added dWLm chunk #391
base: main
Are you sure you want to change the base?
added dWLm chunk #391
Conversation
digitaltvguy
commented
Dec 10, 2023
- Add dWLm chunk
- Added ITI BT.2408 reference
- Added ISO 22028-5 reference
<tr> | ||
<td>203 cd/m<sup>2</sup></td> | ||
<td>203</td> | ||
<td>CB 00</td> |
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 need to double check the endianness here.
It might be 00 CB?
I'll get back to you on that.
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 should be a PNG unsigned four byte integer anyway, like cLLi
index.html
Outdated
|
||
<tr> | ||
<td>Diffuse white luminance metadata</td> | ||
<td>2 bytes</td> |
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.
Here's the HEIF TuC document that contains proposed language for this same piece of metadata (look for ndwt
):
https://www.mpeg.org/wp-content/uploads/mpeg_meetings/144_Hannover/w23211.zip
I think it would be beneficial to make this a uint32_t
with a unit of 0.0001 cd/m^2 so that we could have lossless metadata conversion between PNG and HEIF.
I have pinged folks internally on whether diffuse_white_light_x/y
would be good additions as well.
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 agree, and 4 bytes unsigned integer with that divisor would be consistent with cLLi
as well
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.
@podborski mentioned the following regarding the x/y values:
IIRC it was bound to the reference viewing environment and was specified in 22028-5 and we needed a way to carry that metadata in HEIF.
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'm not sure I follow. And I don't have access to ISO 22028-5 to check for myself.
Does 22028-5 specify 4 bytes here? Thus, this comment would put us in line with both 22028-5 and HEIF?
Or does 22028-5 specify 2 bytes and HEIF is different from 22028-5?
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'm not sure I follow. And I don't have access to ISO 22028-5 to check for myself. Does 22028-5 specify 4 bytes here? Thus, this comment would put us in line with both 22028-5 and HEIF? Or does 22028-5 specify 2 bytes and HEIF is different from 22028-5?
From ISO/DTS 22028-5:
4.5.5 Default nominal diffuse white luminance
The default nominal diffuse white luminance shall be 203 cd/m2.
NOTE This nominal luminance value is defined in ITU-R BT.2408-4 as the HDR Reference White.
If the reference nominal diffuse white for specific content has a different displayed luminance, the
reference nominal diffuse white for that content should be indicated using metadata, as defined in 4.6.5.
and then
4.6.5 Diffuse white luminance metadata
The diffuse white luminance metadata stores the luminance value of the reference nominal diffuse
white, in cd/m2.
If the luminance of the diffuse white differs from the default reference display nominal diffuse white
luminance defined in 4.5.5, it should be indicated using metadata.
So the encoding is not defined at all. Also, it is clear that this metadata is only to be used when media white has a luminance other than 203 cd/m2
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.
The diffuse_white_light_x/y
fields have been removed from the HEIF text. Latest text can be found here:
https://dms.mpeg.expert/doc_end_user/documents/147_Sapporo/wg11/MDS24143_WG03_N01297.zip
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 looks like a good change to me. Merely noting a different white point without also specifying a chromatic adaptation transform (CAT) is not especially helpful, and the most useful information (for PQ, at least) is the absolute luminance of the media white.
<p>The two-byte chunk type field contains the hexadecimal values</p> | ||
|
||
<pre> | ||
<!-- xx xx xx xx |
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.
<!-- xx xx xx xx | |
64 57 4C 6D |
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.
Suggest renaming dWLm chunk to conform with HEIF effort which labels it ndwt (Nominal Diffuse White)
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 support this idea: nDWt.
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.
Field should be uint32_t
Friendly ping: There are changes requested in order for this patch to land. |
diffuse white as defined in in [[ITU-R-BT.2408]], the contents diffuse white luminance level | ||
should be indicated using this metadata.</p> | ||
|
||
<p>The following specifies the syntax of the <span class="chunk">dWLn</span> chunk:</p> |
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.
<p>The following specifies the syntax of the <span class="chunk">dWLn</span> chunk:</p> | |
<p>The following specifies the syntax of the <span class="chunk">dWLm</span> chunk:</p> |
|
||
<p>The following specifies the syntax of the <span class="chunk">dWLn</span> chunk:</p> | ||
|
||
<table id="dWLn-chunk-syntax" class="numbered simple"> |
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.
<table id="dWLn-chunk-syntax" class="numbered simple"> | |
<table id="dWLm-chunk-syntax" class="numbered simple"> |
<p>The following specifies the syntax of the <span class="chunk">dWLn</span> chunk:</p> | ||
|
||
<table id="dWLn-chunk-syntax" class="numbered simple"> | ||
<caption>dWLn chunk components</caption> |
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.
<caption>dWLn chunk components</caption> | |
<caption>dWLm chunk components</caption> |
</table> | ||
|
||
<aside class="example"> | ||
Example <span class="chunk">dWLn</span> Diffuse white luminance metadata: |
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.
Example <span class="chunk">dWLn</span> Diffuse white luminance metadata: | |
Example <span class="chunk">dWLm</span> Diffuse white luminance metadata: |
<aside class="example"> | ||
Example <span class="chunk">dWLn</span> Diffuse white luminance metadata: | ||
|
||
<table id="dWLn-diffuse-white-luminance-metadata-example" class="numbered simple"> |
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.
<table id="dWLn-diffuse-white-luminance-metadata-example" class="numbered simple"> | |
<table id="dWLm-diffuse-white-luminance-metadata-example" class="numbered simple"> |
<table id="dWLn-diffuse-white-luminance-metadata-example" class="numbered simple"> | ||
<tr> | ||
<th>Actual value</th> | ||
<th>Stored Decimal value</th> |
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.
<th>Stored Decimal value</th> | |
<th>Stored decimal value</th> |
(although I would prefer to remove this column, as it's redundant because it shows the same information as Actual value)
<tr> | ||
<td>203 cd/m<sup>2</sup></td> | ||
<td>203</td> | ||
<td>CB 00</td> |
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 should be a PNG unsigned four byte integer anyway, like cLLi
We should note that ISO 22028-5 is not a freely available specification. It costs 128 swiss francs. However, it is pretty much an interoperability profile of ITU BT.2100-2 plus a reference display, reference viewing conditions, and mandated MDCV metadata. ISO 22028-5 explicitly does not define what you do with the metadata. |
9f464dc
to
9bec304
Compare
</tr> | ||
|
||
<tr> | ||
<td>203 cd/m<sup>2</sup></td> |
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.
203 is a poor example because this metadata is to be used when the diffuse white luminance is not 203!
<td>203 cd/m<sup>2</sup></td> | |
<td>100 cd/m<sup>2</sup></td> |
|
||
<tr> | ||
<td>203 cd/m<sup>2</sup></td> | ||
<td>203</td> |
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.
<td>203</td> | |
<td>1,000,000</td> |
<tr> | ||
<td>203 cd/m<sup>2</sup></td> | ||
<td>203</td> | ||
<td>CB 00</td> |
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.
<td>CB 00</td> |
We don't need to say how a uint32_t works, do we?
<tr> | ||
<th>Actual value</th> | ||
<th>Stored Decimal value</th> | ||
<th>Stored 2-byte unsigned value</th> |
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.
<th>Stored 2-byte unsigned value</th> |
We don't need to say how a uint32_t works, do we? The other chunks don't.
Adding a comment from our meeting today: |
I think that account is a bot. |
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.
As noted inline
1. Add dWLm chunk 2. Added ITI BT.2408 reference 3. Added ISO 22028-5 reference
9bec304
to
e93a3e4
Compare
Co-authored-by: Chris Needham <[email protected]>
Co-authored-by: Chris Needham <[email protected]>
Co-authored-by: Chris Needham <[email protected]>