[vlc-devel] [PATCH 3/4] video_output: try to create a decoder device even if creating the window failed
Thomas Guillem
thomas at gllm.fr
Mon Jan 13 16:23:55 CET 2020
On Mon, Jan 13, 2020, at 16:19, Steve Lhomme wrote:
> On 2020-01-13 16:17, Thomas Guillem wrote:
> > Sorry I meant:
> >
> > Why not only requiring a disabled window from a decoder device ?
>
> I don't have an objection if it works.
That way:
- all decoder devices created for display will be created with a window (disabled, even if it's not used by most of them)
- all decoder devices created for sout will be created without a window
- we won't have to wonder if a decoder device created for display was created with or without a window (it will help future debugging).
>
> > The big question for me is: do d3d require the window to be enabled ? (because no other HW APIs need it)
>
> d3d doesn't use the window at all.
I think Only VA need a window_t and it can be disabled.
>
> > I know the problem, I already read it twice ;)
> >
> > On Mon, Jan 13, 2020, at 16:14, Steve Lhomme wrote:
> >> On 2020-01-13 16:03, Thomas Guillem wrote:
> >>>
> >>>
> >>> On Mon, Jan 13, 2020, at 15:59, Steve Lhomme wrote:
> >>>>
> >>>> On 2020-01-13 15:54, Thomas Guillem wrote:
> >>>>>
> >>>>> On Mon, Jan 13, 2020, at 15:47, Rémi Denis-Courmont wrote:
> >>>>>> Hi,
> >>>>>>
> >>>>>> Does VA require an actual window? I'd think it only requires a
> >>>>>> display, so a vout_window_t in disabled state should suffice.
> >>>>>
> >>>>> Indeed, we don't have any API that require an enabled window.
> >>>>> VA only need:
> >>>>> Display *x11dpy = XOpenDisplay(window->display.x11);
> >>>>> VADisplay vadpy = *vadpyp = vaGetDisplay(x11dpy);
> >>>>>
> >>>>> So, I think we could simplify everything by not requiring an enabled
> >>>>> window. I that case, you can load the window module, and create the
> >>>>> decoder device without a valid size.
> >>>>
> >>>> Luckily that may be what this patch does. It's using
> >>>> sys->display_cfg.window regarless of its state. It may be NULL or set
> >>>> but not enabled.
> >>>
> >>> I disagree with this patch. It add one more complexity, different bugs because the decoder device was created with or without a window because the decoder probed it during Open() or when the size was known...
> >>>
> >>> What not requiring the window to be enabled ?
> >>
> >> Because...
> >>
> >>>>>>
> >>>>>> Le 13 janvier 2020 15:56:43 GMT+02:00, Thomas Guillem <thomas at gllm.fr>
> >>>>>> a écrit :
> >>>>>>
> >>>>>>
> >>>>>> On Mon, Jan 13, 2020, at 13:35, Steve Lhomme wrote:
> >>>>>>
> >>>>>> "In some cases (nvdec, mmal, mediacodec ?) the actual size of
> >>>>>> the video
> >>>>>> to decode is known only after data have been processed, long
> >>>>>> after the
> >>>>>> device is setup to use with this API. There are hacks to try
> >>>>>> to guess
> >>>>>> the size early but in the end it's just a hack."
> >>
> >> ...of this...
> >>
> >>
> >>>>>> What to we do when we have a hw decoder that doesn't know its
> >>>>>> actual size but a window is required ? (vaapi).
> >>>>>>
> >>>>>> >
> >>>>>>
> >>>>>> This removes the need for these hacks for those using a
> >>>>>> decoder device
> >>>>>> that doesn't need a window.
> >>>>>>
> >>>>>> On 2020-01-13 13:27, Alexandre Janniaux wrote:
> >>>>>>
> >>>>>> Hi,
> >>>>>>
> >>>>>> This looks dangerous if it is expected to have an enabled
> >>>>>> window at decoder device creation. vout_window handle
> >>>>>> values will basically be undefined without any way for the
> >>>>>> decoder device to know it. Why do you need this?
> >>>>>>
> >>>>>> Regards,
> >>>>>> --
> >>>>>> Alexandre Janniaux
> >>>>>> Videolabs
> >>>>>>
> >>>>>> On Mon, Jan 13, 2020 at 12:07:33PM +0100, Steve Lhomme wrote:
> >>>>>>
> >>>>>> ------------------------------------------------------------------------
> >>>>>> src/video_output/video_output.c | 20 +++++++++-----------
> >>>>>> 1 file changed, 9 insertions(+), 11 deletions(-)
> >>>>>>
> >>>>>> diff --git a/src/video_output/video_output.c
> >>>>>> b/src/video_output/video_output.c
> >>>>>> index 5d583dbe0ca..9540707ab2b 100644
> >>>>>> --- a/src/video_output/video_output.c
> >>>>>> +++ b/src/video_output/video_output.c
> >>>>>> @@ -2099,20 +2099,18 @@ vlc_decoder_device
> >>>>>> *vout_GetDevice(const vout_device_configuration_t *cfg)
> >>>>>> assert(cfg->fmt != NULL);
> >>>>>> vout_thread_sys_t *sys = cfg->vout->p;
> >>>>>>
> >>>>>> - if (!VoutCheckFormat(cfg->fmt))
> >>>>>> - return NULL;
> >>>>>> -
> >>>>>> - video_format_t original;
> >>>>>> - VoutFixFormat(&original, cfg->fmt);
> >>>>>> -
> >>>>>> vlc_mutex_lock(&sys->window_lock);
> >>>>>> - int res = EnableWindowLocked(cfg->vout, &original);
> >>>>>> - if (res == 0 && sys->dec_device == NULL)
> >>>>>> + if (VoutCheckFormat(cfg->fmt))
> >>>>>> + {
> >>>>>> + video_format_t original;
> >>>>>> + VoutFixFormat(&original, cfg->fmt);
> >>>>>> +
> >>>>>> + EnableWindowLocked(cfg->vout, &original);
> >>>>>> + video_format_Clean(&original);
> >>>>>> + }
> >>>>>> + if (sys->dec_device == NULL)
> >>>>>> sys->dec_device =
> >>>>>> vlc_decoder_device_Create(&cfg->vout->obj,
> >>>>>> sys->display_cfg.window);
> >>>>>> dec_device = sys->dec_device ?
> >>>>>> vlc_decoder_device_Hold( sys->dec_device ) : NULL;
> >>>>>> vlc_mutex_unlock(&sys->window_lock);
> >>>>>> - video_format_Clean(&original);
> >>>>>> - if (res != 0)
> >>>>>> - return NULL;
> >>>>>> return dec_device;
> >>>>>> }
> >>>>>> --
> >>>>>> 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
> >>>>>>
> >>>>>> ------------------------------------------------------------------------
> >>>>>> 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
> >>>>>
> >>>>>
> >>>>> _______________________________________________
> >>>>> 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
> >>> _______________________________________________
> >>> 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
> > _______________________________________________
> > 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
More information about the vlc-devel
mailing list