[FFmpeg-devel] [PATCH] V210 decoder and encoder

[Michael Niedermayer]

On Mon, May 11, 2009 at 11:37:11PM -0700, Baptiste Coudurier wrote:
> Hi Ramiro,
> 
> On 5/11/2009 4:23 PM, Ramiro Polla wrote:
> > On Mon, May 11, 2009 at 6:41 PM, Baptiste Coudurier
> > <baptiste.coudurier at gmail.com> wrote:
> >> On 5/11/2009 1:59 PM, Reimar D?ffinger wrote:
> >>> On Mon, May 11, 2009 at 12:35:49PM -0700, Baptiste Coudurier wrote:
> >>>> On 5/11/2009 12:27 PM, Reimar D?ffinger wrote:
> >>>>>> +            *udst++ = (v & 0x3FF)      <<  6;
> >>>>>> +            *ydst++ = (v & 0xFFC00)    >>  4;
> >>>>>> +            *vdst++ = (v & 0x3FF00000) >> 14;
> >>>>> Both
> >>>>>
> >>>>>> +            *udst++ = (v & 0x000003FF) <<  6;
> >>>>>> +            *ydst++ = (v & 0x000FFC00) >>  4;
> >>>>>> +            *vdst++ = (v & 0x3FF00000) >> 14;
> >>>>> and
> >>>>>
> >>>>>> +            *udst++ = (v <<  6) & 0xFFC0;
> >>>>>> +            *ydst++ = (v >>  4) & 0xFFC0;
> >>>>>> +            *vdst++ = (v >> 14) & 0xFFC0;
> >>>>> Seem nicer to me.
> >>>> I don't get what you mean.
> >>> That I consider both alternatives more readable.
> >>>
> >>>>> I think the later one might have a speed advantage due to needing only one
> >>>>> constant.
> >>>>> Also like in the encoder you don't really need one of the ands.
> >>>> You mean ydst >> 14 ? What if the 2 bits are not zero ? Doesn't the bit
> >>>> gets replicated ?
> >>> No, the << 6 one, Upper bits are dropped because udst is only 16 bits, lower
> >>> bits 0 is shifted in. Thus the & is pointless.
> >> I see what you mean now.
> >>
> >> Update attached.
> > 
> >> +            v= le2me_32(*src++);
> >> +            *udst++ =  v <<  6;
> >> +            *ydst++ = (v >>  4) & 0xFFC0;
> >> +            *vdst++ = (v >> 14) & 0xFFC0;
> >> +
> >> +            v= le2me_32(*src++);
> >> +            *ydst++ =  v <<  6;
> >> +            *udst++ = (v >>  4) & 0xFFC0;
> >> +            *ydst++ = (v >> 14) & 0xFFC0;
> >> +
> >> +            v= le2me_32(*src++);
> >> +            *vdst++ =  v <<  6;
> >> +            *ydst++ = (v >>  4) & 0xFFC0;
> >> +            *udst++ = (v >> 14) & 0xFFC0;
> >> +
> >> +            v= le2me_32(*src++);
> >> +            *ydst++ =  v <<  6;
> >> +            *vdst++ = (v >>  4) & 0xFFC0;
> >> +            *ydst++ = (v >> 14) & 0xFFC0;
> > 
> > Maybe:
> > #define FOOBAR(a, b, c) \
> > [...]
> > 
> > FOOBAR(u, v, y)
> > FOOBAR(y, u, y)
> > FOOBAR(v, y, u)
> > FOOBAR(y, v, y)
> > 
> > And there are some more further down the patch.
> 
> Why not, updated patch attached.
[...]


> +#if CONFIG_V210_DECODER
[...]
> +#endif
> +
> +#if CONFIG_V210_ENCODER

if the code is splited in 2 files these become unneeded


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

While the State exists there can be no freedom; when there is freedom there
will be no State. -- Vladimir Lenin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090512/043f0141/attachment.pgp>