[FFmpeg-devel] [RFC] Installing libavutil's crc.h?

Michael Niedermayer michaelni
Wed Dec 12 23:46:16 CET 2007


On Wed, Dec 12, 2007 at 11:22:26PM +0100, Aurelien Jacobs wrote:
> Michael Niedermayer wrote:
> 
> > On Wed, Dec 12, 2007 at 01:48:55AM +0100, Aurelien Jacobs wrote:
> > > Diego Biurrun wrote:
> > > 
> > > > On Tue, Dec 11, 2007 at 11:23:26PM +0100, Michael Niedermayer
> > > > wrote:
> > > > > On Tue, Dec 11, 2007 at 07:35:20PM +0100, Diego Biurrun wrote:
> > > > > > On Tue, Dec 11, 2007 at 04:30:25PM +0100, Diego 'Flameeyes'
> > > > > > Petten? wrote:
> > > > > > > 
> > > > > > > So okay for bswap.h there's the configh dependency that
> > > > > > > disallows from installing it (but I'm still thinking how to
> > > > > > > get over the dep, maybe I'll provide a patch to use
> > > > > > > unifdef).
> > > > > > > 
> > > > > > > For crc.h is there something else stopping it from being
> > > > > > > installed?
> > > > > > 
> > > > > > This sounds backwards to me.  Headers should not be installed
> > > > > > without a good reason.
> > > > > 
> > > > > is it not enogh that its needed to use the crc code in
> > > > > libavuitl? its a mere oversight that it isnt installed
> > > > 
> > > > It is a good enough reason.
> > > > 
> > > > I'm thinking that we could solve the LIBAVUTIL_VERSION_INT issue
> > > > by switching the condition around, i.e.:
> > > > 
> > > > #if LIBAVUTIL_VERSION_INT  < (50<<16)
> > > > extern AVCRC *av_crcEDB88320;
> > > > extern AVCRC *av_crc04C11DB7;
> > > > extern AVCRC *av_crc8005    ;
> > > > extern AVCRC *av_crc07      ;
> > > > #else
> > > > extern AVCRC av_crcEDB88320[];
> > > > extern AVCRC av_crc04C11DB7[];
> > > > extern AVCRC av_crc8005    [];
> > > > extern AVCRC av_crc07      [];
> > > > #endif
> > > 
> > > If crc.h wasn't part of public API up to now, I wonder why do we
> > > try to keep stable API/ABI ?
> > > Is it to ensure compatibility between lavu and lavf compiled from
> > > different svn revision ? (I thought this wasn't supported ?)
> > > 
> > > So a simple solution to solve the LIBAVUTIL_VERSION_INT issue would
> > > be to just drop the legacy version of those declaration.
> > 
> > hmm, i guess thats ok as well
> 
> I now realize that we are talking about making those extern tables part
> of the public API. Exactly what we are trying to avoid right now...
> So I think the right thing to do is to make those tables static and
> provide another API to use them. Right ?
> See attached patch.
> 
> Aurel

> Index: libavutil/crc.c
> ===================================================================
> --- libavutil/crc.c	(revision 11206)
> +++ libavutil/crc.c	(working copy)
> @@ -21,17 +21,11 @@
>  #include "common.h"
>  #include "crc.h"
>  
> -#if LIBAVUTIL_VERSION_INT  < (50<<16)
> -AVCRC *av_crcEDB88320;
> -AVCRC *av_crc04C11DB7;
> -AVCRC *av_crc8005    ;
> -AVCRC *av_crc07      ;
> -#else
> -AVCRC av_crcEDB88320[257];
> -AVCRC av_crc04C11DB7[257];
> -AVCRC av_crc8005    [257];
> -AVCRC av_crc07      [257];
> -#endif
> +static AVCRC av_crcEDB88320[257];
> +static AVCRC av_crc04C11DB7[257];
> +static AVCRC av_crc1021    [257];
> +static AVCRC av_crc8005    [257];
> +static AVCRC av_crc07      [257];


/**
 * Inits a crc table.
 * @param ctx must be an array of sizeof(AVCRC)*257 or sizeof(AVCRC)*1024
                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

and besides this i do not like the patch at all, it would significantly
increase the call overhead of the crc functions

why dont you just hardcode the tables then broken linkers can copy them
around as they see fit? that also would allow them to be actually shared

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

Frequently ignored awnser#1 FFmpeg bugs should be sent to our bugtracker, user
questions for the command line tools ffmpeg, ffplay, ... as well as questions
about how to use libav* should be sent to the ffmpeg-user mailinglist.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20071212/ac5cb993/attachment.pgp>



More information about the ffmpeg-devel mailing list