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

More figures and tables titles #265

Merged
merged 1 commit into from
Jul 9, 2021

Conversation

JeromeMartinez
Copy link
Contributor

No description provided.

Copy link
Contributor

@dericed dericed left a comment

Choose a reason for hiding this comment

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

Looks good to me. Some redundancy but I agree that having captions for tables and figures is a good goal and in a few places, they more a greater level of clarity. I recommend merging this one first and I'll rebase and clean up #264 when the AUTH48 is complete.

@dericed
Copy link
Contributor

dericed commented Jul 7, 2021

https://www.rfc-editor.org/auth48/rfc9043 very close :)

ffv1.md Outdated
@@ -288,6 +288,7 @@ SVGI:![svg](quantizationtablesets.svg "quantization table sets")
SVGI:!---
SVGC:quantizationtablesets.svg=$$Q_{j}[k]=quant\\_tables[i][j][k\\&255]$$
AART:Q_(j)[k] = quant_tables[i][j][k&255]
Figure: Description of the coding of the Quantized Sample Difference. {#figureQuantizationTableSets}
Copy link
Member

Choose a reason for hiding this comment

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

This can be read as Quantized (Sample Difference) and (Quantized Sample) Difference, this should be worded so as to be free of this double interpretation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

"Quantized Sample Difference" is the wording used in the previous paragraph (actually already 6 times in the spec), should it be changed everywhere? It is a bit much as we are in the final part of the RFC, I am reluctant to change everywhere or use a wording not same as elsewhere.

Copy link
Member

Choose a reason for hiding this comment

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

does each occurance of "Quantized Sample Difference" refer to the same thing ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

does each occurance of "Quantized Sample Difference" refer to the same thing ?

All but one are at the same place (in "Quantization Table Sets" part or just after, in "Context"), the other one is in "quant_tables" part which refers to the same topic, as I understand.

Copy link
Member

Choose a reason for hiding this comment

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

The quantization table converts sample differences in quantized (sample differences). But the storage of the quantization table has no samples, it stores really the quantization thresholds or levels or their differences. So iam a bit confused by the wording and what "Quantized Sample Difference" refers to here

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 modified the patch accordingly.

Copy link
Member

@michaelni michaelni Jul 9, 2021

Choose a reason for hiding this comment

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

From my reading of Jerome's PR, I presumed that "Quantized Sample Difference" referred back to the expression of the same phrase earlier in the section.

Quantization Tables are used on Sample Differences (see (#coding-of-the-sample-difference)), so Quantized Sample Differences are stored in the bitstream.

Where are the Quantized Sample Differences stored ?

If i look at the text its not clearly worded

Relative to any Sample X, the Quantized Sample Differences L-l, l-tl, tl-t, T-t, and t-tr are used as context:

L-l, l-tl, tl-t, T-t, and t-tr are Sample Differences, they become Quantized when passing through the quantization table, thats why its called a quantization table.

Maybe iam missing something but if not then the followig sounds more correct and maybe something like this could be used:
Figure: Description of the mapping from sample differences to the corresponding Quantized Sample Differences

Copy link
Contributor

Choose a reason for hiding this comment

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

I updated @JeromeMartinez's patch per @michaelni's proposal.

Copy link
Contributor

Choose a reason for hiding this comment

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

Acknowledging the @JeromeMartinez is afk but okays change with google hangouts discussion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just retrieved my credentials :). Fine for me.

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

dericed commented Jul 8, 2021

The updates look good to me.

@dericed
Copy link
Contributor

dericed commented Jul 8, 2021

I see that Jean updated the xml. I'll offer that once this PR is merged, I'll reply to Jean with directions on what edits to make to rfc9043.xml to correlate to this PR. Is any further discussion here needed though?

Copy link
Contributor

@dericed dericed left a comment

Choose a reason for hiding this comment

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

LGTM, thanks Jerome. I'll await @michaelni's review and if merged, I'll prep a change request for the AUTH48 editors.

@michaelni michaelni merged commit e37bdae into FFmpeg:master Jul 9, 2021
@michaelni
Copy link
Member

I merged this so we can move forward but maybe some of this text can be worded better, i think there are still 2 occurances of "coded" in there that would be better to be worded differently

@dericed
Copy link
Contributor

dericed commented Jul 9, 2021

Any proposal for a different term? The next step is for the 3 authors to okay rfc9043.xml after the captions are added, so it's really the last chance.

@michaelni
Copy link
Member

I hope i can stiill think clearly after all that terminology lawyering that cant be healthy. but i would say:
Definition of the state transition table for a get_rac readout value of 0/1
I also notice that one_state is connexted to 0 and zero to 1, maybe thats correct but it just caught my eye

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.

4 participants