[vlc-devel] [PATCH] android: utils: avoid non-android windows

[Alexandre Janniaux]

Hi,

Indeed you're right, the fix is a side effect of wnd type not
being defined in window Open before creating the AWindowHandler
and is inapropriate to fix the real issue.

The correct fix should be removing this special case instead:

diff --git a/lib/media_player.c b/lib/media_player.c
index 58dfbc39fb..9341c49a68 100644
--- a/lib/media_player.c
+++ b/lib/media_player.c
@@ -1036,12 +1036,7 @@ bool libvlc_video_set_output_callbacks(libvlc_media_player_t *mp,
                                        void *opaque)
 {
     static_assert(libvlc_video_engine_disable == 0, "No engine set must default to 0");
-#ifdef __ANDROID__
-    //use the default android window
-    var_SetString( mp, "window", "");
-#else
     var_SetString( mp, "window", "wextern");
-#endif

     if( engine == libvlc_video_engine_gles2 )
     {

I'll test that it fixes the crash and submit it.

Thanks for review,
--
Alexandre Janniaux
Videolabs

On Fri, Jan 22, 2021 at 07:59:54AM +0100, Steve Lhomme wrote:
> On 2021-01-21 10:33, Alexandre Janniaux wrote:
> > When using opengl callbacks on Android, a dummy window is passed instead
> > of NULL or an Android window. As long as JVM has been given, we can
> > still create the AWindowHandler for the decoder device though.
>
> Is this fixing anything ? Because I only see 2 calls.
>
> In android/window OpenDecDevice:
>     if (window && window->type == VOUT_WINDOW_TYPE_ANDROID_NATIVE)
>         awh = window->handle.anativewindow;
>     else
>         awh = AWindowHandler_new(VLC_OBJECT(device), NULL, NULL);
>
> In this case the value is NULL and the code may explode during calls to to
> internal functions that use it to log things or even other code.
> AWindowHandler_new itself properly checks for NULL (but your patch doesn't
> and will crash with a NULL dereference).
>
> In android/window Open:
>     AWindowHandler *p_awh = AWindowHandler_new(VLC_OBJECT(wnd), wnd,
>         &(awh_events_t) { OnNewWindowSize, OnNewMouseCoords });
>     if (p_awh == NULL)
>         return VLC_EGENERIC;
>
>     wnd->type = VOUT_WINDOW_TYPE_ANDROID_NATIVE;
>
> Here the window type is not even set during the call.
>
> > ---
> >   modules/video_output/android/utils.c | 3 +++
> >   1 file changed, 3 insertions(+)
> >
> > diff --git a/modules/video_output/android/utils.c b/modules/video_output/android/utils.c
> > index e46b8e004d..c4c549e476 100644
> > --- a/modules/video_output/android/utils.c
> > +++ b/modules/video_output/android/utils.c
> > @@ -818,6 +818,9 @@ AWindowHandler_new(vlc_object_t *obj, vout_window_t *wnd, awh_events_t *p_events
> >   #define AWINDOW_REGISTER_FLAGS_SUCCESS 0x1
> >   #define AWINDOW_REGISTER_FLAGS_HAS_VIDEO_LAYOUT_LISTENER 0x2
> > +    if (wnd->type != VOUT_WINDOW_TYPE_ANDROID_NATIVE)
> > +        wnd = NULL;
> > +
> >       AWindowHandler *p_awh;
> >       JNIEnv *p_env;
> >       JavaVM *p_jvm = var_InheritAddress(obj, "android-jvm");
> > --
> > 2.30.0
> >
> > _______________________________________________
> > 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