[vlc-devel] [PATCH 10/39] decoder: request decoder device when getting the vout

[Alexandre Janniaux]

Hi,

Comments inline,


On Mon, Oct 07, 2019 at 09:04:19PM +0300, Rémi Denis-Courmont wrote:
> Le maanantaina 7. lokakuuta 2019, 17.28.49 EEST Steve Lhomme a écrit :
> > The decoder device is kept in the decoder for now (in further patches it
> > will come with the video context).
> > ---
> >  src/input/decoder.c  | 12 ++++++++++--
> >  src/input/resource.c |  5 +++--
> >  src/input/resource.h |  5 +++--
> >  3 files changed, 16 insertions(+), 6 deletions(-)
> >
> > diff --git a/src/input/decoder.c b/src/input/decoder.c
> > index df2efd7717a..b22f4c66362 100644
> > --- a/src/input/decoder.c
> > +++ b/src/input/decoder.c
> > @@ -122,6 +122,7 @@ struct decoder_owner
> >      audio_output_t *p_aout;
> >
> >      vout_thread_t   *p_vout;
> > +    vlc_decoder_device *p_dec_dev; // TEMPORARY
> >
> >      /* -- Theses variables need locking on read *and* write -- */
> >      /* Preroll */
> > @@ -553,13 +554,17 @@ static int CreateVoutIfNeeded(struct decoder_owner
> > *p_owner) .dpb_size = dpb_size + p_dec->i_extra_picture_buffers + 1,
> > .mouse_event = MouseEvent, .mouse_opaque = p_dec
> >      };
> > -    p_vout = input_resource_GetVout( p_owner->p_resource,
> > -                                    &cfg, &order );
> > +    vlc_decoder_device *dec_dev = NULL;
> > +    p_vout = input_resource_GetVoutDecoderDevice( p_owner->p_resource,
> > +                                    &cfg, &order, &dec_dev );
> >      if (p_vout)
> >          decoder_Notify(p_owner, on_vout_added, p_vout, order);
> >
> >      vlc_mutex_lock( &p_owner->lock );
> >      p_owner->p_vout = p_vout;
> > +    if ( p_owner->p_dec_dev != NULL )
> > +        vlc_decoder_device_Release( p_owner->p_dec_dev );
> > +    p_owner->p_dec_dev = dec_dev;
>
> Again, AFAIK, the plan was to PUSH the decoder device from the decoder to the
> video output, not the other way around to PULL. Otherwis, this is pull model
> all over again. Specifically, the decoder should have the choice to request a
> device from the decoder owner, and that's what most (initially all) would do.
>
> But to eventually support capture devices or such things, the decoder must be
> able to push another decoder device of its own anyway.
>

Is it really meaningful to push a decoder device in the
pipeline? If it is conveying an hint on which hardware will
be used for the final rendering, and is created from a
window, I can't see how intermediate filters and objects in
the pipeline can possibly change it to another one.

Maybe you're talking about changing the wrapper around the
decoder device for interoperability purpose? This can
probably be schemed after all the pipeline is in place but
it feels like duplicating the role of the video context.

>From the display perspective, as it is directly attached to
windowing resources, I guess it makes sense that it can
directly interact with the pipeline's decoder device?

I'm not sure of what was exactly decided but I'm sure we
talked first about video context, then hw_hints, then did
the initial work on decoder device to tie both concepts.
I'm not sure it was planned that decoder device was to
replace video context, but maybe it was confusing as we
didn't always thought about what it would be like with
filters and simplified the scheme for the initial work.

Your question around capture devices is interesting. It
might be a bit early to develop this now, but Steve has
probably some work gravitating around DXGI capture and
probably thought about it. It is starting to be out of my
own understanding but I have the feeling that it is
actually the same thing as the paragraph above, meaning
that it would ideally be the DXGI capture device (the
decoder here), which adapts to the decoder device and not
the opposite.

Regards,
--
Alexandre Janniaux
Videolabs

> > @@ -1925,6 +1930,9 @@ static void DeleteDecoder( decoder_t * p_dec )
> >      const enum es_format_category_e i_cat =p_dec->fmt_in.i_cat;
> >      decoder_Clean( p_dec );
> >
> > +    if ( p_owner->p_dec_dev )
> > +        vlc_decoder_device_Release( p_owner->p_dec_dev );
> > +
> >      /* Free all packets still in the decoder fifo. */
> >      block_FifoRelease( p_owner->p_fifo );
> >
> > diff --git a/src/input/resource.c b/src/input/resource.c
> > index 0c9693fb9e8..984d68e3ffa 100644
> > --- a/src/input/resource.c
> > +++ b/src/input/resource.c
> > @@ -368,9 +368,10 @@ void input_resource_PutVout(input_resource_t
> > *p_resource, vlc_mutex_unlock( &p_resource->lock );
> >  }
> >
> > -vout_thread_t *input_resource_GetVout(input_resource_t *p_resource,
> > +vout_thread_t *input_resource_GetVoutDecoderDevice(input_resource_t
> > *p_resource, const vout_configuration_t *cfg, -
> >          enum vlc_vout_order *order) +
> > enum vlc_vout_order *order, +
> > vlc_decoder_device **pp_dec_dev) {
> >      vout_configuration_t cfg_buf;
> >      vout_thread_t *vout;
> > diff --git a/src/input/resource.h b/src/input/resource.h
> > index a13ca0dc85c..537033c9d7c 100644
> > --- a/src/input/resource.h
> > +++ b/src/input/resource.h
> > @@ -37,9 +37,10 @@ void input_resource_SetInput( input_resource_t *,
> > input_thread_t * ); */
> >  sout_instance_t *input_resource_RequestSout( input_resource_t *,
> > sout_instance_t *, const char *psz_sout );
> >
> > -vout_thread_t *input_resource_GetVout(input_resource_t *,
> > +vout_thread_t *input_resource_GetVoutDecoderDevice(input_resource_t *,
> >                                        const vout_configuration_t *,
> > -                                      enum vlc_vout_order *order);
> > +                                      enum vlc_vout_order *order,
> > +                                      vlc_decoder_device **);
> >  void input_resource_PutVout(input_resource_t *, vout_thread_t *);
> >
> >  /**
>
>
> --
> 雷米‧德尼-库尔蒙
> http://www.remlab.net/
>
>
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel