[FFmpeg-devel] [PATCH] HAM6/HAM8 support for IFF demuxer/decoder
Sun May 9 19:21:08 CEST 2010
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?
More information about the ffmpeg-devel