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

Auth48 changes #264

Closed
wants to merge 28 commits into from
Closed

Auth48 changes #264

wants to merge 28 commits into from

Conversation

dericed
Copy link
Contributor

@dericed dericed commented Jun 11, 2021

This is a part one of work to align the xml output of this repository to bring in the corrections of the auth48 process for what will be RFC9043.

ffv1.md Outdated

`YCbCr`: A reference to the method of storing the value of a Pixel by using three numeric values that represent the luma of the Pixel (Y) and the chroma of the Pixel (Cb and Cr). YCbCr word is used for historical reasons and currently references any color space relying on 1 luma Sample and 2 chroma Samples, e.g. YCbCr, YCgCo or ICtCp. The exact meaning of the three numeric values is unspecified.
YCbCr:
: A reference to the method of storing the value of a pixel by using three numeric values that represent the luma of the pixel (Y) and the chroma of the pixel (Cb and Cr). The term YCbCr is used for historical reasons and currently references any color space relying on one luma Sample and two chroma Samples, e.g., YCbCr, YCgCo (luma, blue-difference chroma, red-difference chroma), or ICtCp (intensity, blue-yellow, red-green).
Copy link
Contributor

Choose a reason for hiding this comment

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

The explanation of YCgCo is not correct. It’s the one for YCbCr.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was corrected upstream in the AUTH48 review.


This specification describes the valid bitstream and how to decode such valid bitstream. Bitstreams not conforming to this specification or how they are handled is outside this specification. A decoder could reject every invalid bitstream or attempt to perform error concealment or re-download or use a redundant copy of the invalid part or any other action it deems appropriate.
This specification describes the valid bitstream and how to decode it. Nonconformant bitstreams and the nonconformant handling of bitstreams are outside this specification. A decoder can perform any action that it deems appropriate for an invalid bitstream: reject the bitstream, attempt to perform error concealment, or re-download or use a redundant copy of the invalid part.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: there are double-spaces in between the sentences

Copy link
Contributor Author

Choose a reason for hiding this comment

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

verified that the double spaces are before and after AUTH48

ffv1.md Outdated Show resolved Hide resolved
ffv1.md Outdated Show resolved Hide resolved
ffv1.md Outdated Show resolved Hide resolved
ffv1.md Outdated Show resolved Hide resolved
Copy link
Contributor

@retokromer retokromer left a comment

Choose a reason for hiding this comment

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

a few nits

@dericed
Copy link
Contributor Author

dericed commented Jun 11, 2021

Within this pull request, I'm working according to reducing the diff between the rfc9043.xml draft and the xml make from here. I'll resolve some of the outdated comments, but others are out of scope with the PR and belong in the AUTH48 process.

@retokromer
Copy link
Contributor

retokromer commented Jun 12, 2021

others are out of scope

I am afraid, I am not sure I understand carefully. Would you prefer that I open an issue for #264 (review)? It’s just the bracket that refers to the previous colour space, not the one to which it belongs.


SVGI:!---
SVGI:![svg](rangebinaryvalues7.svg "range binary values 7")
SVGI:!---
SVGC:rangebinaryvalues7.svg=$$j_{0}=2$$
AART:j_(0) = 2
Figure: The initial value for "j", the length of the bytestream encoding. {#figureInitializeLength}
Figure: The initial value for j, the length of the bytestream encoding. {#figureInitializeLength}
Copy link
Contributor

Choose a reason for hiding this comment

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

Should it be

value for `j`, the length

?

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 don't know why this changed within the AUTH48 process. For this PR, my goal was to sync with AUTH48 so that the changes there are available for ongoing work on version 4.

Copy link
Contributor

Choose a reason for hiding this comment

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

To me it’s the same change that you made a few lines later with k, therefore I mentioned it for consistency.

Copy link
Contributor

@retokromer retokromer left a comment

Choose a reason for hiding this comment

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

One nit, otherwise LGTM.

@dericed
Copy link
Contributor Author

dericed commented Jul 2, 2021

This PR is updated according to the version of rfc9043.xml provided on June 28th. Apologies for the messy commits, it was a challenge to organize the changes clearly. The xml output of the make process of this repo still differs from the rfc9043.xml version, but in ways ways that the mmark process can not accommodate.

@dericed dericed mentioned this pull request Jul 10, 2021
@dericed
Copy link
Contributor Author

dericed commented Aug 31, 2021

will delete, as this is superseded by #266

@dericed dericed closed this Aug 31, 2021
@dericed dericed deleted the auth48-changes branch August 31, 2021 13:28
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.

2 participants