[Ffmpeg-devel] [PATCH] TIFF encoder (Google SoC qualification task)
Michael Niedermayer
michaelni
Mon Apr 2 20:18:40 CEST 2007
Hi
On Mon, Apr 02, 2007 at 03:01:29PM +0200, Kamil Nowosad wrote:
> Hi
>
> On Sun, Apr 01, 2007 at 08:23:41PM +0200, Michael Niedermayer wrote:
> > > - *out++ = 0x80 | (count - 1);
> > > + if (style == FF_RLE_SETBIT)
> > > + *out++ = 0x80 | (count - 1);
> > > + else
> > > + *out++ = -count + 1;
> >
> > this can be done without a branch, also it can be done more generically
>
> Is this sufficent:
>
> a = (uint8_t[]{ 1,-1})[style];
> b = (uint8_t[]{0x80, 0})[style];
this is unneeded a/b can just be passed as parameters to the function
>
> and in loop:
>
> *out++ = ((count - 1) | b) * a
> ?
> Or maybe I don't see a more simple solution? (I see only two analogous
> with +, and ^ instead of |)
well your solution is close to what i had in mind but it can be simplified
further (your needs 2 simply arithmetic operations and 1 multiply, it can
be done with only 2 simple arithmetic operations too)
[...]
> > > Index: libavcodec/tiffenc.c
> > > ===================================================================
> > > --- libavcodec/tiffenc.c (wersja 0)
> > > +++ libavcodec/tiffenc.c (wersja 0)
> > [...]
> >
> > > + uint16_t *color_map;
> >
> > this could as well be a local array
>
> Shouldn't I rather allocate the memory for that in tiff_init and free
> in tiff_end ? I forgot about moving it there...
why? its not needed after encoding the current frame or?
[...]
> > > + case 0: // XXX: when a user chose vlc it also assigns the default
> > > + s->compression = TIFF_DEFAULT;
> > > + break;
> >
> > hmm maybe its better to fail with an error if a unsupported compression is
> > choosen
>
> Yes, but the coder_type is 0 both in cases when user uses "-coder vlc" and
> when he doesn't use -coder at all. My idea is to redefine:
>
> #define FF_CODER_TYPE_VLC 0
> #define FF_CODER_TYPE_AC 1
> #define FF_CODER_TYPE_RAW 2 // no coder
> #define FF_CODER_TYPE_RLE 3 // run-length
> #define FF_CODER_TYPE_DEFLATE 4
>
> in avcodec.h to something like that
>
> #define FF_CODER_TYPE_DEFAULT 0
> #define FF_CODER_TYPE_VLC 1
> #define FF_CODER_TYPE_AC 2
> #define FF_CODER_TYPE_RAW 3 // no coder
> #define FF_CODER_TYPE_RLE 4 // run-length
> #define FF_CODER_TYPE_DEFLATE 5
>
> However, it needs changes in ffv1.c. I guess, that the separate patch is
> needed ;-). Should I do it in such way?
well either leave them as is and let the user specifiy the coder or
add a
FF_CODER_TYPE_DEFAULT -1
your suggestion might break applications which dont use AVOption to set the
parameters ...
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
The worst form of inequality is to try to make unequal things equal.
-- Aristotle
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070402/ad08751e/attachment.pgp>
More information about the ffmpeg-devel
mailing list