[FFmpeg-devel] [PATCH 2/2] lavfi/frei0r: factorize plugin loading with static path

[Marton Balint]

On Sat, 22 Feb 2014, Reimar Döffinger wrote:

> On Sat, Feb 22, 2014 at 05:57:52PM +0100, Marton Balint wrote:
>> +    for (i = 0; !s->dl_handle && frei0r_pathlist[i];) {
>> +        ret = load_path(ctx, &s->dl_handle, frei0r_pathlist[i++], dl_name);
>
> Wouldn't it be nicer to have the i++ in the for () so it looks like
> a standard look and one doesn't have to scratch one's head?
> Otherwise both look good to me.

Okay, I will fix it and post a v2.

>
>>          if (ret < 0)
>>              return ret;
>
> Though I kind of wonder if it wouldn't make sense to continue even if
> load_path returns < 0.
> I.e. something like
>
>> +    for (i = 0; !s->dl_handle && frei0r_pathlist[i]; ++) {
>> +        ret = load_path(ctx, &s->dl_handle, frei0r_pathlist[i], dl_name);
>>          if (ret < 0)
>               s->dl_handle = NULL;
>>      }
>       if (ret < 0)
>           return ret;
>
> I didn't check when load_path returns < 0, so it may or may not make
> sense.

I don't think it matters, the way I see it load_path only returns 
AVERROR(ENOMEM), so it is probably better to bail out asap.

Regards,
Marton