[FFmpeg-devel] [PATCH] HAM6/HAM8 support for IFF demuxer/decoder

[Martin Storsjö]

On Sun, 9 May 2010, Sebastian Vater wrote:

> Sebastian Vater a ?crit :
> > Martin Storsj? a ?crit :
> >   
> >> No, it is not important for this change. In this patch, you remove usage 
> >> of ff_cmap_read_palette from libavformat/iff.c. You don't _need_ to change 
> >> this now. The code will work just as well without changing this.
> >>   
> >>     
> >
> > It is, simply because it is strictly illegal now to call
> > cmap_read_palette starting with this patch.
> > The thing is when I keep the function as is, I have to keep the
> > prototype declaration also in the header file.
> >
> > But that is dangerous, because people could think that it's still legal
> > to use it with this patch, but it isn't anymore!
> > Or may I move the iff.h declaration of ff_cmap_read_palette in
> > libavcodec/iff.c?
> >   
> 
> The other possibility would be, though that I just do an extra patch
> which removes it from libavfilter alone and on the same hand rename the

libavformat, I assume.

> function and make it static and then create the HAM patch based on this.
> 
> Is this better?

Yes, if you can do that as a separate step, then it's definitely better.

The steps would then be:
1) Make lavf/iff.c stop using ff_cmap_read_palette
2) Make ff_cmap_read_palette static, remove ff_ prefix
3) Add the HAM stuff

Then each step is easily reviewable, don't you agree?

libavformat using functions from libavcodec that aren't in the public API 
is always problematic, and removing such is always a step in a good 
direction, but I think care may need to be taken. Otherwise, if libavcodec 
is upgraded to a newer version without ff_cmap_read_palette, an older 
version of libavformat may break.

So I'd like Reinhard comment on what to do in this case. Is it necessary 
to keep a version of ff_cmap_read_palette with unmodified behaviour, as a 
fallback for older libavformat?

// Martin