[FFmpeg-devel] Broken compile with latest libavutil/common.h

[Aurelien Jacobs]

Michael Niedermayer wrote:

> On Thu, Jan 15, 2009 at 12:43:27AM +0000, M?ns Rullg?rd wrote:
> > Aurelien Jacobs <aurel at gnuage.org> writes:
> > 
> > > M?ns Rullg?rd wrote:
> > > [...]
> > > Index: libavutil/internal.h
> > > ===================================================================
> > > --- libavutil/internal.h	(revision 16611)
> > > +++ libavutil/internal.h	(working copy)
> > > @@ -301,4 +301,140 @@
> > >  }
> > >  #endif /* HAVE_TRUNCF */
> > >  
> > > +/* median of 3 */
> > > +static inline av_const int mid_pred(int a, int b, int c)
> > > +{
> > > +#if HAVE_CMOV
> > > +    int i=b;
> > > +    __asm__ volatile(
> > > +        "cmp    %2, %1 \n\t"
> > > +        "cmovg  %1, %0 \n\t"
> > > +        "cmovg  %2, %1 \n\t"
> > > +        "cmp    %3, %1 \n\t"
> > > +        "cmovl  %3, %1 \n\t"
> > > +        "cmp    %1, %0 \n\t"
> > > +        "cmovg  %1, %0 \n\t"
> > > +        :"+&r"(i), "+&r"(a)
> > > +        :"r"(b), "r"(c)
> > > +    );
> > > +    return i;
> > > +#elif 0
> > > +    int t= (a-b)&((a-b)>>31);
> > > +    a-=t;
> > > +    b+=t;
> > > +    b-= (b-c)&((b-c)>>31);
> > > +    b+= (a-b)&((a-b)>>31);
> > > +
> > > +    return b;
> > > +#else
> > > +    if(a>b){
> > > +        if(c>b){
> > > +            if(c>a) b=a;
> > > +            else    b=c;
> > > +        }
> > > +    }else{
> > > +        if(b>c){
> > > +            if(c>a) b=c;
> > > +            else    b=a;
> > > +        }
> > > +    }
> > > +    return b;
> > > +#endif
> > > +}  
> > 
> > This would be better placed nearer the only place that uses it, which
> > is somewhere in lavc.

Attached patch moves it to libavcodec/internal.h.

> [...]
> > 
> > > +/* math */
> > > +int64_t av_const ff_gcd(int64_t a, int64_t b);
> > 
> > The function is in mathematics.c.  Maybe mathematics.h would be a good
> > place for this prototype.  
> 
> > Maybe we should even make that function
> > public.
> 
> I think so too

Done in attached patch.

> > > +/**
> > > + * converts fourcc string to int
> > > + */
> > > +static inline av_pure int ff_get_fourcc(const char *s){
> > > +    assert( strlen(s)==4 );
> > > +    return (s[0]) + (s[1]<<8) + (s[2]<<16) + (s[3]<<24);
> > > +}
> > 
> > This looks a lot like AV_RL32().  
> 
> indeed and i wouldnt mind seeing all ff_get_fourcc replaced by it

See attached patch.

> > > +#if ARCH_X86 || ARCH_PPC || ARCH_BFIN
> > > +#define AV_READ_TIME read_time
> > > +#if ARCH_X86
> > > +static inline uint64_t read_time(void)
> > > +{
> > > +    uint32_t a, d;
> > > +    __asm__ volatile("rdtsc\n\t"
> > > +                 : "=a" (a), "=d" (d));
> > > +    return ((uint64_t)d << 32) + a;
> > > +}
> > > +#elif ARCH_BFIN
> > > +static inline uint64_t read_time(void)
> > > +{
> > > +    union {
> > > +        struct {
> > > +            unsigned lo;
> > > +            unsigned hi;
> > > +        } p;
> > > +        unsigned long long c;
> > > +    } t;
> > > +    __asm__ volatile ("%0=cycles; %1=cycles2;" : "=d" (t.p.lo), "=d" (t.p.hi));
> > > +    return t.c;
> > > +}
> > > +#else //FIXME check ppc64
> > > +static inline uint64_t read_time(void)
> > > +{
> > > +    uint32_t tbu, tbl, temp;
> > > +
> > > +     /* from section 2.2.1 of the 32-bit PowerPC PEM */
> > > +     __asm__ volatile(
> > > +         "1:\n"
> > > +         "mftbu  %2\n"
> > > +         "mftb   %0\n"
> > > +         "mftbu  %1\n"
> > > +         "cmpw   %2,%1\n"
> > > +         "bne    1b\n"
> > > +     : "=r"(tbl), "=r"(tbu), "=r"(temp)
> > > +     :
> > > +     : "cc");
> > > +
> > > +     return (((uint64_t)tbu)<<32) | (uint64_t)tbl;
> > > +}
> > > +#endif
> > > +#elif HAVE_GETHRTIME
> > > +#define AV_READ_TIME gethrtime
> > > +#endif
> > > +
> > > +#ifdef AV_READ_TIME
> > > +#define START_TIMER \
> > > +uint64_t tend;\
> > > +uint64_t tstart= AV_READ_TIME();\
> > > +
> > > +#define STOP_TIMER(id) \
> > > +tend= AV_READ_TIME();\
> > > +{\
> > > +    static uint64_t tsum=0;\
> > > +    static int tcount=0;\
> > > +    static int tskip_count=0;\
> > > +    if(tcount<2 || tend - tstart < FFMAX(8*tsum/tcount, 2000)){\
> > > +        tsum+= tend - tstart;\
> > > +        tcount++;\
> > > +    }else\
> > > +        tskip_count++;\
> > > +    if(((tcount+tskip_count)&(tcount+tskip_count-1))==0){\
> > > +        av_log(NULL, AV_LOG_ERROR, "%"PRIu64" dezicycles in %s, %d runs, %d skips\n",\
> > > +               tsum*10/tcount, id, tcount, tskip_count);\
> > > +    }\
> > > +}
> > > +#else
> > > +#define START_TIMER
> > > +#define STOP_TIMER(id) {}
> > > +#endif
> > 
> > How about a new header, timer.h or so?
> 
> iam in favor of this too

Committed.

Aurel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: move_mid_pred.diff
Type: text/x-patch
Size: 2888 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090116/6d0fa1f7/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: av_gcd.diff
Type: text/x-patch
Size: 7729 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090116/6d0fa1f7/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: replace_ff_get_fourcc.diff
Type: text/x-patch
Size: 13651 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090116/6d0fa1f7/attachment-0002.bin>