[FFmpeg-devel] [PATCH] IFF: Make lavf/iff.c stop using ff_cmap_read_palette

[Martin Storsjö]

On Sun, 9 May 2010, Sebastian Vater wrote:

> Martin Storsj? a ?crit :
> > On Sun, 9 May 2010, Sebastian Vater wrote:
> >
> >   
> >> Martin recommeded to do these as a separate patch.
> >>
> >> It removes call to ff_cmap_read_palette in lavf/iff.c for preparation of
> >> the HAM patch.
> >>     
> >
> > Does an old lavc/iff.c handle a new demuxer behaving in this way, or why 
> > is the change to lavc/iff.c included here? In that case, this can lead to 
> > problems when mixing lavf/lavc versions.
> >   
> 
> Removing ff_cmap_read_palette from lavf/iff.c requires that the decoder
> handles IFF-PBM uncompressed files by itself, since it can't be passed
> to CODEC_ID_RAWVIDEO anymore.
> 
> But that isn't a problem, because old lavf/iff.c doesn't pass the data
> at all to lavc/iff.c in that case.
> 
> However a new lavf/iff.c could pass to old lavc/iff.c in that case it
> won't decode uncompressed IFF-PBM files anymore. It would threat them as
> IFF-ILBM uncompressed and therefore call the decodeplane8 version which
> will result in corrupted image data.
> 
> The only way I see to fix this is to increment the lavc version number
> or to add the new IFF_EXTRA_CONTEXT_SIZE stuff, also. What do you suggest?

Ok, so this is a general problem that you would have got even if you 
didn't split this out as a separate patch, but now it is much much easier 
to spot and discuss, instead of having it mixed up in the middle of 
discussions of fifty other things. See how splitting things into coherent 
pieces aid the review?

A third option would be to make a local copy of ff_cmap_read_palette in 
lavf, so that lavf/iff wouldn't depend on functions from lavc/iff, but 
that's a bit of code duplication, and probably not appreciated.

In this case, if a new version of lavf demuxes stuff to a new "format" 
which earlier lavc doesn't support, I find it acceptable, especially since 
it's a fringe format... If the old lavc still decodes it, but incorrectly, 
that's a problem with the earlier lavc version which is fixed by a newer 
version of lavc.

// Martin