[vlc-devel] [PATCH] caopengllayer: do not return an error when crop/zoom/aspect/etc fail

Steve Lhomme robux4 at ycbcr.xyz
Thu Oct 24 12:02:13 CEST 2019


On 2019-10-24 11:50, Rémi Denis-Courmont wrote:
> The code is already quite confusing without the patch. It does not make 
> much sense to fail locking the GL, unless there is a bug somewhere else.

There are 2 calls that can fail OpenglLock() one of them being a call to 
Apple API. I agree it should not happen in real like, hence the unlikely().

I think it's good to not silently return no error when in fact the 
original control was not handled properly. At some point we'll probably 
have to handle RESET_PICTURES in modules that currently don't handle the 
controls correctly.

In this case we just avoid the code ending up in vlc_assert_unreachable().

> Le 24 octobre 2019 12:14:29 GMT+03:00, Marvin Scholz 
> <epirat07 at gmail.com> a écrit :
> 
> 
> 
>     On 24 Oct 2019, at 11:10, Steve Lhomme wrote:
> 
>         Otherwise the core will call VOUT_DISPLAY_RESET_PICTURES which we
>         don't support.
> 
>         This is what is done in macosx.m when vlc_gl_MakeCurrent() fails.
>         ------------------------------------------------------------------------
>         modules/video_output/caopengllayer.m | 4 ++--
>         1 file changed, 2 insertions(+), 2 deletions(-)
> 
>         diff --git a/modules/video_output/caopengllayer.m
>         b/modules/video_output/caopengllayer.m
>         index 3b542d1e637..249f7b96d5d 100644
>         --- a/modules/video_output/caopengllayer.m
>         +++ b/modules/video_output/caopengllayer.m
>         @@ -343,8 +343,8 @@ static int Control (vout_display_t *vd, int
>         query,
>         va_list ap)
> 
>         vout_display_place_t place;
>         vout_display_PlacePicture(&place, &vd->source, &cfg_tmp);
>         - if (OpenglLock(sys->gl))
>         - return VLC_EGENERIC;
>         + if (unlikely(OpenglLock(sys->gl)))
>         + return VLC_SUCCESS;
> 
> 
>     Can you add a comment here explaining that? Else the code looks quite
>     confusing here without context.
> 
>     >
> 
>         vout_display_opengl_SetWindowAspectRatio(sys->vgl,
>         (float)place.width / place.height);
>         OpenglUnlock(sys->gl);
>         -- 
>         2.17.1
>         ------------------------------------------------------------------------
>         vlc-devel mailing list
>         To unsubscribe or modify your subscription options:
>         https://mailman.videolan.org/listinfo/vlc-devel
> 
>     ------------------------------------------------------------------------
>     vlc-devel mailing list
>     To unsubscribe or modify your subscription options:
>     https://mailman.videolan.org/listinfo/vlc-devel
> 
> 
> -- 
> Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser 
> ma brièveté.
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
> 


More information about the vlc-devel mailing list