On Sat, Oct 24, 2015 at 9:32 PM, Ronald S. Bultje <rsbul...@gmail.com> wrote:
> Hi,
>
> On Sat, Oct 24, 2015 at 9:02 PM, Ganesh Ajjanagadde <gajjanaga...@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?
Such a wholesale change was not liked by wm4 and Clement due to the
increase in binary size. I have a patch for one such instance that I
believe should be replaced: the performance hit of per frame calls to
qsort as opposed to AV_QSORT in avcodec has bigger impact than the
increase in binary size IMO. This has been ok'ed by Michael.
Thus, this change will be done gradually, with performance measurements.
>
> Ronald
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
