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

Clarifies fallback strategy and editorial clean-up #4

Merged

Conversation

palemieux
Copy link
Contributor

Closes #3

@palemieux palemieux self-assigned this Jun 23, 2017
@palemieux palemieux requested a review from svgeesus June 23, 2017 18:26
index.html Outdated
<p class="note">The <code>gAMA</code> and embedded ICC profile are provided solely for compatibility with processors that do
not conform to this specification.</p>
<p class="note">The <code>gAMA</code> chunk, <code>cHRM</code> chunk, and embedded ICC profile specified in the table above are
not necessarily accurate, and, even if accurate, their processing is not required to reproduce the image as intended by the
Copy link
Contributor

Choose a reason for hiding this comment

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

Not necessarily? They are known to be inaccurate.
Suggest "... in the table above are inaccurate, due to limitations in ICC V4 with HDR images. However, their processing is not required ..."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@svgeesus Yes, at this point, all are inaccurate. I used not necessarily to keep the door open for some of these parameters to be accurate if/when more rows are added to the table in the future.

Copy link
Contributor

Choose a reason for hiding this comment

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

I prefer to clearly state that the current ones are known to be very inaccurate.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok. cHRM is in fact accurate (it reflects Rec 2020 primaries), so I plan to revise the note to state that ICC and gamma are inaccurate.

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.

Overall I am much happier with this specification given these changes. See review comments for a small modification.

index.html Outdated
not conform to this specification.</p>
<p class="note">The <code>gAMA</code> chunk, <code>cHRM</code> chunk, and embedded ICC profile specified in the table above are
not necessarily accurate, and, even if accurate, their processing is not required to reproduce the image as intended by the
author. They are provided for graceful fallback for implementations that do not recognize the semantics associated with a
Copy link
Contributor

Choose a reason for hiding this comment

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

Positioning these as fallback is good. I don't know if it is graceful however. You end up with an SDR image with a peak luminance of 80 cd/m^2,, right? So the whole thing looks super dim?

Copy link
Contributor

@svgeesus svgeesus Jul 4, 2017

Choose a reason for hiding this comment

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

(That isn't a change request btw, just that I would like to understand the fallback better.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You end up with an SDR image with a peak luminance of 80 cd/m^2,, right?

Well SDR systems typically scale peak sRGB luminance to their peak luminance, so I would expect the same to apply here.

Copy link
Contributor

Choose a reason for hiding this comment

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

I meant in the case where a system with HDR capability uses the fallback, because it does not follow this spec..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I meant in the case where a system with HDR capability uses the fallback, because it does not follow this spec..

I do not think it is possible to predict exactly what HDR systems will do when processing an ICC profile that was not designed for HDR systems... some might map the 80 nits of the ICC profile to what peak SDR luminance would be in SDR mode, others might map it to 80 nits and others might map it to peak HDR luminance.

index.html Outdated
not necessarily accurate, and, even if accurate, their processing is not required to reproduce the image as intended by the
author. They are provided for graceful fallback for implementations that do not recognize the semantics associated with a
<code>iCCP</code> Chunk Profile Names defined by this specification. Future versions of this specification might include
additional ICC profiles, e.g. to reflect improvements in ICC profile capabilities.</p>
Copy link
Contributor

Choose a reason for hiding this comment

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

Like that part, it allows a V5 profile to be added later which actually describes the image.

@palemieux
Copy link
Contributor Author

@svgeesus See update text.

@svgeesus
Copy link
Contributor

svgeesus commented Jul 7, 2017

r=me good to go

@palemieux palemieux merged commit baa26cb into master Jul 7, 2017
@svgeesus svgeesus deleted the issue-0001-fallback-clarification+editorial-cleanup branch July 7, 2017 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants