[FFmpeg-devel] Various small fixes

[Måns Rullgård]

"Anders Gr?nberg" <galileo.m2 at gmail.com> writes:

> I have been experimenting with FFmpeg and this patch fixes some small
> issues I found in the code base that should be beneficial to other
> FFmpeg developers.
>
> Index: libavcodec/dsputil.c
> ===================================================================
> --- libavcodec/dsputil.c	(revision 16089)
> +++ libavcodec/dsputil.c	(working copy)
> @@ -3458,7 +3458,7 @@
>  static void diff_bytes_c(uint8_t *dst, uint8_t *src1, uint8_t *src2, int w){
>      long i;
>  #ifndef HAVE_FAST_UNALIGNED
> -    if((long)src2 & (sizeof(long)-1)){
> +    if((intptr_t)src2 & (sizeof(src2)-1)){
>          for(i=0; i+7<w; i+=8){
>              dst[i+0] = src1[i+0]-src2[i+0];
>              dst[i+1] = src1[i+1]-src2[i+1];
> @@ -4186,7 +4186,7 @@
>      static int did_fail=0;
>      DECLARE_ALIGNED_16(int, aligned);
>  
> -    if((long)&aligned & 15){
> +    if((intptr_t)&aligned & 15){
>          if(!did_fail){
>  #if defined(HAVE_MMX) || defined(HAVE_ALTIVEC)
>              av_log(NULL, AV_LOG_ERROR,

There was some system that didn't have intptr_t (although it is
required), so using long (or int) here is more portable.  What do you
suppose this change fixes?

> Index: libavcodec/dsputil.h
> ===================================================================
> --- libavcodec/dsputil.h	(revision 16089)
> +++ libavcodec/dsputil.h	(working copy)
> @@ -712,11 +712,11 @@
>   * @param   n       size of half window
>   */
>  void ff_sine_window_init(float *window, int n);
> -extern float ff_sine_128 [ 128];
> -extern float ff_sine_256 [ 256];
> -extern float ff_sine_512 [ 512];
> -extern float ff_sine_1024[1024];
> -extern float ff_sine_2048[2048];
> +DECLARE_ALIGNED(16, extern float, ff_sine_128 [ 128]);
> +DECLARE_ALIGNED(16, extern float, ff_sine_256 [ 256]);
> +DECLARE_ALIGNED(16, extern float, ff_sine_512 [ 512]);
> +DECLARE_ALIGNED(16, extern float, ff_sine_1024[1024]);
> +DECLARE_ALIGNED(16, extern float, ff_sine_2048[2048]);
>  extern float *ff_sine_windows[5];
>  
>  int ff_mdct_init(MDCTContext *s, int nbits, int inverse);

These are extern declarations of data.  Using DECLARE_ALIGNED here
will not make change anything.

> Index: libavcodec/imgconvert.c
> ===================================================================
> --- libavcodec/imgconvert.c	(revision 16089)
> +++ libavcodec/imgconvert.c	(working copy)
> @@ -783,7 +783,7 @@
>      dst_pix_fmt = -1;
>      min_dist = 0x7fffffff;
>      for(i = 0;i < PIX_FMT_NB; i++) {
> -        if (pix_fmt_mask & (1 << i)) {
> +        if (pix_fmt_mask & (1ULL << i)) {
>              loss = avcodec_get_pix_fmt_loss(i, src_pix_fmt, has_alpha) & loss_mask;
>              if (loss == 0) {
>                  dist = avg_bits_per_pixel(i);

This is madness.  What do you believe the benefit of that to be?

> Index: libavformat/mov.c
> ===================================================================
> --- libavformat/mov.c	(revision 16089)
> +++ libavformat/mov.c	(working copy)
> @@ -166,7 +166,7 @@
>      int (*parse)(MOVContext *ctx, ByteIOContext *pb, MOVAtom atom);
>  } MOVParseTableEntry;
>  
> -static const MOVParseTableEntry mov_default_parse_table[];
> +const MOVParseTableEntry mov_default_parse_table[];
>  
>  static int mov_read_default(MOVContext *c, ByteIOContext *pb, MOVAtom atom)
>  {

Why?

> Index: libavutil/common.h
> ===================================================================
> --- libavutil/common.h	(revision 16089)
> +++ libavutil/common.h	(working copy)
> @@ -108,7 +108,7 @@
>  /* assume b>0 */
>  #define ROUNDED_DIV(a,b) (((a)>0 ? (a) + ((b)>>1) : (a) - ((b)>>1))/(b))
>  #define FFABS(a) ((a) >= 0 ? (a) : (-(a)))
> -#define FFSIGN(a) ((a) > 0 ? 1 : -1)
> +#define FFSIGN(a) ((a) >= 0 ? 1 : -1)
>  
>  #define FFMAX(a,b) ((a) > (b) ? (a) : (b))
>  #define FFMAX3(a,b,c) FFMAX(FFMAX(a,b),c)

Did this fix something?  Are you sure it didn't break something?

-- 
M?ns Rullg?rd
mans at mansr.com