[FFmpeg-devel] [PATCH] Make h264idct.c compilation optional

Aurelien Jacobs aurel
Tue May 22 19:42:04 CEST 2007


On Tue, 22 May 2007 11:23:15 +0200
Panagiotis Issaris <takis.issaris at uhasselt.be> wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Hi,
> 
> Panagiotis Issaris wrote:
> > Loren Merritt wrote:
> >> On Mon, 14 May 2007, Aurelien Jacobs wrote:
> >>> Panagiotis Issaris wrote:
> >>>
> >>>> The attached patch tries to make the h264idct.c compilation optional.
> >>>> Not meant for direct inclusion, as I am not sure about the correctness...
> >>>>
> >>>> In dsputil_init() currently [1], for lowres decoding, when idct_algo's
> >>>> other than FF_IDCT_INT or FF_IDCT_AUTO are selected, h264 related
> >>>> functions (ff_h264_lowres_idct_put|add_c) are being used. Are these two
> >>>> functions in fact more generic then their name implies (in fact just
> >>>> misnamed), and should they be relocated to some other common file?
> >>>>
> >>>> Or are they really H.264 specific functions? ... but in that case it
> >>>> seems weird that selecting f.e. FF_IDCT_VP3 or FF_IDCT_SIMPLEMMX would
> >>>> result in using these H.264 specific functions.
> >>> Patch seems ok, but I also would like to hear first what's the purpose
> >>> of ff_h264_lowres_idct_put/add_c (is it h264 only or more general ?).
> >> ff_h264_lowres_idct* are not used in h264. They're so named because 
> >> they apply h264's idct algo to other codecs, when whe user requests 
> >> non-standards-compliant speedups. They're not the default because they're 
> >> even less precise than the other lowres idct.
> > 
> > I see; thanks for the explanation.
> > 
> > So, I could:
> > * Move both functions to dsputil.c as they are only used there.
> > * Keep them in h264idct.c, but disable their usage when H.264 decoder
> > support is not compiled in. This shouldn't break anything as in that
> > case ff_jref_idct4_put() would be used instead for lowres==1.
> > 
> > Any opinions?
> 
> This is an updated patch of above mentioned option 2, which was what the
> patch that started this thread did.

After some thought, this solution looks ok to me.
Michael ?

Aurel




More information about the ffmpeg-devel mailing list