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

added dWLm chunk #391

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

digitaltvguy
Copy link
Contributor

  1. Add dWLm chunk
  2. Added ITI BT.2408 reference
  3. Added ISO 22028-5 reference

index.html Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
<tr>
<td>203 cd/m<sup>2</sup></td>
<td>203</td>
<td>CB 00</td>
Copy link
Collaborator

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.

Copy link
Contributor

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 Show resolved Hide resolved
index.html Outdated

<tr>
<td>Diffuse white luminance metadata</td>
<td>2 bytes</td>

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.

Copy link
Contributor

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

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.

Copy link
Collaborator

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?

Copy link
Contributor

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

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

Copy link
Contributor

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.

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
<p>The two-byte chunk type field contains the hexadecimal values</p>

<pre>
<!-- xx xx xx xx
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<!-- xx xx xx xx
64 57 4C 6D

Copy link
Contributor Author

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)

Copy link
Collaborator

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.

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
Copy link
Contributor

@svgeesus svgeesus left a 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

@ProgramMax
Copy link
Collaborator

Friendly ping: There are changes requested in order for this patch to land.

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
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>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<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">
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<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>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<caption>dWLn chunk components</caption>
<caption>dWLm chunk components</caption>

</table>

<aside class="example">
Example <span class="chunk">dWLn</span> Diffuse white luminance metadata:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
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">
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<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>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<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>
Copy link
Contributor

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

@svgeesus
Copy link
Contributor

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.

@svgeesus svgeesus force-pushed the 390-add-png-dwlm-diffuse-white-luminance-metadata-chunk-to-conform-to-iso-22028-5 branch from 9f464dc to 9bec304 Compare February 20, 2024 19:19
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
</tr>

<tr>
<td>203 cd/m<sup>2</sup></td>
Copy link
Contributor

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!

Suggested change
<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>
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<td>203</td>
<td>1,000,000</td>

<tr>
<td>203 cd/m<sup>2</sup></td>
<td>203</td>
<td>CB 00</td>
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<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>
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<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.

@ProgramMax
Copy link
Collaborator

Adding a comment from our meeting today:
The dWLm chunk is for Fourth Edition, not Third. So this pull request is being delayed until Third Edition is wrapped up.
(We could maintain a separate branch but this is the easier path for now.)

@ProgramMax
Copy link
Collaborator

I think that account is a bot.
Gonna sleep on it then probably delete it's comments.

@w3c w3c deleted a comment Aug 12, 2024
@w3c w3c deleted a comment Aug 12, 2024
@w3c w3c deleted a comment Aug 12, 2024
Copy link
Contributor

@svgeesus svgeesus left a 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
@svgeesus svgeesus force-pushed the 390-add-png-dwlm-diffuse-white-luminance-metadata-chunk-to-conform-to-iso-22028-5 branch from 9bec304 to e93a3e4 Compare August 13, 2024 04:06
svgeesus and others added 3 commits August 13, 2024 07:08
Co-authored-by: Chris Needham <[email protected]>
Co-authored-by: Chris Needham <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add PNG dWLm (Diffuse white luminance metadata ) chunk to conform to ISO 22028-5
5 participants