On Sun, Oct 18, 2015 at 11:03 AM, Clément Bœsch <u...@pkh.me> wrote:
> On Sun, Oct 18, 2015 at 10:47:52AM -0400, Ganesh Ajjanagadde wrote:
> [...]
>> diff --git a/libavformat/subtitles.c b/libavformat/subtitles.c
>> index bb89766..16ab245 100644
>> --- a/libavformat/subtitles.c
>> +++ b/libavformat/subtitles.c
>> @@ -23,6 +23,7 @@
>> #include "avio_internal.h"
>> #include "libavutil/avassert.h"
>> #include "libavutil/avstring.h"
>> +#include "libavutil/qsort.h"
>>
>> void ff_text_init_avio(void *s, FFTextReader *r, AVIOContext *pb)
>> {
>> @@ -197,9 +198,12 @@ void ff_subtitles_queue_finalize(void *log_ctx,
>> FFDemuxSubtitlesQueue *q)
>> {
>> int i;
>>
>> - qsort(q->subs, q->nb_subs, sizeof(*q->subs),
>> - q->sort == SUB_SORT_TS_POS ? cmp_pkt_sub_ts_pos
>> - : cmp_pkt_sub_pos_ts);
>> + if (q->sort == SUB_SORT_TS_POS) {
>> + AV_QSORT(q->subs, q->nb_subs, AVPacket, cmp_pkt_sub_ts_pos);
>> + }
>> + else
>> + AV_QSORT(q->subs, q->nb_subs, AVPacket, cmp_pkt_sub_pos_ts);
>> +
>
> Weird style.
There were two reasons for this:
1. This ensures inlining of the cmp callback.
2. I was not sure whether putting the cmp ternary inside the AV_QSORT
is safe since it is a macro. Did not want to check that, so used the
above.
>
> BTW, AV_QSORT() Macro should be replaced with a do { ... } while (0) form
> to make this kind of code safer.
Maybe, but that is separate from this patch.
>
> Also note that using these macro has an impact on final binary size which
> might not be worth the trouble in various cases.
So how do you want me to measure binary size impact? Or more
concretely: which of the above replacements do you think is not
worthwhile (e.g due to small array size)?
>
> [...]
>
> --
> Clément B.
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
