[FFmpeg-devel] [PATCH 3/3] cljrenc: 2x2 ordered dither support.

Michael Niedermayer michaelni at gmx.at
Fri Dec 9 19:01:21 CET 2011


On Fri, Dec 09, 2011 at 03:11:13PM +0000, Paul B Mahol wrote:
> On 12/9/11, Michael Niedermayer <michaelni at gmx.at> wrote:
> > Signed-off-by: Michael Niedermayer <michaelni at gmx.at>
> > ---
> >  libavcodec/cljr.c |    6 ++++++
> >  1 files changed, 6 insertions(+), 0 deletions(-)
> >
> > diff --git a/libavcodec/cljr.c b/libavcodec/cljr.c
> > index ea95901..3d35d8f 100644
> > --- a/libavcodec/cljr.c
> > +++ b/libavcodec/cljr.c
> > @@ -137,6 +137,11 @@ static int encode_frame(AVCodecContext *avctx, unsigned
> > char *buf,
> >      AVFrame *p = data;
> >      int x, y;
> >      uint32_t dither= avctx->frame_number;
> > +    static const uint32_t ordered_dither[2][2]=
> 
> space before =
> > +    {
> > +        {0x10400000, 0x104F0000},
> > +        {0xCB2A0000, 0xCB250000},
> 
> spaces after { and before }
> > +    };
> >
> >      p->pict_type = AV_PICTURE_TYPE_I;
> >      p->key_frame = 1;
> > @@ -151,6 +156,7 @@ static int encode_frame(AVCodecContext *avctx, unsigned
> > char *buf,
> >              switch(a->dither_type){
> >                  case 0: dither = 0x492A0000;                       break;
> >                  case 1: dither = dither*1664525+1013904223;        break;
> > +                case 2: dither = ordered_dither[y&1][(x>>2)&1];    break;
> 
> add spaces around operators, it is harder to read it otherwise.

added some within the available space, changed the rest
suggestions to improve it are welcome


> >              }
> >              put_bits(&pb, 5, (luma[3] +  (dither>>29)   ) >> 3);
> >              put_bits(&pb, 5, (luma[2] + ((dither>>26)&7)) >> 3);
> 
> also new patch which fixes style issues introduced in commit 43a36ad2 which
> was not submitted here for review would be nice.

I think we should concentrate on technical and not
so much style issues.

Also style is generally the choice of the maintainer, i dont want to
mess with it unless iam the maintainer or the maintainer asks me to
do it. Its generally considered rude and childish to reformat a file
maintained by someone else without asking. Now cljr is close to
unmaintained sadly so i dont even know who to ask. But id say before
a big reformat, someone should come forth and volunteer as maintainer
of cljr, its a small file and would likely only take a few minutes
to maintain per month.


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

When the tyrant has disposed of foreign enemies by conquest or treaty, and
there is nothing more to fear from them, then he is always stirring up
some war or other, in order that the people may require a leader. -- Plato
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20111209/a7e07ba2/attachment.asc>


More information about the ffmpeg-devel mailing list