[vlc-devel] [PATCH] HLS: Wait for segment to be available

[Frederic YHUEL]

On Sat, Apr 14, 2012 at 2:25 AM, Tobias Güntner <fatbull at web.de> wrote:
> Hello!
>

Hello Tobias!

> Am 13.04.2012 16:43, schrieb Frédéric Yhuel:
>>          segment_t *segment = GetSegment(s);
>> -        if (segment == NULL)
>> -            break;
>> +        vlc_mutex_lock(&p_sys->download.lock_wait);
>> +        while (segment == NULL)
>> +        {
>> +            msg_Dbg(s, "GetSegment returned NULL");
>> +            if (p_sys->b_error)
>> +            {
>> +                vlc_mutex_unlock(&p_sys->download.lock_wait);
>> +                break;
>
> Something is wrong here. This exits the inner loop only. The result is a
> double unlock and potentially a segfault later because segment is still
> NULL.
>

Ah yes :-s

> It might be cleaner to move the whole code to a separate function;
> something like
>
>        segment_t *segment = WaitForSegment(s);
>        if (segment == NULL)
>            break;
>
> perhaps?
>

Yes good idea.

>> +            }
>> +            vlc_cond_wait(&p_sys->download.wait, &p_sys->download.lock_wait);
>> +            segment = GetSegment(s);
>
> All calls to GetSegment except the first are made while holding the
> lock. Is this really correct/necessary?
>

I don't know if this is correct but this is clearly unnecessary :-)

I will re-submit, thanks for your review!

-- 
Frédéric