On Sat, May 02, 2015 at 01:57:17AM +0200, Jerome Martinez wrote: > New patch because I misunderstood the definition of plane_count. > Now is 1 + ( ( chroma_planes || version < 4 ) ? 1 : 0 ) + ( > alpha_plane ? 1 : 0 ) > > Le 02/05/2015 01:33, Jerome Martinez a écrit : > >Some notes: > >- I discarded the "if version >= 4" stuff for grayscale because I > >don't see such limitation in the bitstream and in the source code. > >I am thinking to add a specific section about decoder limitations > >(e.g. bits_per_raw_sample accepted range, gray/alpha support...) > >- I hesitated to define > > * quant_table_index_count = 1 + ( chroma_planes ? 1 : 0 ) + ( > >alpha_plane ? 1 : 0 ) > > * plane_count = 1 + ( chroma_planes ? 2 : 0 ) + ( alpha_plane ? 1 : 0 ) > > but they are nowhere else in the specification, so I direclty > >put these formulas in the "if". > > I think it is a bit verbose in the "if" but has the advantage > >not to have to define extra parameters and we focus on the > >bitstream instead. > >- xxPlanes and xxLines were never defined, so I replace undefined > >functions by other undefined functions. Planes and Lines functions > >will be defined in future patches. > > > > > >_______________________________________________ > >ffmpeg-devel mailing list > >ffmpeg-devel@ffmpeg.org > >http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
> ffv1.lyx | 794 > +++------------------------------------------------------------ > 1 file changed, 46 insertions(+), 748 deletions(-) > 5f46fea9b250e90d66cf86d4fc50daada26238af > 0001-Reduce-redundancy-in-the-description-of-xxPlane-and-.patch > From a70a7132fe56a144a668736cc7864d9dd232d818 Mon Sep 17 00:00:00 2001 > From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Martinez?= <jer...@mediaarea.net> > Date: Sat, 2 May 2015 01:53:47 +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. 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 -+----------------------------------------+------+ -| for( j = 0; j < plane_count; j++) | | -+----------------------------------------+------+ ++--------------------------------------------------------------------------------------------------------+------+ +| for( j = 0; j < 1 + ( ( chroma_planes || version < 4 ) ? 1 : +0 ) + ( alpha_plane ? 1 : 0 ); j++) | | ++--------------------------------------------------------------------------------------------------------+------+ also a more critical problem is that this patch makes the spec less well defined. 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 ? [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB It is dangerous to be right in matters on which the established authorities are wrong. -- Voltaire
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel