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

Reply via email to