[vlc-devel] [PATCH 2/2] android: window: check jobj before creating window

[Alexandre Janniaux]

Hi,

On Fri, Jan 22, 2021 at 03:31:22PM +0100, Thomas Guillem wrote:
>
>
> On Fri, Jan 22, 2021, at 15:02, Alexandre Janniaux wrote:
> > To avoid creating a AWindowHandler without window.
> > ---
> >  modules/video_output/android/window.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/modules/video_output/android/window.c
> > b/modules/video_output/android/window.c
> > index 0b8d0337ed..0f601e4af2 100644
> > --- a/modules/video_output/android/window.c
> > +++ b/modules/video_output/android/window.c
> > @@ -90,6 +90,11 @@ static const struct vout_window_operations ops = {
> >   */
> >  static int Open(vout_window_t *wnd)
> >  {
> > +    /* We cannot create a window without the associated AWindow
> > instance. */
> > +    jobject jobj = var_InheritAddress(wnd, "drawable-androidwindow");
> > +    if (jobj == NULL)
> > +        return VLC_EGENERIC;
> > +
> >      AWindowHandler *p_awh = AWindowHandler_new(VLC_OBJECT(wnd), wnd,
> >          &(awh_events_t) { OnNewWindowSize, OnNewMouseCoords });
> >      if (p_awh == NULL)
>
> Maybe adding an extra argument to AWindowHandler_new() like a 'bool require_awindow' could be more clear. AWindowHandler_new() would return NULL is the variable is not found and require_awindow is true.

This was the previous behaviour of the AWindowHandler but I think
it goes backward to the last changes in the Android platform.

I'll give a bit of precision here:

My underlying final goal will be to have the AWindowHandler code
only for window module and a different container for decoder device
module (which would be able to access multiple class) but it needs
future changes:

 - a system to get the correct class loader, which is on its way too.

 - the split of AWindow handling to have the JNI part in the java
   binding and allow native implementation, moving the need for
   AWindow class loading.

 - a cleaner probing system (using decoder_UpdateVideoOutput) for
   the pass-through case (android/display.c) which doesn't use the
   decoder device for getting the output, or even unifying the
   surface-based cases. I haven't planned that yet because it's a
   bit complex and I'd like to design this with others.
   In particular, it's a bit backward regarding the push design
   but mandatory unless we support only Android > 26 (and basically
   HardwareBuffer bringing a normal buffer API).

The changes made so far are mostly feature addition, basically:

 - split of SurfaceTexture from AWindow, allowing abritrary
   SrufaceTexture to be created, for instance for transcoding.

 - move of jclass loading to a dedicated container to have
   different AWindow, thus potentially different players
   at the same time.

Those two have cluttered a bit the code since it uses AWindowHandler
even when not handling AWindow at all, but it pushes towards a
cleaner android implementation in the core closer to other platforms.

In the end, I think it'll be much cleaner to have android/utils.c
being the toolkit to use the features of the decoder device (ie.
surface handling) and the AWindowHandler being removed and inlined
into the window module itself.

What's your opinion on that?

Regards,
--
Alexandre Janniaux
Videolabs


> > --
> > 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