[FFmpeg-devel] [PATCH] ffplay: fix a crash caused by aborting the video queue

[Marton Balint]

On Sun, 28 Aug 2011, avcoder wrote:

> On Sun, Aug 28, 2011 at 12:40 AM, Marton Balint <cus at passwd.hu> wrote:
>>
>> On Sat, 27 Aug 2011, avcoder wrote:
>>
>>> On Sat, Aug 27, 2011 at 6:09 PM, Marton Balint <cus at passwd.hu> wrote:
>>>>
>>>> Yes, the patch description is probably not entirely correct, sorry, my
>>>> mistake. The thought of changing w_index somehow remained in my mind from
>>>> the time I debugged this.
>>>>
>>>> The main problem with the code is that multiple ALLOC events may occur at
>>>> the same time if we don't pop them from the event queue on abort. If
>>>> these
>>>> alloc events tries to allocate the same vp, vp->bmp changes (free-d, and
>>>> the
>>>> then allocated again), I typically got the crash when
>>>> SDL_LockYUVOverlay(vp->bmp) was called from the video thread, after
>>>> SDL_FreeYUVOverlay(vp->bmp) was called from the main thread.
>>>
>>> Why do you get multiple ALLOC envents?
>>>
>>> I checked the ALLOC events, FF_ALLOC_EVENT is only pushed in
>>> "queue_picture()" which is called by "video_thread()", it is
>>> impossible to have MULTIPLE FF_ALLOC_EVENT.
>>
>> When you abort the video queue, an ALLOC event may be in the event queue.
>> When another video_thread() is stared (e.g. when changing the video stream)
>> the event from the first video_thread may still be in the queue.
>>
>
> OK, that's the point.
>
> But this scenario has no relationship with your previous explanation
> -----'if these alloc events tries to allocate the same vp...., "
> The vp is not same if you start another video_thread(). so there's no
> scenario to produce your bug: "  I typically got the crash when
> SDL_LockYUVOverlay(vp->bmp) was called from the video thread, after
> SDL_FreeYUVOverlay(vp->bmp) was called from the main thread."
>
> Your patch is harmless, but useless.
>

I don't agree, because with my patch, alloc_picture and 
SDL_LockYUVOverlay of queue_picture cannot run at the same time, 
therefore there is no chance that SDL_LockYUVOverlay will do a lock 
using a currently freed pointer, which may be the case, if 
SDL_LockYUVOverlay happens when the main thread is in the middle of 
alloc_picture.

Regards,
Marton