[vlc-devel] [PATCH] picture_pool: fix double mutex_unlock if pic_lock fails

[Thomas Guillem]

On Mon, Nov 3, 2014, at 11:44, Rémi Denis-Courmont wrote:
> Le 2014-11-03 13:22, Thomas Guillem a écrit :
> > ---
> >  src/misc/picture_pool.c | 1 -
> >  1 file changed, 1 deletion(-)
> >
> > diff --git a/src/misc/picture_pool.c b/src/misc/picture_pool.c
> > index 15d0791..dac7ef6 100644
> > --- a/src/misc/picture_pool.c
> > +++ b/src/misc/picture_pool.c
> > @@ -253,7 +253,6 @@ picture_t *picture_pool_Get(picture_pool_t *pool)
> >              vlc_mutex_lock(&pool->lock);
> >              sys->in_use = false;
> >              pool->refs--;
> > -            vlc_mutex_unlock(&pool->lock);
> >              continue;
> >          }
> 
> The patch is obviously correct.
> 
> I suspect you are abusing the lock feature though. Locking a picture 
> should only fail in exceptional circumstances such as OOM. To retain 
> "busy" pictures, the video output must use 
> picture_Hold()/picture_Release(), not lock/unlock.

I didn't write the code that fails here.
But I had a look, and found where if fails:

    if (info->w != aligned_width || info->h != sh ||
    sys->b_changed_crop) {
        // input size doesn't match the surface size -> request a resize
        jni_SetAndroidSurfaceSize(aligned_width, sh,
        sys->fmt.i_visible_width, sys->fmt.i_visible_height,
        sys->i_sar_num, sys->i_sar_den);
        // When using ANativeWindow, one should use
        ANativeWindow_setBuffersGeometry
        // to set the size and format. In our case, these are set via
        the SurfaceHolder
        // in Java, so we seem to manage without calling this
        ANativeWindow function.
        sys->s_unlockAndPost(surf);
        jni_UnlockAndroidSurface();
        sys->b_changed_crop = false;
        return VLC_EGENERIC;
    }

I think, instead of returning return VLC_EGENERIC, we should try again
to lock a picture from android. 

> 
> -- 
> Rémi Denis-Courmont
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel