[vlc-devel] [PATCH 00/19] picture cleaning

[Sean McGovern]

My experience for builder patterns (esp. from Java which I tackle on a day
to day basis in my job) has always been as a fix for object constructors
that have far too many arguments, such that any reasonable processor
architecture might have to represent as multiple stack frames, but I can
also agree that there is value in having sane defaults from a
private/protected constructor.

Sometimes this can be easier or more comfortably/readably expressed as a
factory pattern though.

-- Sean McGovern

On Fri., Jul. 31, 2020, 01:09 Steve Lhomme, <robux4 at ycbcr.xyz> wrote:

> On 2020-07-30 18:10, Rémi Denis-Courmont wrote:
> > Le torstaina 30. heinäkuuta 2020, 15.54.29 EEST Alexandre Janniaux a
> écrit :
> >> Hi,
> >>
> >> I'm not at ease with this patchset, it feels like removing the
> >> picture_resource_t layer and doing it by hand.
> >
> > Leaving aside my minor detail comments, I think the patchset makes sense.
> >
> > The whole point of push-buffers was to uniformise picture buffer
> allocation.
> > Everything should be allocated as shared memory that can be manipulated
> > locally, passed onto compositor with zero copy IPC, and eventually
> exchanged
> > with sandboxed decoders orf ilters. picture_NewFromFormat() takes care of
> > that. Pictures resources should not be needed anymore.
> >
> > Regardless we still need a mean to wrap opaque picture buffers, which
> cannot
> > really use picture_NewFromFormat().
> >
> > However...
> >
> >> This value previously held vout data in the pull model, if
> >> I am not wrong. I still use it in scenario where the vout
> >> binds an object (eg. wl_buffer) to an underlying object in
> >> the picture (eg. a GBM buffer).
> >
> > If we need to handle non-opaque pictures coming from an input peripheral
> (as
> > opposed to going to an output peripheral), then we need something like
> similar
> > to picture_NewFromResource(). I guess that is your implication?
>
> It seems VideoToolbox does that. See cvpxpic_create_mapped(). D3D11 and
> D3D9 have something similar as well. It's always to copy to/from CPU
> from/to GPU. Just using planes could work. (VT even does the plane
> copying manually)
>
> vmem, kms and fb display modules could also use an approach with just
> planes.
>
> That involves more patching and more testing. But now they are more
> visible. It's all the picture_NewFromResource() calls with a NULL gc.
>
> See branch https://code.videolan.org/robUx4/vlc/-/commits/picture-clean/2
>
> > Even if that use case materialises, picture_NewFromResource() cannot
> handle it
> > as it stands: it lacks provision for passing the file descriptor,
> mapping offset
> > and size. So that would need a whole new picture_New*() function in any
> case.
> >
> > --
> > 雷米‧德尼-库尔蒙
> > http://www.remlab.net/
> >
> >
> >
> > _______________________________________________
> > 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20200731/2bbc9db1/attachment.html>