Subject: [Cellar] I-D Action: draft-ietf-cellar-ffv1-15.txt

From: [email-masked]
Date: Tue, 23 Jun 2020 12:12:49 -0700

A New Internet-Draft is available from the on-line Internet-Drafts directories.
This draft is a work item of the Codec Encoding for LossLess Archiving and Realtime transmission WG of the IETF.

        Title           : FFV1 Video Coding Format Version 0, 1, and 3
        Authors         : Michael Niedermayer
                          Dave Rice
                          Jerome Martinez
	Filename        : draft-ietf-cellar-ffv1-15.txt
	Pages           : 53
	Date            : 2020-06-23

Abstract:
   This document defines FFV1, a lossless intra-frame video encoding
   format.  FFV1 is designed to efficiently compress video data in a
   variety of pixel formats.  Compared to uncompressed video, FFV1
   offers storage compression, frame fixity, and self-description, which
   makes FFV1 useful as a preservation or intermediate video format.

The IETF datatracker status page for this draft is:
https://datatracker.ietf.org/doc/draft-ietf-cellar-ffv1/

There are also htmlized versions available at:
https://tools.ietf.org/html/draft-ietf-cellar-ffv1-15
https://datatracker.ietf.org/doc/html/draft-ietf-cellar-ffv1-15

A diff from the previous version is available at:
https://www.ietf.org/rfcdiff?url2=draft-ietf-cellar-ffv1-15

Please note that it may take a couple of minutes from the time of submission
until the htmlized version and diff are available at tools.ietf.org.

Internet-Drafts are also available by anonymous FTP at:
ftp://ftp.ietf.org/internet-drafts/

---

From: Spencer Dawkins at IETF <[email-masked]>
Date: Tue, 23 Jun 2020 14:25:37 -0500

Hi, Murray,

We just confirmed during the Cellar virtual meeting that -15 IS actually
ready for you to check and confirm that we've addressed the AD comments on
this draft.

Best,

Spencer

On Tue, Jun 23, 2020 at 2:12 PM <[email-masked]> wrote:

> _______________________________________________
> Cellar mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/cellar
>

---

From: "Murray S. Kucherawy" <[email-masked]>
Date: Thu, 25 Jun 2020 09:24:57 -0700

Hi, thanks for this.  I'm looking now only at the diff between -13 (the
last version I looked at) and this one.

First and foremost, this is mostly better.  Kudos for putting the work in.

Hooray, most of those "type" columns are gone!  But, alas, not all of
them.  They're still present in Sections 4.1 and below.  I counted at least
seven of them just in the diff I'm reviewing.  As before, they should also
be removed, or at least explained.

A minor point: Prior to Section 3.8.2.3, it looks like all of the examples
are in C.  After that, they switch to pseudo code.  Any reason we can't be
consistent?

I suggest that the prose below Table 8, which is a mix of pseudocode and
English, would be better broken apart.

Chairs: RFC 4732 is a downref; please make sure the shepherd calls this out
in the writeup.

There are a few code expressions that would benefit from being wrapped
differently.  For example:

(chroma_planes == 1 && (p == 1 || p == 2)) ? ceil(slice_pixel_height
/ (1 << log2_v_chroma_subsample)) : slice_pixel_height

Maybe this?

chroma_planes == 1 && (p == 1 || p == 2)
  ? ceil(slice_pixel_height / (1 << log2_v_chroma_subsample))
  : slice_pixel_height

-MSK

On Tue, Jun 23, 2020 at 12:26 PM Spencer Dawkins at IETF <
[email-masked]> wrote:

>> This draft is a work item of the Codec Encoding for LossLess Archiving
>> and Realtime transmission WG of the IETF.
>>

---

From: Spencer Dawkins at IETF <[email-masked]>
Date: Thu, 25 Jun 2020 11:42:28 -0500

Dear Cellar,

On one point,

On Thu, Jun 25, 2020 at 11:25 AM Murray S. Kucherawy <[email-masked]>
wrote:

> Chairs: RFC 4732 is a downref; please make sure the shepherd calls this
> out in the writeup.
>

This means that RFC 4732 is not a standards-track document, while our draft
is standards-track. That's fine, but we need to point that out when the
document is sent for IETF Last Call, so the shepherd (or one of the working
group chairs) should mention this in the "Other Points" section of
https://datatracker.ietf.org/doc/draft-ietf-cellar-ffv1/shepherdwriteup/.

Best,

Spencer

---

From: Jerome Martinez <[email-masked]>
Date: Thu, 25 Jun 2020 19:19:42 +0200

Hi Murray,

My answers inline:

On 25/06/2020 18:24, Murray S. Kucherawy wrote:
> Hi, thanks for this.  I'm looking now only at the diff between -13 
> (the last version I looked at) and this one.
>
> them.  They're still present in Sections 4.1 and below. I counted at 
> least seven of them just in the diff I'm reviewing.  As before, they 
> should also be removed, or at least explained.
>
> A minor point: Prior to Section 3.8.2.3, it looks like all of the 
> examples are in C.  After that, they switch to pseudo code.  Any 
> reason we can't be consistent?

The figures before section 4 are autonomous functions, used for 
describing a part of the algorithm and not used for directly reading out 
any symbol from the bitstream, so it was making sense to remove the 
"type" column at these places (this column was not used there). They 
could be more pseudocode nonetheless, as the C types ("int" etc) are not 
really relevant (they depends on the programming language).

The figures in section 4 are for explaining the bitstream parsing order, 
and the "type" column is used for indicating the kind of value it is 
(types are described in 
https://tools.ietf.org/id/draft-ietf-cellar-ffv1-15.html#table-4 , I see 
that it is called "Symbol" here, we'll change "Symbol" to "Type" in this 
table).
This kind of method for describing a video bitstream is very classic, 
for example in:
- H.264, see the full spec (PDF to download) 
https://www.itu.int/rec/T-REC-H.264-201906-I/en
- AV1, see an online direct example at 
https://aomediacodec.github.io/av1-spec/#frame-obu-syntax

As in theses specs, we have some figures without usage of the "type" 
column because they are intermediate figures (the "type" column is used 
in the figures using this figure and the figures called by this figure), 
split from another figure, in order to have the spec more readable 
(focusing on a specific topic). AV1 uses it for example at 
https://aomediacodec.github.io/av1-spec/#decode-tile-syntax

>
> I suggest that the prose below Table 8, which is a mix of pseudocode 
> and English, would be better broken apart.

We'll try to find a better wording for this line.

>
> Chairs: RFC 4732 is a downref; please make sure the shepherd calls 
> this out in the writeup.

(Removing my question as Spencer answered my question before I send it :-p )

>   ? ceil(slice_pixel_height / (1 << log2_v_chroma_subsample))

It is on a full line in the specification source code, I see that the 
raw text version and the HTML version have different automatic line 
break position, we'll force some line breaks.

Jérôme

---

From: "Murray S. Kucherawy" <[email-masked]>
Date: Thu, 25 Jun 2020 10:31:46 -0700

On Thu, Jun 25, 2020 at 10:19 AM Jerome Martinez <[email-masked]>
wrote:

> The figures before section 4 are autonomous functions, used for describing
> a part of the algorithm and not used for directly reading out any symbol
> from the bitstream, so it was making sense to remove the "type" column at
> these places (this column was not used there). They could be more
> pseudocode nonetheless, as the C types ("int" etc) are not really relevant
> (they depends on the programming language).
>
> and the "type" column is used for indicating the kind of value it is (types
> are described in
> This kind of method for describing a video bitstream is very classic, for
> example in:
> column because they are intermediate figures (the "type" column is used in
> the figures using this figure and the figures called by this figure), split
> from another figure, in order to have the spec more readable (focusing on a
> specific topic). AV1 uses it for example at

Your previous AD and I were both confused by the format and what it's
trying to convey.  I appreciate that it might be classic in some realms,
but as neither of us had seen it, I imagine others on the IESG who don't
work in our layers (i.e., most of them) will also ask about it.  If you
want to leave them in, I suggest including either some text about how to
interpret them, or a reference to such guidance elsewhere.

-MSK

---

From: Dave Rice <[email-masked]>
Date: Thu, 25 Jun 2020 13:34:41 -0400

Hi Murray,

> On Jun 25, 2020, at 1:19 PM, Jerome Martinez <[email-masked]> wrote:
> 
> Hi Murray,
> 
> My answers inline:
> 
>> Hi, thanks for this.  I'm looking now only at the diff between -13 (the last version I looked at) and this one.
>> 
>> Hooray, most of those "type" columns are gone!  But, alas, not all of them.  They're still present in Sections 4.1 and below.  I counted at least seven of them just in the diff I'm reviewing.  As before, they should also be removed, or at least explained.
>> 
>> A minor point: Prior to Section 3.8.2.3, it looks like all of the examples are in C.  After that, they switch to pseudo code.  Any reason we can't be consistent?
> The figures before section 4 are autonomous functions, used for describing a part of the algorithm and not used for directly reading out any symbol from the bitstream, so it was making sense to remove the "type" column at these places (this column was not used there). They could be more pseudocode nonetheless, as the C types ("int" etc) are not really relevant (they depends on the programming language).
> 
> The figures in section 4 are for explaining the bitstream parsing order, and the "type" column is used for indicating the kind of value it is (types are described in https://tools.ietf.org/id/draft-ietf-cellar-ffv1-15.html#table-4 , I see that it is called "Symbol" here, we'll change "Symbol" to "Type" in this table).
> This kind of method for describing a video bitstream is very classic, for example in:
> - H.264, see the full spec (PDF to download) https://www.itu.int/rec/T-REC-H.264-201906-I/en
> - AV1, see an online direct example at https://aomediacodec.github.io/av1-spec/#frame-obu-syntax
> 
> As in theses specs, we have some figures without usage of the "type" column because they are intermediate figures (the "type" column is used in the figures using this figure and the figures called by this figure), split from another figure, in order to have the spec more readable (focusing on a specific topic). AV1 uses it for example at https://aomediacodec.github.io/av1-spec/#decode-tile-syntax
> 
>> I suggest that the prose below Table 8, which is a mix of pseudocode and English, would be better broken apart.
>> Chairs: RFC 4732 is a downref; please make sure the shepherd calls this out in the writeup.
>> There are a few code expressions that would benefit from being wrapped differently.  For example:
>> 
> It is on a full line in the specification source code, I see that the raw text version and the HTML version have different automatic line break position, we'll force some line breaks.

I add line-breaks to this equation and a related one as you suggested in this commit https://github.com/FFmpeg/FFV1/pull/215/commits/1a4f83fb05591a1cfb4ef197682d06212ecf0b56. I also reviewed other equations and moved many to code blocks and manually set the line-wrapping. This work is to be reviewed in the pull request at https://github.com/FFmpeg/FFV1/pull/215.

Best Regards,
Dave Rice

---

From: Dave Rice <[email-masked]>
Date: Thu, 25 Jun 2020 14:57:17 -0400

> On Jun 25, 2020, at 1:31 PM, Murray S. Kucherawy <[email-masked]> wrote:
> 
> On Thu, Jun 25, 2020 at 10:19 AM Jerome Martinez <[email-masked]> wrote:
> The figures in section 4 are for explaining the bitstream parsing order, and the "type" column is used for indicating the kind of value it is (types are described in https://tools..ietf.org/id/draft-ietf-cellar-ffv1-15.html#table-4 , I see that it is called "Symbol" here, we'll change "Symbol" to "Type" in this table).
> Your previous AD and I were both confused by the format and what it's trying to convey.  I appreciate that it might be classic in some realms, but as neither of us had seen it, I imagine others on the IESG who don't work in our layers (i.e., most of them) will also ask about it.  If you want to leave them in, I suggest including either some text about how to interpret them, or a reference to such guidance elsewhere.

As a draft to try to address this, I added this paragraph and figure to Section 2.2.1 which describes the use of pseudo-code within the document:

>    In some instances, pseudo-code is presented in a two-column format
>    such as shown in Figure 1.  In this form the "type" column provides a
>    symbol as defined in Table 4 that defines the storage of the data
>    referenced in that same line of pseudo-code.
> 
>    pseudo-code                                                   | type
>    --------------------------------------------------------------|-----
>    ExamplePseudoCode( ) {                                        |
>        value                                                     | ur
>    }                                                             |
> 
>        Figure 1: A depiction of type-labelled pseduo-code used within
>                                this document.

This proposed change is at https://github.com/FFmpeg/FFV1/pull/217/files, comments/suggestions welcome.

Note that Section 4.8 of https://aomediacodec.github.io/av1-spec/#method-of-describing-bitstream-syntax uses a similar description for its own pseudo-code.

Dave