Subject: [Cellar] [PATCH FFV1] Byte alignment is made explicit

From: Jerome Martinez <[email-masked]>
Date: Tue, 24 May 2016 22:58:28 +0200

Currently, if we follow the specification, slice_size error_status, 
slice_crc_parity are not at the right place compared to what is written 
by the FFmpeg encoder because the specified bitstream is not aligned 
after the parsing of the last Line() in the case the count of bits 
written in the bitstream is not a multiple of 8, for Golomb Rice coding.
FFmpeg encoder just "closes" the bitstream with zero padding for the 
last byte of the bitstream then writes bytes. As the bitstream 
description does not do any difference between bits and bytes, this 
padding is not caught by the specification.

If we don't add such information in the specification, a decoder relying 
on the specification will not be able to read correctly slice_size 
error_status, slice_crc_parity in case it reads the bitstream in 
sequential order (e.g. no seek support and/or no complete frame 
available and/or corrupted slice_size).

This byte alignment procedure is not used for the Range Coder because 
the Range Coder uses bytes. Explicit definition about how to switch from 
Range Coder to Golomb Rice and vice versa will be added in a later patch.

From fbed46ad3890012c0a603d74f1fbb3dda23c245b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Martinez?= <[email-masked]>
Date: Tue, 24 May 2016 21:33:18 +0200
Subject: [PATCH] Byte alignment is made explicit

With Golomb Rice coding mode, bits are consumed one per one
but the last bits in the bitstream (slice_size, error_status,
slice_crc_parity) are byte aligned, so there is some padding
in the bitstream before slice_size.
---
 ffv1.md | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/ffv1.md b/ffv1.md
index cf26a10..2d5b339 100644
--- a/ffv1.md
+++ b/ffv1.md
@@ -153,6 +153,8 @@ __a...b__ means any value starting from a to b, inclusive.
 
 **remaining\_bits\_in\_bitstream( )** means the count of remaining bits after the current position in the bitstream. It is computed from the NumBytes value multiplied by 8 minus the count of bits already read by the bitstream parser.
 
+**byte\_aligned( )** means remaining\_bits\_in\_bitstream( ) % 8 is 0.
+
 \pagebreak
 
 # General Description
@@ -541,6 +543,9 @@ See [NUT](#references) for more information about elements.
 |            for( p = 0; p \< primary\_color\_count; p++ ) { |       |
 |                Line( p, y )                                |       |
 |    }                                                       |       |
+|    if ( coder\_type )                                      |       |
+|        while ( !byte\_aligned() )                          |       |
+|            padding                                         |  u(1) |
 |    if( i \|\| version \> 2 )                               |       |
 |        slice\_size                                         | u(24) |
 |    if( ec ) {                                              |       |
@@ -551,6 +556,9 @@ See [NUT](#references) for more information about elements.
 
 **primary\_color\_count** is defined as 1 + ( chroma_planes ? 2 : 0 ) + ( alpha_plane ? 1 : 0 ).
 
+**padding** specifies a bit without any significance and used only for byte alignment.
+MUST be 0.
+
 **slice_size** indicates the size of the slice in bytes.
 Note: this allows finding the start of slices before previous slices have been fully decoded. And allows this way parallel decoding as well as error resilience.
 
-- 
2.7.0.windows.1

---

From: Michael Niedermayer <[email-masked]>
Date: Wed, 25 May 2016 01:32:12 +0200

On Tue, May 24, 2016 at 10:58:28PM +0200, Jerome Martinez wrote:
> slice_crc_parity are not at the right place compared to what is
> written by the FFmpeg encoder because the specified bitstream is not
> aligned after the parsing of the last Line() in the case the count
> of bits written in the bitstream is not a multiple of 8, for Golomb
> Rice coding.
> If we don't add such information in the specification, a decoder
> relying on the specification will not be able to read correctly
> slice_size error_status, slice_crc_parity in case it reads the
> bitstream in sequential order (e.g. no seek support and/or no
> complete frame available and/or corrupted slice_size).
> 
> This byte alignment procedure is not used for the Range Coder
> because the Range Coder uses bytes. Explicit definition about how to
> switch from Range Coder to Golomb Rice and vice versa will be added
> in a later patch.
> 

> d7edd8f81e9f520bd632a3bc4933bbe2fd6e3e59  0001-Byte-alignment-is-made-explicit.patch

> +|    if ( coder\_type )                                      |       |

coder_type == 0

> +**padding** specifies a bit without any significance and used only for byte alignment.

ok, i suggest though to change this for the next version of the
bitstream
its better to write a 1 bit and then pad with 0 to a byte boundary
or (0 bit + padding with 1) that way the end of the previous bitstream
can be determind with certanity from the last byte

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I do not agree with what you have to say, but I'll defend to the death your
right to say it. -- Voltaire

---

From: Jerome Martinez <[email-masked]>
Date: Wed, 25 May 2016 08:41:21 +0200

>> +|    if ( coder\_type )                                      |       |

Oops, very stupid typo.
Fixed.

> can be determind with certanity from the last byte

I keep it in mind for when I open discussions about v4+ improvements.

Jérôme
>From ec4b9647c45834102e278d5f573e9180d13b6fde Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Martinez?= <[email-masked]>
Date: Wed, 25 May 2016 08:28:37 +0200
Subject: [PATCH] [PATCH] Byte alignment is made explicit

diff --git a/ffv1.md b/ffv1.md
index cf26a10..f2e1a7c 100644
--- a/ffv1.md
+++ b/ffv1.md
@@ -153,6 +153,8 @@ __a...b__ means any value starting from a to b, inclusive.

 
 # General Description
+|    if ( coder\_type == 0 )                                 |       |

 
-- 
2.7.0.windows.1

---

From: Michael Niedermayer <[email-masked]>
Date: Wed, 25 May 2016 16:02:26 +0200

> fe47d32273bedfbe86a2d564af484e4687c311b7  0001-PATCH-Byte-alignment-is-made-explicit.patch

applied

thanks

[...]

In fact, the RIAA has been known to suggest that students drop out
of college or go to community college in order to be able to afford
settlements. -- The RIAA