[FFmpeg-devel] [PATCH] all: simplify qsort comparators, and add const-correctness

[Ronald S. Bultje]

Hi,

On Sat, Oct 24, 2015 at 9:02 PM, Ganesh Ajjanagadde <gajjanagadde at gmail.com>
wrote:

> All the comparator API needs is > 0, < 0, or = 0 signalling: it does not
> need +1, -1, 0. This avoids some useless branching.
> [..]

diff --git a/cmdutils_opencl.c b/cmdutils_opencl.c
> index 61478e2..d9095b6 100644
> --- a/cmdutils_opencl.c
> +++ b/cmdutils_opencl.c
> @@ -206,7 +206,7 @@ end:
>
>  static int compare_ocl_device_desc(const void *a, const void *b)
>  {
> -    return ((OpenCLDeviceBenchmark*)a)->runtime -
> ((OpenCLDeviceBenchmark*)b)->runtime;
> +    return ((const OpenCLDeviceBenchmark*)a)->runtime - ((const
> OpenCLDeviceBenchmark*)b)->runtime;
>  }
>

OK.


> @@ -2578,8 +2578,7 @@ static InputStream *get_input_stream(OutputStream
> *ost)
>
>  static int compare_int64(const void *a, const void *b)
>  {
> -    int64_t va = *(int64_t *)a, vb = *(int64_t *)b;
> -    return va < vb ? -1 : va > vb ? +1 : 0;
> +    return *(const int64_t *)a - *(const int64_t *)b;
>  }
>

What if the result doesn't fit in int? The input is not int.


> @@ -367,7 +367,7 @@ static int cmp_intervals(const void *a, const void *b)
>      int64_t ts_diff = i1->start_ts - i2->start_ts;
>      int ret;
>
> -    ret = ts_diff > 0 ? 1 : ts_diff < 0 ? -1 : 0;
> +    ret = ts_diff;
>      return ret == 0 ? i1->index - i2->index : ret;
>  }
>

Same here.


>  static int cmp_int(const void *p1, const void *p2)
>  {
> -    int left = *(const int *)p1;
> -    int right = *(const int *)p2;
> -
> -    return ((left > right) - (left < right));
> +    return *(const int *)p1 - *(const int *)p2;
>  }
>

OK.


> @@ -146,12 +146,9 @@ static int cmp_pkt_sub_ts_pos(const void *a, const
> void *b)
>  {
>      const AVPacket *s1 = a;
>      const AVPacket *s2 = b;
> -    if (s1->pts == s2->pts) {
> -        if (s1->pos == s2->pos)
> -            return 0;
> -        return s1->pos > s2->pos ? 1 : -1;
> -    }
> -    return s1->pts > s2->pts ? 1 : -1;
> +    if (s1->pts == s2->pts)
> +        return s1->pos - s2->pos;
> +    return s1->pts - s2->pts;
>  }
>

pos is also int64_t.


> -static int cmp(const int *a, const int *b){
> -    return *a - *b;
> +static int cmp(const void *a, const void *b){
> +    return *(const int *)a - *(const int *)b;
>  }
>

OK.


> -    qsort(remaining_tests + max_tests - num_tests, num_tests,
> sizeof(remaining_tests[0]), (void*)cmp);
> +    qsort(remaining_tests + max_tests - num_tests, num_tests,
> sizeof(remaining_tests[0]), cmp);


I thought all qsorts were changed to AV_QSORT?

Ronald