Re: [FFmpeg-devel] [PATCH] FFV1 specification: Merge of FrameHeader01() and GlobalHeader()
On Wed, May 20, 2015 at 09:24:37AM +0200, Jerome Martinez wrote: > Le 20/05/2015 03:44, Michael Niedermayer a écrit : > >[...] > > > >If you want to allow multiple w/h/colorspace, its probably better > >to allow multiple global headers as in h264/h265 and put a index > >in the frame header to choose one of the parameter sets > >some of the tables are also large IIRC initial_state_delta is. > >it makes not much sense for this to be anywhere but global > > Proposals for the next version of FFV1 is planned ;-). > But for the moment, I focus on having a specification of the > existing versions. > > >maybe none of these headers should have been called frame header > >i dont know ... > > I thought it would have been better to separates subjects (on one > side the merge, on another side the renaming). I dont agree that a A/B -> X change should be factored into A -> B, B -> X when the interediate state is just wrong one could have factored it like A->X, B->X either way, patch merged thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB it is not once nor twice but times without number that the same ideas make their appearance in the world. -- Aristotle signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] FFV1 specification: Merge of FrameHeader01() and GlobalHeader()
Le 20/05/2015 03:44, Michael Niedermayer a écrit :
[...]
If you want to allow multiple w/h/colorspace, its probably better
to allow multiple global headers as in h264/h265 and put a index
in the frame header to choose one of the parameter sets
some of the tables are also large IIRC initial_state_delta is.
it makes not much sense for this to be anywhere but global
Proposals for the next version of FFV1 is planned ;-).
But for the moment, I focus on having a specification of the existing
versions.
maybe none of these headers should have been called frame header
i dont know ...
I thought it would have been better to separates subjects (on one side
the merge, on another side the renaming).
Anyway, new patch attached:
- FrameHeader() renamed to Parameters()
- additional line in initial_state_delta[ i ][ j ][ k ] description removed.
https://github.com/MediaArea/FFV1/tree/HeaderMerge is also up to date.
LyX Document
>From 5580818c24c5de2a721d4bce505a6cfb9f9833f9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Martinez?=
Date: Wed, 20 May 2015 09:11:27 +0200
Subject: [PATCH] Merge of FrameHeader01() and GlobalHeader()
FrameHeader01() and GlobalHeader() have a lot of common fields
and having a common function + default value for fields unused
in previous versions is less complex and more coherent than repeating
the common part.
They are merged and renamed to Parameters().
---
ffv1.lyx | 528 ---
1 file changed, 201 insertions(+), 327 deletions(-)
diff --git a/ffv1.lyx b/ffv1.lyx
index 01f7308..d50eba7 100644
--- a/ffv1.lyx
+++ b/ffv1.lyx
@@ -1860,7 +1860,7 @@ alternative state transition table
The alternative state transition table has been build using iterative
minimizati
on of frame sizes and generally performs better than the default.
To use it, the coder_type has to be set to 2 and the difference to the
- default has to be stored in the header.
+ default has to be stored in the parameters.
The reference implemenation of FFV1 in FFmpeg uses this table by default
at the time of this writing when Range coding is used.
\end_layout
@@ -2557,6 +2557,10 @@ In the case of a bitstream with version >= 2, a
configuration record is
\begin_inset Newline newline
\end_inset
+It contains the parameters used for all frames.
+\begin_inset Newline newline
+\end_inset
+
The size of the configuration record, NumBytes, is supplied by the underlying
container.
\end_layout
@@ -2642,7 +2646,7 @@ ConfigurationRecordIsPresent = 1
\begin_inset space ~
\end_inset
-GlobalHeader( )
+Parameters( )
\end_layout
\end_inset
@@ -3172,7 +3176,7 @@ if( keyframe && !ConfigurationRecordIsPresent)
\begin_inset space ~
\end_inset
-FrameHeader01( )
+Parameters( )
\end_layout
\end_inset
@@ -5155,16 +5159,12 @@ reserved for future use
\end_layout
\begin_layout Subsection
-Header
-\end_layout
-
-\begin_layout Subsubsection
-Version 0 and 1
+Parameters
\end_layout
\begin_layout Standard
\begin_inset Tabular
-
+
@@ -5173,7 +5173,7 @@ Version 0 and 1
\begin_inset Text
\begin_layout Plain Layout
-FrameHeader01( ) {
+Parameters( ) {
\end_layout
\end_inset
@@ -5224,6 +5224,92 @@ ur
+
+\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( version > 2 )
+\end_layout
+
+\end_inset
+
+
+\begin_inset Text
+
+\begin_layout Plain Layout
+
+\end_layout
+
+\end_inset
+
+
+
+
+\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
+
+micro_version
+\end_layout
+
+\end_inset
+
+
+\begin_inset Text
+
+\begin_layout Plain Layout
+ur
+\end_layout
+
+\end_inset
+
+
+
\begin_inset Text
@@ -5482,7 +5568,7 @@ if( version > 0 )
-
+
\begin_inset Text
\begin_layout Plain Layout
@@ -5692,7 +5778,7 @@ br
\begin_inset space ~
\end_inset
-QuantizationTable( 0 )
+if ( version > 1 ) {
\end_layout
\end_inset
@@ -5708,52 +5794,42 @@ QuantizationTable( 0 )
-
+
\begin_inset Text
\begin_layout Plain Layout
-}
-\end_layout
+\begin_inset space ~
+\end_inset
+
+\begin_inset space ~
\end_inset
-
-
-\begin_inset Text
-\begin_layout Plain Layout
-\end_layout
+\begin_inset space ~
+\end_inset
+
+\begin_inset space ~
\end_inset
-
-
-
+
+\begin_inset space ~
\end_inset
-\end_layout
+\begin_inset space ~
+\end_inset
-\begin_layout Subsubsection
-Version 3
-\end_layout
-\begin_layout Standard
-Version 2 and later files use a global header and a per frame header.
-\end_layout
+\begin_inset space ~
+\end_inset
-\begin_layout Standard
-\begin_inset Tabular
-
-
-
-
-
Re: [FFmpeg-devel] [PATCH] FFV1 specification: Merge of FrameHeader01() and GlobalHeader()
On Tue, May 19, 2015 at 10:24:02PM +0200, Jerome Martinez wrote: > Le 19/05/2015 21:15, Michael Niedermayer a écrit : > >On Mon, May 18, 2015 at 09:04:01PM +0200, Jerome Martinez wrote: > > > >>FrameHeader01() and GlobalHeader() have a lot of common fields > >>and having a common function + default value for fields unused > >>in previous versions is less complex and more coherent than repeating > >>the common part. > >maybe but calling the GlobalHeader FrameHeader is very confusing > >and wrong > > From my point of view, the GlobalHeader() is still a frame header > because it still contains the same pieces of information about > frames (frame width, frame height, frame bit depth...), and it is In h264 its called a sequence parameter set, in MPEG-1/2 a sequence header. Iam not aware of anything calling a global header a "frame header" version information is global its not a frame property also w/h/colorspace/quantization tables and inititial states and all that are global they are not frame properties you could make w/h/colorspace frame properties in principle but If you want to allow multiple w/h/colorspace, its probably better to allow multiple global headers as in h264/h265 and put a index in the frame header to choose one of the parameter sets some of the tables are also large IIRC initial_state_delta is. it makes not much sense for this to be anywhere but global maybe none of these headers should have been called frame header i dont know ... [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB I have often repented speaking, but never of holding my tongue. -- Xenocrates signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] FFV1 specification: Merge of FrameHeader01() and GlobalHeader()
Le 19/05/2015 21:15, Michael Niedermayer a écrit : On Mon, May 18, 2015 at 09:04:01PM +0200, Jerome Martinez wrote: FrameHeader01() and GlobalHeader() have a lot of common fields and having a common function + default value for fields unused in previous versions is less complex and more coherent than repeating the common part. maybe but calling the GlobalHeader FrameHeader is very confusing and wrong From my point of view, the GlobalHeader() is still a frame header because it still contains the same pieces of information about frames (frame width, frame height, frame bit depth...), and it is just not repeated (moved from the beginning of the frame to the configuration record). During the decoding, it is exactly like if the old GlobalHeader() is repeated per keyframe (a global header is equivalent to a frame header repeated per keyframe), and actually the exact same FFV1Context members are used in ffv1dec.c whatever is the version of the FFV1 bitstream. Actually, and still from my point of view, FrameHeader() is very confusing and wrong as much as FrameHeader01() in the original specification of v0/1 (e.g. if we have a stream with one keyframe and all other frames not keyframes, we have only one FrameHeader01() call for all frames, as with GlobalHeader(), so FrameHeader01() is confusing in that case) so I don't add more confusion or more wrongness. I also tried not to change everything, and I just "upgraded" the original FrameHeader01() function without renaming it (I only removed the versioning). I also added the following sentence in the Configuration Record subsection: " LyX Document It (the Configuration Record) contains the frame header used for all frames". Isn't it explicit enough? And I don't find a better wording for something which is per frame (actually not per frame, only per keyframes) in v0/1 and global in v2+. "Header()"? (I am not a big fan of it because it is too much general) "Parameters()"? I argue for not renaming too much functions before reaching a larger audience (IETF and so on) and getting their points of view. [...] @@ -7939,6 +7809,10 @@ pred = j ? initial_states[ i ][j - 1][ k ] : 128 initial_state[ i ][ j ][ k ] = ( pred + initial_state_delta[ i ][ j ][ k ] ) & 255 +\begin_inset Newline newline +\end_inset + +Inferred to be 0 if not present. what is inferred to be 0? initial_state? initial_state_delta? i think this could be misunderstood as the paragraph is talking about multiple variables I planned to reword the description of initial_state_delta (separation between the bitstream syntax which should be in this subsection and formulas which should be in another section dedicated to initial_state description) in another patch, but in the meanwhile this could be misunderstood, true. I propose to change to "Theses formulas are not applied if initial_state_delta[ i ][ j ][ k ] is not present" or "Theses formulas are not applied if LyX Document states_coded is 0" or just remove the line. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] FFV1 specification: Merge of FrameHeader01() and GlobalHeader()
On Mon, May 18, 2015 at 09:04:01PM +0200, Jerome Martinez wrote: > > ffv1.lyx | 530 > --- > 1 file changed, 204 insertions(+), 326 deletions(-) > 38da62e7514e7ece306076f4de29aa1f99279920 > 0001-Merge-of-FrameHeader01-and-GlobalHeader.patch > From c6f16e561d40972e058f4e163ff753bce8fc8acc Mon Sep 17 00:00:00 2001 > From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Martinez?= > Date: Mon, 18 May 2015 20:59:09 +0200 > Subject: [PATCH] Merge of FrameHeader01() and GlobalHeader() > > FrameHeader01() and GlobalHeader() have a lot of common fields > and having a common function + default value for fields unused > in previous versions is less complex and more coherent than repeating > the common part. maybe but calling the GlobalHeader FrameHeader is very confusing and wrong [...] > @@ -7939,6 +7809,10 @@ pred = j ? initial_states[ i ][j - 1][ k ] : 128 > > initial_state[ i ][ j ][ k ] = ( pred + initial_state_delta[ i ][ j ][ k > ] ) & 255 > +\begin_inset Newline newline > +\end_inset > + > +Inferred to be 0 if not present. what is inferred to be 0? initial_state? initial_state_delta? i think this could be misunderstood as the paragraph is talking about multiple variables [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Asymptotically faster algorithms should always be preferred if you have asymptotical amounts of data signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
