[Ffmpeg-devel] [PATCH] dlopen libamrnb / libamrwb instead of linking

Michael Niedermayer michaelni
Fri Apr 20 23:48:20 CEST 2007


Hi

On Fri, Apr 20, 2007 at 06:45:27PM +0400, Pavlov Konstantin wrote:
> On Thu, Apr 19, 2007 at 09:11:05PM +0200, Michael Niedermayer wrote:
> 
> 
> Here's the updated patch. It solves some problems you mentioned.
> 
> Actually, i'm acting more like a proxy between the patch author and you,
> ffmpeg guys. :]
> 
> > summary, i really like to have dlopen support for amr* as that allows binary
> > packages of ffmpeg to have amr support without strictly depending on libamr
> > to be installed on the end users system
> > but
> > 1. it can be expected that the package maintainer building the binary
> > has libamr
> > 2. the whole code is a mess this can be done much cleaner,that is with
> > a fraction of the #ifdef hell ...
> 
> What we're trying to achieve is to be able to build ffmpeg package in a
> repository that *DOES NOT* contain libamrwb / libamrnb packages due to
> legal issues. That means that package maintainer (me, if it matters)
> cannot add BuildRequires: libamr{n,w}b-devel to ffmpeg spec file.
> 
> That means we have to duplicate some code to not depend on headers from
> those packages.

why not reimplement the headers which are needed?
and no i wont accept the headers in svn but it would be more sane and
would reduce the difference between your ffmpeg with the header and
official ffmpeg with just amr dl support


[...]

> +#define amrnb_decode_fix_avctx(avctx) amr_decode_fix_avctx(avctx, 8000, 160)

this has nothing to do with dl support


[...]
> @@ -479,11 +571,15 @@
>          return -1;
>      }
>  
> +#ifdef CONFIG_LIBAMRNBBIN
> +    written = s->amrnb_encode(s->enstate, s->enc_bitrate, data, frame, 0, 0);
> +#else
>      written = Encoder_Interface_Encode(s->enstate,
>          s->enc_bitrate,
>          data,
>          frame,
>          0);
> +#endif

this and the 500 similar cases can be done by simply always using 
s->amrnb_encode() and seeting that to Encoder_Interface_Encode for the non
dl case


[...]
> @@ -491,8 +587,6 @@
>  
>  #endif
>  
> -#if defined(CONFIG_AMR_NB) || defined(CONFIG_AMR_NB_FIXED)
> -
>  AVCodec amr_nb_decoder =
>  {
>      "amr_nb",

?


[...]

>  
> -/* Common code for fixed and float version*/
>  typedef struct AMRWB_bitrates
>  {

i dont know if this is correct or not but it doesnt belong into a patch which
adds dl support


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I do not agree with what you have to say, but I'll defend to the death your
right to say it. -- Voltaire
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070420/aa2a8642/attachment.pgp>



More information about the ffmpeg-devel mailing list