Subject: [Cellar] updating FFV1 referencing and formatting

From: Dave Rice <[email-masked]>
Date: Sat, 3 Dec 2016 20:28:56 -0500

Hi all,

I submitted a pull request at https://github.com/FFmpeg/FFV1/pull/25 <https://github.com/FFmpeg/FFV1/pull/25> which focuses on adjusting the formatting of ffv1.md for better handling once transformed through rfc2xml. Note that in the first version of the ffv1 draft, many of the code blocks and tables are poorly formed, see https://tools.ietf.org/html/draft-niedermayer-cellar-ffv1-00#section-5.2 <https://tools.ietf.org/html/draft-niedermayer-cellar-ffv1-00#section-5.2> for instance. This PR adjust these sections to present them correctly.

The pull request also updates the citations of the ffv1 draft to use the form suggested by mmark (which prepared markdown for xml2rfc) at https://github.com/miekg/mmark/wiki/Syntax#citations <https://github.com/miekg/mmark/wiki/Syntax#citations>.

A preview of the rendered text of the pull request is available at https://gist.github.com/dericed/2f4bf9c7d3b15aa4ba54f59184aa964f <https://gist.github.com/dericed/2f4bf9c7d3b15aa4ba54f59184aa964f>.

Dave Rice

---

From: Michael Niedermayer <[email-masked]>
Date: Sun, 4 Dec 2016 04:42:59 +0100

> The pull request also updates the citations of the ffv1 draft to use the form suggested by mmark (which prepared markdown for xml2rfc) at https://github.com/miekg/mmark/wiki/Syntax#citations <https…

i havnt yet had time to really review this but i spoted this:
-|                                                                           |      |
-|---------------------------------------------------------------------------|------|
-| QuantizationTablePerContext(i, j, scale) {                                | type |
-|    v = 0                                                                  |      |
-|    for( k = 0; k \< 128; ) {                                              |      |
-|        len - 1                                                            | sr   |
-|        for( a = 0; a \< len; a++ ) {                                      |      |
-|            quant\_tables[ i ][ j ][ k ] = scale* v                        |      |
-|            k++                                                            |      |
-|        }                                                                  |      |
-|        v++                                                                |      |
-|    }                                                                      |      |
-|    for( k = 1; k \< 128; k++ ) {                                          |      |
-|        quant\_tables[ i ][ j ][ 256 - k ] = -quant\_tables[ i ][ j ][ k ] |      |
-|    }                                                                      |      |
-|    quant\_tables[ i ][ j ][ 128 ] = -quant\_tables[ i ][ j ][ 127 ]       |      |
-|    len\_count[ i ][ j ] = v                                               |      |
-|}                                                                          |      |
+```c
+QuantizationTablePerContext(i, j, scale) {                    | type
+    v = 0                                                     |
+    for( k = 0; k \< 128; ) {                                 |
+        len - 1                                               | sr
+        for( a = 0; a \< len; a++ ) {                         |
+            quant_tables[ i ][ j ][ k ] = scale* v            |
+            k++                                               |
+        }                                                     |
+        v++                                                   |
+    }                                                         |
+    for( k = 1; k < 128; k++ ) {
+        quant_tables[ i ][ j ][ 256 - k ] = -quant_tables[ i ][ j ][ k ]
+    }
+    quant_tables[ i ][ j ][ 128 ] = -quant_tables[ i ][ j ][ 127 ]
+    len_count[ i ][ j ] = v
+}
+```

the right column is shortened

also not specific to this patch but the text is formating wise
still terrible

The SliceFooter contains random \ in the pdf rendered output

the type column heading is aligned with the function name
there should be a matching heading term on the left column i think
or it should be otherwise changed
some if() have a space, some do not

The Configuration Record has no table style but rather C comments

3.3 Context is horizontally spreat over the whole page in pdf

the formating for 2.2.3 and 2.2.4 is different in pdf output

...

Ive just today installed pandoc 1.19 on the box where i normally review
patches so i didnt fully realize before its that bad looking, i
generally assumed my old tools where at fault ...

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Good people do not need laws to tell them to act responsibly, while bad
people will find a way around the laws. -- Plato

---

From: Dave Rice <[email-masked]>
Date: Sun, 4 Dec 2016 12:29:38 -0500

Hi,

> On Dec 3, 2016, at 10:42 PM, Michael Niedermayer <[email-masked]> wrote:
> 

I had shortened it to prevent the line of code plus the right column from exceeding the 72 character per line rfc limit.

I adjusted this and pushed an update, so now the two lower lines are wrapped with a backslah, as in:

for( k = 1; k < 128; k++ ) { |
quant_tables[ i ][ j ][ 256 - k ] = \ |
-quant_tables[ i ][ j ][ k ] |
} |
quant_tables[ i ][ j ][ 128 ] = \ |
-quant_tables[ i ][ j ][ 127 ] | |
len_count[ i ][ j ] = v |

> The SliceFooter contains random \ in the pdf rendered output

Fixed and pushed. In tables the \ were needed for some escaping but in code blocks they aren't and I neglected to change it in all cases. Updated now. Please re-review.

> or it should be otherwise changed

such as?

function | type
---------|-----

> some if() have a space, some do not

I added https://github.com/FFmpeg/FFV1/pull/25/commits/c298a2f054f965bdf5b026b98a791ec53a3964cd to standardize ifs into `if (...)` form.

> The Configuration Record has no table style but rather C comments

Updated.

> 3.3 Context is horizontally spreat over the whole page in pdf

I adjusted this to use a grid table in a codeblock, which seems to be a good compromise between the rfc and pandoc outputs.

```
+---+---+---+---+
|   |   | T |   |
+---+---+---+---+
|   |tl | t |tr |
+---+---+---+---+
| L | l | X |   |
+---+---+---+---+
```

> the formating for 2.2.3 and 2.2.4 is different in pdf output

Fixed in https://github.com/FFmpeg/FFV1/pull/25/commits/ab88dd92d2d1ac6a646d40d313a5ebf73dc189dd.

> generally assumed my old tools where at fault ...

I've been using pandoc 1.19 for my review as well, though please pay attention to the html and txt outputs of rfc2xml as well.

[...]

Dave Rice

---

From: Michael Niedermayer <[email-masked]>
Date: Sun, 4 Dec 2016 19:24:16 +0100

On Sun, Dec 04, 2016 at 12:29:38PM -0500, Dave Rice wrote:
[...]
> > or it should be otherwise changed

yes

thx

[...]

When you are offended at any man's fault, turn to yourself and study your
own failings. Then you will forget your anger. -- Epictetus