[FFmpeg-devel] [PATCH] X-Face image encoder and decoder
Stefano Sabatini
stefasab at gmail.com
Sun Oct 14 11:38:11 CEST 2012
On date Sunday 2012-10-14 01:46:39 +0200, Clément Bœsch encoded:
> On Sun, Oct 14, 2012 at 01:30:33AM +0200, Stefano Sabatini wrote:
> [...]
> > > > + return AVERROR(EINVAL);
> > > > + }
> > > > + }
> > > > + avctx->width = XFACE_WIDTH;
> > > > + avctx->height = XFACE_HEIGHT;
> > > > +
> > > > + /* convert image from MONOWHITE to 1=black 0=white bitmap */
> > > > + buf = frame->data[0];
> > > > + for (i = 0, j = 0; i < XFACE_PIXELS; ) {
> > > > + for (k = 0; k < 8; k++)
> > > > + xface->bitmap[i++] = !!((1<<(7-k)) & buf[j]);
> > >
> > > Wouldn't it be faster/simpler to do something like (untested):
> > >
> > > xface->bitmap[i++] = buf[j]>>(7-k) & 1;
> > >
> > > ?
> >
> > changed to:
> > xface->bitmap[i++] = !!(buf[j] & (0x80>>k));
> >
> > which I consider a bit more intelligible (and consistent with what I
> > did in xface.c).
> >
>
> FYI, it seems your original code and my proposal outputs the same assembler
> with gcc (-O2), but your proposition adds a test:
>
> uint8_t f1(uint8_t b, int k) { return !!((1<<(7-k)) & b); }
> uint8_t f2(uint8_t b, int k) { return b>>(7-k) & 1; }
> uint8_t f3(uint8_t b, int k) { return !!(b & 0x80>>k); }
>
>
> 0000000000000000 <f1>:
> 0: b9 07 00 00 00 mov ecx,0x7
> 5: 40 0f b6 c7 movzx eax,dil
> 9: 29 f1 sub ecx,esi
> b: d3 f8 sar eax,cl
> d: 83 e0 01 and eax,0x1
> 10: c3 ret
>
> 0000000000000020 <f2>:
> 20: b9 07 00 00 00 mov ecx,0x7
> 25: 40 0f b6 c7 movzx eax,dil
> 29: 29 f1 sub ecx,esi
> 2b: d3 f8 sar eax,cl
> 2d: 83 e0 01 and eax,0x1
> 30: c3 ret
>
> 0000000000000040 <f3>:
> 40: 89 f1 mov ecx,esi
> 42: b8 80 00 00 00 mov eax,0x80
> 47: 40 0f b6 ff movzx edi,dil
> 4b: d3 f8 sar eax,cl
> 4d: 85 f8 test eax,edi
> 4f: 0f 95 c0 setne al
> 52: c3 ret
>
> (I'm not trying to argue about any solution, just found that funny)
The !! is tricking gcc. Anyway changed globally to:
(buf[j]>>(7-k))&1;
>
> Ah and BTW, I think you can make buf const.
Changed.
I'll push the patch in a day if I read no more comments.
--
FFmpeg = Funny Frenzy Mastering Portable Enhancing Guru
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-lavc-add-xface-image-decoder-and-encoder.patch
Type: text/x-diff
Size: 36323 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20121014/a80db7d7/attachment.bin>
More information about the ffmpeg-devel
mailing list