[Ffmpeg-devel] [PATCH] TIFF encoder (Google SoC qualification task)

[Kamil Nowosad]

Hi

On Mon, Apr 02, 2007 at 08:18:40PM +0200, Michael Niedermayer wrote:
> Hi
> 
> > 
> > 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)

Huh, Bartlomiej has overtaken me with a good idea of xor and + ;-). I
think it'll be good to use it.

> > > > 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?

Although the solution with local array is somewhat simpler, I think that
allocating the memory for palette once in tiff_init and freeing in
tiff_end is somewhat more elegant than reserving it each time (even if
it the palette is not used at all) on the stack.

> > > > +    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 ...

I have applied the second solution.

-- 
Best regards,
Kamil Nowosad