Make Quantization descriptions more coherent #68

JeromeMartinez · 2017-06-03T15:16:33Z

I understand that there is some incoherencies in the description of Quantization related items, i.e. "context" is used with 2 different meanings, the are in some places "quantization table set" never defined and in other places "quantization table" which is similar the "quantization table set".

I tried to have something more coherent, with 3 items:

"quantization tables", having quant_table_count "quantization table"
"quantization table", having 5 "blocks"
"block", having 256 entries

At the beginning, I tried to do with:

"quantization tables sets", having quant_table_set_count "quantization table set"
"quantization table set", having 5 "quantization table"
"quantization table", having 256 entries
but it was more changes in the document, and I preferred to avoid so many changes, is it OK?

Other changes:
a − b replaced by k, as a − b detail is not needed at this place (and k is chosen because it is already used in QuantizationTable() definition)
set removed when present, in order to have a single item for the same thing
Table_ is removed because it is never defined and same as quant_tables[i]
Quantization table index is described (with more background about the exception)
QuantizationTablePerContext changed to QuantizationTablePerBlock because Context is also used (see Context paragraph)
Added missing space in the list of negative numbers

dericed · 2017-06-07T00:31:20Z

ffv1.md


-There are 5 quantization tables for the 5 sample differences, both the number of quantization steps and their distribution are stored in the bitstream. Each quantization table has exactly 256 entries, and the 8 least significant bits of the sample difference are used as index:
+Each quantization table contains 5 blocks, each block corresponding to 1 of the 5 sample differences, both the number of quantization steps and their distribution are stored in the bitstream. Each quantization table block has exactly 256 entries, and the 8 least significant bits of the sample difference are used as index:


Perhaps clearer language. For instance a table of 10 blocks does contain 5 blocks. Could you say contains exactly 5 blocks?

Perhaps a titlecase, backtick-quoted term Quantized Sample Differences should be used both in this section and in the prior Context section to clarify that these are the same thing.

Similar to prior comment the concept of blocks and quantization table block above could be a more formal term such as:

Each Quantization Table contains 5 Quantization Table Blocks with each block corresponding to 1 of the 5 Quantized Sample Differences. Both the number of quantization steps and their distribution are stored in the bitstream. Each Quantization Table Block has exactly 256 entries, and the 8 least significant bits of the Quantized Sample Difference used as index:

Regarding each block corresponding to 1 of the 5 sample differences, is an order implied?

Regarding are stored in the bitstream, could this be more specific as to where this is stored?

Except each block corresponding to 1 of the 5 sample differences comments (IMO order is explicated with Q_1, Q_2... And will be more explicit with the next PR changing them in an array), all comments are included in a the new version.

dericed · 2017-06-07T00:40:28Z

ffv1.md

 RFC:```

+In this formula, `i` is the quantization table index of the currently parsed data, `j` is the sample difference index, `k` the sample difference.


Do I understand correctly, that the = would be read as is assigned?

Right (see "Assignment operators" part)

dericed · 2017-06-07T00:41:46Z

ffv1.md

+
+## Quantization table indexes
+
+Several quantization tables can be used in the same bitstream.


Is this sentence needed? It just seems to say that the bitstream (ffv1 bitstream, I suppose) can contain more than 1 quantization table.

dericed · 2017-06-07T00:42:58Z

ffv1.md

@@ -797,7 +810,7 @@ Inferred to be 1 if not present.

 ### quant_table_index

-`quant_table_index` indicates the index to select the quantization table set and the initial states for the slice.
+`quant_table_index` indicates the index to select the quantization table and the initial states for the slice.


Since Quantization table indexes is designed earlier, I think the term should be used in the definition here.

JeromeMartinez · 2017-06-07T13:05:33Z

I added another commit so changes compared to remarks are visible, please review then I can squash them.

dericed · 2017-06-07T23:21:50Z

ffv1.md


-There are 5 quantization tables for the 5 sample differences, both the number of quantization steps and their distribution are stored in the bitstream. Each quantization table has exactly 256 entries, and the 8 least significant bits of the sample difference are used as index:
+Each Quantization Table contains 5 exactly Quantization Table Blocks, each block corresponding to 1 of the 5 Quantized Sample Differences, both the number of quantization steps and their distribution [are stored in the bitstream](#quantization-table). Each Quantization Table Block has exactly 256 entries, and the 8 least significant bits of the Quantized Sample Differences are used as index:


I think you mean exactly 5 rather than 5 exactly.

I don't understand the intent of and their distribution [are stored in the bitstream](#quantization-table). The bracketed label does not seem to match the section link.

dericed · 2017-06-07T23:24:26Z

ffv1.md

+
+## Quantization Table indexes
+
+For each plane of each slice, a Quantization Table is selected from an index:


You may need an extra line break after line 249 for the bulleted list to render correctly in the RFC outputs.

dericed · 2017-06-07T23:28:48Z

ffv1.md

@@ -160,7 +160,7 @@ NumBytes is a non-negative integer that expresses the size in 8-bit octets of pa

 # General Description

-Samples within a plane are coded in raster scan order (left->right, top->bottom). Each sample is predicted by the median predictor from samples in the same plane and the difference is stored see [Coding of the Sample Difference](#coding-of-the-sample-difference).
+Samples within a plane are coded in raster scan order (left->right, top->bottom). Each sample is predicted by the median predictor from samples in the same plane and the difference is stored see [Coding of the Quantized Sample Differences](#coding-of-the-quantized-sample-difference).


There's a mismatch in the section link.

coding-of-the-quantized-sample-difference vs Coding of the Quantized Sample Differences

I suggest adding an s to the end of the link.

JeromeMartinez · 2017-06-08T07:13:35Z

@dericed I added another update, please review.

I did not change something for:

I don't understand the intent of and their distribution are stored in the bitstream. The bracketed label does not seem to match the section link.

as I don't see what you expect: you wanted to know where the quantization tables are stored, are linked to the corresponding place ("Quantization table" section in the bitstream part of the specifications).
Please suggest another method if this one does not fit.

JeromeMartinez · 2017-06-08T15:10:08Z

Commits are combined now, please review again and confirm all is fine.

dericed

To the extent that I understand the content, LGTM.

michaelni · 2017-06-09T23:50:56Z

ffv1.md

@@ -160,7 +160,7 @@ NumBytes is a non-negative integer that expresses the size in 8-bit octets of pa

 # General Description

-Samples within a plane are coded in raster scan order (left->right, top->bottom). Each sample is predicted by the median predictor from samples in the same plane and the difference is stored see [Coding of the Sample Difference](#coding-of-the-sample-difference).
+Samples within a plane are coded in raster scan order (left->right, top->bottom). Each sample is predicted by the median predictor from samples in the same plane and the difference is stored see [Coding of the Quantized Sample Differences](#coding-of-the-quantized-sample-differences).


The sample difference is coded not the quantized sample difference.

michaelni · 2017-06-09T23:56:27Z

ffv1.md


-There are 5 quantization tables for the 5 sample differences, both the number of quantization steps and their distribution are stored in the bitstream. Each quantization table has exactly 256 entries, and the 8 least significant bits of the sample difference are used as index:
+Each Quantization Table contains exactly 5 Quantization Table Blocks, each block corresponding to 1 of the 5 Quantized Sample Differences, both the number of quantization steps and their distribution are stored in the bitstream. Each Quantization Table Block has exactly 256 entries, and the 8 least significant bits of the Quantized Sample Differences are used as index:


A Table in the most general sense maps an input parameter to an output parameter.
Before this change each of the 5 tables mapped a bounded scalar integer to a scalar integer of smaller range. Aka performing scalar quantization.
What does the single table do afterwards ? Its input has to be a 5 dimensional vector to fill the same role. While this works it is not how it can be implemented. I think the text was clearer before

I think I also prefer the original use of the term 'table' as a map rather than introduce the new concept of 'table block' which is a term I have less understanding of. Relatedly I see ISO-14495 uses the term 'mapping table' and 'mapping table entries'. Is it worthwhile sharing this terminology?

Current spec uses at one place "quantization table set", please comment about using this terminology everywhere (it was a request in my PR introduction).

I am okay with "quantization table set" but it should be defined and probably reused in the line 238 we're discussion above.

@michaelni and @dericed are you both OK for using the alternative form proposed in the intro of my PR ("quantization tables sets", "quantization table set", "quantization table"), meaning that there will be lot of changes a bit everywhere in the spec ("quantization table" must be change nearly everywhere by "quantization table set").
I don't want to spend time on that if the idea is not agreed by everyone first.

I see above you had:

At the beginning, I tried to do with:
"quantization tables sets", having quant_table_set_count "quantization table set"
"quantization table set", having 5 "quantization table"
"quantization table", having 256 entries
but it was more changes in the document, and I preferred to avoid so many changes, is it > OK?

I don't have a strong opinion, but to me 'quantization table set' and 'quantization table' make sense and I suggest in addition to putting these in backtick quotes and title case that they be defined as local terminology as well.

To me 'quantization tables sets' is less elegant, particularly since it contains data that is not only 'quantization tables set'.

I suggest the 'entries' mentioned above be a defined term as well.

ping, I wish to know in which direction you want me to go in order to get the PR accepted.

I don't have a strong opinion, but to me 'quantization table set' and 'quantization table' make sense [..]
To me 'quantization tables sets' is less elegant, particularly since it contains data that is not only 'quantization tables set'.

+1

dericed · 2017-06-30T15:48:33Z

Cut-off date for draft updates for IETF99 is July 3rd. Any chance we can resolve the issues of this pull request before then.

JeromeMartinez · 2017-06-30T15:50:06Z

Any chance we can resolve the issues of this pull request before then.

I plan to work again on it tomorrow and send a new proposal, then it will depends on comments, I guess.

JeromeMartinez · 2017-07-01T08:51:59Z

@dericed and @michaelni:
I switched to the second possibility for renaming.
I also reverted the change about sample difference.

Please review again.

dericed · 2017-07-01T15:02:11Z

ffv1.md

@@ -1227,11 +1242,11 @@ QuantizationTablePerContext(i, j, scale) {                    |

 ### quant_tables

-`quant_tables` indicates the quantification table values.
+`quant_tables[ i ][ j ][ k ]` indicates the quantification table value of the Quantized Sample Difference `k` of the Quantization Table `j` of the Set Quantization Table `i`.


Shouldn't this end with?

of the Quantization Table Set i.

Shouldn't this end with?

of the Quantization Table Set i.

Fixed.

dericed · 2017-07-01T15:05:45Z

ffv1.md

 RFC:```

+In this formula, `i` is the Quantization Table Set index, `j` is the Quantized Table index, `k` the Quantized Sample Difference.
+
+## Quantization Table Set indexes


Nitpick but I would suggest using the singular index in the section header.

IMO this is a general section about all Quantization Table Set indexes (there are several indexes in each slice, ~1 per plane), I am in favor of keeping plural without being against singular, waiting for another person for suggestion.

dericed · 2017-07-01T15:07:23Z

Two minor comments, but otherwise this looks good. Thanks.

"Quantization Table" becomes "Quantization Table Set" "Quantization Table Per Context" becomes "Quantization Table" And some rewording in order to be more explicit

dericed

LGTM, thanks.

michaelni · 2017-07-02T13:52:29Z

This requires to be re checked, there are issues in it, for example quant_tables is interpreted as quant subscript t. There is also trailing whitespace, The "the Template for Context and Median Prediction" link also doesnt work, thats seems to be an older issue though

michaelni · 2017-07-02T13:53:30Z

The "Restrictions" section seems to have "Quantization Table Set" as parent in the pdf at least

JeromeMartinez · 2017-07-02T13:56:13Z

There is also trailing whitespace

The trailing whitespaces I added are for carriage return when converted to HTML (markdown considers single carriage returns without white space as no carriage return), was from my point of view easier to read. I suggest to keep them, without being against removing them.

I push soon another PR for the other issues.

michaelni · 2017-07-02T14:02:13Z

The trailing whitespaces I added are for carriage return when converted to HTML (markdown considers single carriage returns without white space as no carriage return), was from my point of view easier to read.

I did not realize that, thx for explaining

JeromeMartinez · 2017-07-02T14:22:52Z

@michaelni

quant_tables is interpreted as quant subscript t

I don't understand this sentence.

michaelni · 2017-07-02T18:22:49Z

quant_tables is interpreted as quant subscript t

I don't understand this sentence.

In the pdf output some quant_tables are written with a small t, as if t was a subscript

JeromeMartinez mentioned this pull request Jun 3, 2017

Improvement of context and quantization tables paragraphs #56

Merged

dericed reviewed Jun 7, 2017

View reviewed changes

JeromeMartinez force-pushed the QuantizationTables branch from c124358 to 4e412b7 Compare June 8, 2017 15:09

JeromeMartinez force-pushed the QuantizationTables branch from 4e412b7 to 076993b Compare June 8, 2017 15:23

dericed approved these changes Jun 8, 2017

View reviewed changes

michaelni reviewed Jun 9, 2017

View reviewed changes

dericed mentioned this pull request Jun 30, 2017

update to version 03 #73

Closed

JeromeMartinez force-pushed the QuantizationTables branch 3 times, most recently from 6e6de94 to f89368b Compare July 1, 2017 08:48

dericed reviewed Jul 1, 2017

View reviewed changes

Make Quantization descriptions more coherent

4aaf7c0

"Quantization Table" becomes "Quantization Table Set" "Quantization Table Per Context" becomes "Quantization Table" And some rewording in order to be more explicit

JeromeMartinez force-pushed the QuantizationTables branch from f89368b to 4aaf7c0 Compare July 1, 2017 15:18

dericed approved these changes Jul 1, 2017

View reviewed changes

michaelni merged commit 4aaf7c0 into FFmpeg:master Jul 1, 2017

dericed mentioned this pull request Jul 2, 2017

escape underscore #78

Merged

JeromeMartinez deleted the QuantizationTables branch January 3, 2018 18:38


		There are 5 quantization tables for the 5 sample differences, both the number of quantization steps and their distribution are stored in the bitstream. Each quantization table has exactly 256 entries, and the 8 least significant bits of the sample difference are used as index:
		Each quantization table contains 5 blocks, each block corresponding to 1 of the 5 sample differences, both the number of quantization steps and their distribution are stored in the bitstream. Each quantization table block has exactly 256 entries, and the 8 least significant bits of the sample difference are used as index:

		RFC:```

		In this formula, `i` is the quantization table index of the currently parsed data, `j` is the sample difference index, `k` the sample difference.


		## Quantization table indexes

		Several quantization tables can be used in the same bitstream.


		There are 5 quantization tables for the 5 sample differences, both the number of quantization steps and their distribution are stored in the bitstream. Each quantization table has exactly 256 entries, and the 8 least significant bits of the sample difference are used as index:
		Each Quantization Table contains 5 exactly Quantization Table Blocks, each block corresponding to 1 of the 5 Quantized Sample Differences, both the number of quantization steps and their distribution [are stored in the bitstream](#quantization-table). Each Quantization Table Block has exactly 256 entries, and the 8 least significant bits of the Quantized Sample Differences are used as index:


		## Quantization Table indexes

		For each plane of each slice, a Quantization Table is selected from an index:


		There are 5 quantization tables for the 5 sample differences, both the number of quantization steps and their distribution are stored in the bitstream. Each quantization table has exactly 256 entries, and the 8 least significant bits of the sample difference are used as index:
		Each Quantization Table contains exactly 5 Quantization Table Blocks, each block corresponding to 1 of the 5 Quantized Sample Differences, both the number of quantization steps and their distribution are stored in the bitstream. Each Quantization Table Block has exactly 256 entries, and the 8 least significant bits of the Quantized Sample Differences are used as index:

Make Quantization descriptions more coherent #68

Make Quantization descriptions more coherent #68

Conversation

JeromeMartinez commented Jun 3, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JeromeMartinez commented Jun 7, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dericed Jun 7, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JeromeMartinez commented Jun 8, 2017

JeromeMartinez commented Jun 8, 2017

dericed left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dericed commented Jun 30, 2017

JeromeMartinez commented Jun 30, 2017

JeromeMartinez commented Jul 1, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dericed commented Jul 1, 2017

dericed left a comment

Choose a reason for hiding this comment

michaelni commented Jul 2, 2017

michaelni commented Jul 2, 2017

JeromeMartinez commented Jul 2, 2017

michaelni commented Jul 2, 2017

JeromeMartinez commented Jul 2, 2017

michaelni commented Jul 2, 2017

dericed Jun 7, 2017 •

edited