Le 03/05/2015 04:57, Michael Niedermayer a écrit :
i dont think its a good idea to replace a named variable in a
for () statement by a construct so long that it needs a linebreak
in the text output [...]
I hesitated and this is a very good point, so I replaced by 2 named
variables:
- plane_count which is the count of planes
- quant_table_index_count which is the count of quant_table indexes
Note that plane_count in the ffv1 source code is actually the count of
quant_table indexes, and is _not_ the actual count of planes, I think it
is dangerous to have this name for the count of quant_table indexes and
I'll propose a ffv1 source code patch for renaming it.
also a more critical problem is that this patch makes the spec
less well defined.
It does not.
As written in the patch description:
"the order of planes is already defined in the General section and has
no impact on the bitstream."
It is referencing the following sentences in the General section:
"In the case of the normal YCbCr colorspace the Y plane is coded first
followed by the Cb and Cr planes, if an Alpha/transparency plane exists,
it is coded last. In the case of the JPEG2000-RCT colorspace the lines
are interleaved to improve caching efficiency since it is most likely
that the RCT will immediately be converted to RGB during decoding; the
interleaved coding order is also Y,Cb,Cr,Alpha."
It is not good to have redundancy in a specification.
Additionally, this is a limitation in the bitstream syntax which should
not be present from my point of view, because the bitstream does not
care about the type of the plane: Y, color or alpha, the bitstream
syntax is same. Plane() and Line() are generic. so the description of
planes should not be in the bitstream syntax. I don't see how I would
define LumaPlane, CbPlane, CrPlane, AlphaPlane without repeating 99% of
the description in each function. And it would limit future extension:
it would be so easy to add any other color space e.g. XYZ (used by
Cinema, see DCP packages), RGB, sRGB, CMYK, HSV or HSL, why not creating
a generic bitstream syntax instead of forcing YUV? Actually, it is not
the future (see below).
Theres no ambiguity in what a Luma, Cb, Cr, Alpha something is
but a plane 0 plane 1 plane 2, line 0 line 1 line 2 line 3
which is which ?
Actually, if I well understood, there is already an issue with the way
you describe the bitstream.
In the case of version = 4, colorspace = 1 and slice_coding_mode = 1,
you describe lines this way:
LumaLine[y]
CbLine[y]
CrLine[y]
but from the source code, I understand that:
BLine[y]
GLine[y]
RLine[y]
if we keep xxLine, code should be transformed from:
LumaLine[y]
CbLine[y]
CrLine[y]
to
if (slice_coding_mode == 0)
{
LumaLine[y]
CbLine[y]
CrLine[y]
}
if (slice_coding_mode == 1)
{
BLine[y]
GLine[y]
RLine[y]
}
so from my point of view your description (in the General section and in
the bitstream syntax) is already wrong (as well is the definition of
chroma_planes which should be transformed to "must be 1" in the case of
colorspace = 1). So this patch fixes one issue (no LumaLine / CbLine /
CrLine in the case of version = 4, colorspace = 1 and slice_coding_mode
= 1), and I plan to fix the other issues (wrong description of planes
and wrong description of chroma_planes in the General section and in in
the case of version = 4, colorspace = 1 and slice_coding_mode = 1) in a
future patch.
This is leading to a proposal of bitstream change I would like to
propose for version 4, but it is maybe better to discuss of this
proposal in its own thread (and it is not the purpose of this patch).
>From ba2af678d50c2c3fa089eb9cee5b73da92bc2a3e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Martinez?= <jer...@mediaarea.net>
Date: Sun, 3 May 2015 11:16:30 +0200
Subject: [PATCH] Reduce redundancy in the description of xxPlane() and
xxLine().
LumaPlane(), CbPlane(), CrPlane() and AlphaPlane() are actually same; the order
of planes is already defined in the General section and has no impact on the
bitstream. Reduced to one Plane( p ) call.
LumaLine(), CbLine(), CrLine() and AlphaLine() are actually same; the order of
lines is already defined in the General section and has no impact on the
bitstream. Reduced to one Line( p, y ) call.
plane_count name may be misleading (it is the count of quant_table_index, which
is not always the count of planes) and does not exist in the bistream, replaced
by the sum of existing bitstream elements.
colorspace_type related "if" sorted in ascending order.
---
ffv1.lyx | 801 +++++----------------------------------------------------------
1 file changed, 53 insertions(+), 748 deletions(-)
diff --git a/ffv1.lyx b/ffv1.lyx
index dd5eb50..07e9348 100644
--- a/ffv1.lyx
+++ b/ffv1.lyx
@@ -2897,7 +2897,7 @@ Slice
\begin_layout Standard
\begin_inset Tabular
-<lyxtabular version="3" rows="27" columns="2">
+<lyxtabular version="3" rows="18" columns="2">
<features rotate="0" tabularvalignment="middle">
<column alignment="left" valignment="top">
<column alignment="center" valignment="top">
@@ -3008,537 +3008,6 @@ SliceHeader( i )
</cell>
</row>
<row>
-<cell alignment="left" valignment="top" topline="true" leftline="true"
usebox="none">
-\begin_inset Text
-
-\begin_layout Plain Layout
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-if( colorspace_type == 1) {
-\end_layout
-
-\end_inset
-</cell>
-<cell alignment="center" valignment="top" topline="true" leftline="true"
rightline="true" usebox="none">
-\begin_inset Text
-
-\begin_layout Plain Layout
-
-\end_layout
-
-\end_inset
-</cell>
-</row>
-<row>
-<cell alignment="left" valignment="top" topline="true" leftline="true"
usebox="none">
-\begin_inset Text
-
-\begin_layout Plain Layout
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-for( y = 0;
-\begin_inset space ~
-\end_inset
-
-y < height;
-\begin_inset space ~
-\end_inset
-
-y++ ) {
-\end_layout
-
-\end_inset
-</cell>
-<cell alignment="center" valignment="top" topline="true" leftline="true"
rightline="true" usebox="none">
-\begin_inset Text
-
-\begin_layout Plain Layout
-
-\end_layout
-
-\end_inset
-</cell>
-</row>
-<row>
-<cell alignment="center" valignment="top" topline="true" leftline="true"
usebox="none">
-\begin_inset Text
-
-\begin_layout Plain Layout
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-LumaLine( y )
-\end_layout
-
-\end_inset
-</cell>
-<cell alignment="center" valignment="top" topline="true" leftline="true"
rightline="true" usebox="none">
-\begin_inset Text
-
-\begin_layout Plain Layout
-
-\end_layout
-
-\end_inset
-</cell>
-</row>
-<row>
-<cell alignment="center" valignment="top" topline="true" leftline="true"
usebox="none">
-\begin_inset Text
-
-\begin_layout Plain Layout
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-CbLine( y )
-\end_layout
-
-\end_inset
-</cell>
-<cell alignment="center" valignment="top" topline="true" leftline="true"
rightline="true" usebox="none">
-\begin_inset Text
-
-\begin_layout Plain Layout
-
-\end_layout
-
-\end_inset
-</cell>
-</row>
-<row>
-<cell alignment="center" valignment="top" topline="true" leftline="true"
usebox="none">
-\begin_inset Text
-
-\begin_layout Plain Layout
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-CrLine( y )
-\end_layout
-
-\end_inset
-</cell>
-<cell alignment="center" valignment="top" topline="true" leftline="true"
rightline="true" usebox="none">
-\begin_inset Text
-
-\begin_layout Plain Layout
-
-\end_layout
-
-\end_inset
-</cell>
-</row>
-<row>
-<cell alignment="center" valignment="top" topline="true" leftline="true"
usebox="none">
-\begin_inset Text
-
-\begin_layout Plain Layout
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-if( alpha_plane )
-\end_layout
-
-\end_inset
-</cell>
-<cell alignment="center" valignment="top" topline="true" leftline="true"
rightline="true" usebox="none">
-\begin_inset Text
-
-\begin_layout Plain Layout
-
-\end_layout
-
-\end_inset
-</cell>
-</row>
-<row>
-<cell alignment="center" valignment="top" topline="true" leftline="true"
usebox="none">
-\begin_inset Text
-
-\begin_layout Plain Layout
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-AlphaLine( y )
-\end_layout
-
-\end_inset
-</cell>
-<cell alignment="center" valignment="top" topline="true" leftline="true"
rightline="true" usebox="none">
-\begin_inset Text
-
-\begin_layout Plain Layout
-
-\end_layout
-
-\end_inset
-</cell>
-</row>
-<row>
-<cell alignment="center" valignment="top" topline="true" leftline="true"
usebox="none">
-\begin_inset Text
-
-\begin_layout Plain Layout
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-}
-\end_layout
-
-\end_inset
-</cell>
-<cell alignment="center" valignment="top" topline="true" leftline="true"
rightline="true" usebox="none">
-\begin_inset Text
-
-\begin_layout Plain Layout
-
-\end_layout
-
-\end_inset
-</cell>
-</row>
-<row>
-<cell alignment="center" valignment="top" topline="true" leftline="true"
usebox="none">
-\begin_inset Text
-
-\begin_layout Plain Layout
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-} else {
-\end_layout
-
-\end_inset
-</cell>
-<cell alignment="center" valignment="top" topline="true" leftline="true"
rightline="true" usebox="none">
-\begin_inset Text
-
-\begin_layout Plain Layout
-
-\end_layout
-
-\end_inset
-</cell>
-</row>
-<row>
<cell alignment="center" valignment="top" topline="true" leftline="true"
usebox="none">
\begin_inset Text
@@ -3558,23 +3027,7 @@ AlphaLine( y )
\begin_inset space ~
\end_inset
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-LumaPlane( )
+if( colorspace_type == 0) {
\end_layout
\end_inset
@@ -3625,7 +3078,7 @@ LumaPlane( )
\begin_inset space ~
\end_inset
-if( chroma_planes ) {
+for( p = 0; p < plane_count; p++ ) {
\end_layout
\end_inset
@@ -3692,7 +3145,7 @@ if( chroma_planes ) {
\begin_inset space ~
\end_inset
-CbPlane( )
+Plane( p )
\end_layout
\end_inset
@@ -3708,7 +3161,7 @@ CbPlane( )
</cell>
</row>
<row>
-<cell alignment="center" valignment="top" topline="true" leftline="true"
usebox="none">
+<cell alignment="left" valignment="top" topline="true" leftline="true"
usebox="none">
\begin_inset Text
\begin_layout Plain Layout
@@ -3727,39 +3180,7 @@ CbPlane( )
\begin_inset space ~
\end_inset
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-
-\begin_inset space ~
-\end_inset
-
-CrPlane( )
+} else if( colorspace_type == 1) {
\end_layout
\end_inset
@@ -3775,7 +3196,7 @@ CrPlane( )
</cell>
</row>
<row>
-<cell alignment="center" valignment="top" topline="true" leftline="true"
usebox="none">
+<cell alignment="left" valignment="top" topline="true" leftline="true"
usebox="none">
\begin_inset Text
\begin_layout Plain Layout
@@ -3810,7 +3231,7 @@ CrPlane( )
\begin_inset space ~
\end_inset
-}
+for( y = 0; y < height; y++ )
\end_layout
\end_inset
@@ -3861,7 +3282,23 @@ CrPlane( )
\begin_inset space ~
\end_inset
-if( alpha_plane )
+
+\begin_inset space ~
+\end_inset
+
+
+\begin_inset space ~
+\end_inset
+
+
+\begin_inset space ~
+\end_inset
+
+
+\begin_inset space ~
+\end_inset
+
+for( p = 0; p < plane_count; p++ ) {
\end_layout
\end_inset
@@ -3928,7 +3365,23 @@ if( alpha_plane )
\begin_inset space ~
\end_inset
-AlphaPlane( )
+
+\begin_inset space ~
+\end_inset
+
+
+\begin_inset space ~
+\end_inset
+
+
+\begin_inset space ~
+\end_inset
+
+
+\begin_inset space ~
+\end_inset
+
+Line( p, y )
\end_layout
\end_inset
@@ -4264,6 +3717,11 @@ u(32)
\end_layout
\begin_layout Description
+plane_count is defined as 1 + ( chroma_planes ? 2 : 0 ) + ( alpha_plane
+ ? 1 : 0 ).
+\end_layout
+
+\begin_layout Description
slice_size indicates the size of the slice in bytes.
\begin_inset Newline newline
\end_inset
@@ -4391,164 +3849,6 @@ reserved for future use
\end_layout
-\begin_layout Description
-plane_count indicates the count of planes and the associated plane types.
-\begin_inset Newline newline
-\end_inset
-
-
-\begin_inset Tabular
-<lyxtabular version="3" rows="7" columns="2">
-<features rotate="0" tabularvalignment="middle">
-<column alignment="center" valignment="top">
-<column alignment="center" valignment="top">
-<row>
-<cell alignment="center" valignment="top" topline="true" bottomline="true"
leftline="true" usebox="none">
-\begin_inset Text
-
-\begin_layout Plain Layout
-value
-\end_layout
-
-\end_inset
-</cell>
-<cell alignment="center" valignment="top" topline="true" bottomline="true"
leftline="true" rightline="true" usebox="none">
-\begin_inset Text
-
-\begin_layout Plain Layout
-plane types
-\end_layout
-
-\end_inset
-</cell>
-</row>
-<row>
-<cell alignment="center" valignment="top" topline="true" leftline="true"
usebox="none">
-\begin_inset Text
-
-\begin_layout Plain Layout
-0
-\end_layout
-
-\end_inset
-</cell>
-<cell alignment="center" valignment="top" topline="true" leftline="true"
rightline="true" usebox="none">
-\begin_inset Text
-
-\begin_layout Plain Layout
-forbidden
-\end_layout
-
-\end_inset
-</cell>
-</row>
-<row>
-<cell alignment="center" valignment="top" topline="true" leftline="true"
usebox="none">
-\begin_inset Text
-
-\begin_layout Plain Layout
-1
-\end_layout
-
-\end_inset
-</cell>
-<cell alignment="center" valignment="top" topline="true" leftline="true"
rightline="true" usebox="none">
-\begin_inset Text
-
-\begin_layout Plain Layout
-if version <4: forbidden; else gray
-\end_layout
-
-\end_inset
-</cell>
-</row>
-<row>
-<cell alignment="center" valignment="top" topline="true" leftline="true"
usebox="none">
-\begin_inset Text
-
-\begin_layout Plain Layout
-2
-\end_layout
-
-\end_inset
-</cell>
-<cell alignment="center" valignment="top" topline="true" leftline="true"
rightline="true" usebox="none">
-\begin_inset Text
-
-\begin_layout Plain Layout
-if version <4: forbidden; else gray+alpha
-\end_layout
-
-\end_inset
-</cell>
-</row>
-<row>
-<cell alignment="center" valignment="top" topline="true" leftline="true"
usebox="none">
-\begin_inset Text
-
-\begin_layout Plain Layout
-3
-\end_layout
-
-\end_inset
-</cell>
-<cell alignment="center" valignment="top" topline="true" leftline="true"
rightline="true" usebox="none">
-\begin_inset Text
-
-\begin_layout Plain Layout
-luma+chroma
-\end_layout
-
-\end_inset
-</cell>
-</row>
-<row>
-<cell alignment="center" valignment="top" topline="true" leftline="true"
usebox="none">
-\begin_inset Text
-
-\begin_layout Plain Layout
-4
-\end_layout
-
-\end_inset
-</cell>
-<cell alignment="center" valignment="top" topline="true" leftline="true"
rightline="true" usebox="none">
-\begin_inset Text
-
-\begin_layout Plain Layout
-luma+chroma+alpha
-\end_layout
-
-\end_inset
-</cell>
-</row>
-<row>
-<cell alignment="center" valignment="top" topline="true" bottomline="true"
leftline="true" usebox="none">
-\begin_inset Text
-
-\begin_layout Plain Layout
-Other
-\end_layout
-
-\end_inset
-</cell>
-<cell alignment="center" valignment="top" topline="true" bottomline="true"
leftline="true" rightline="true" usebox="none">
-\begin_inset Text
-
-\begin_layout Plain Layout
-reserved for future use
-\end_layout
-
-\end_inset
-</cell>
-</row>
-</lyxtabular>
-
-\end_inset
-
-
-\end_layout
-
\begin_layout Subsection
Slice Header
\end_layout
@@ -4739,7 +4039,7 @@ ur
\begin_inset space ~
\end_inset
-for( j = 0; j < plane_count; j++)
+for( j = 0; j < quant_table_index_count; j++)
\end_layout
\end_inset
@@ -5107,6 +4407,11 @@ Inferred to be 1 if not present.
\end_layout
\begin_layout Description
+quant_table_index_count is defined as 1 + ( ( chroma_planes || version <
+ 4 ) ? 1 : 0 ) + ( alpha_plane ? 1 : 0 ).
+\end_layout
+
+\begin_layout Description
quant_table_index indicates the index to select the quantization table set
and the initial states for the slice.
\begin_inset Newline newline
--
1.9.5.msysgit.1
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
