<div dir="auto"><div>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.<div dir="auto"><br></div><div dir="auto">Sometimes this can be easier or more comfortably/readably expressed as a factory pattern though.<br><br><div data-smartmail="gmail_signature" dir="auto">-- Sean McGovern</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri., Jul. 31, 2020, 01:09 Steve Lhomme, <<a href="mailto:robux4@ycbcr.xyz">robux4@ycbcr.xyz</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 2020-07-30 18:10, Rémi Denis-Courmont wrote:<br>
> Le torstaina 30. heinäkuuta 2020, 15.54.29 EEST Alexandre Janniaux a écrit :<br>
>> Hi,<br>
>><br>
>> I'm not at ease with this patchset, it feels like removing the<br>
>> picture_resource_t layer and doing it by hand.<br>
> <br>
> Leaving aside my minor detail comments, I think the patchset makes sense.<br>
> <br>
> The whole point of push-buffers was to uniformise picture buffer allocation.<br>
> Everything should be allocated as shared memory that can be manipulated<br>
> locally, passed onto compositor with zero copy IPC, and eventually exchanged<br>
> with sandboxed decoders orf ilters. picture_NewFromFormat() takes care of<br>
> that. Pictures resources should not be needed anymore.<br>
> <br>
> Regardless we still need a mean to wrap opaque picture buffers, which cannot<br>
> really use picture_NewFromFormat().<br>
> <br>
> However...<br>
> <br>
>> This value previously held vout data in the pull model, if<br>
>> I am not wrong. I still use it in scenario where the vout<br>
>> binds an object (eg. wl_buffer) to an underlying object in<br>
>> the picture (eg. a GBM buffer).<br>
> <br>
> If we need to handle non-opaque pictures coming from an input peripheral (as<br>
> opposed to going to an output peripheral), then we need something like similar<br>
> to picture_NewFromResource(). I guess that is your implication?<br>
<br>
It seems VideoToolbox does that. See cvpxpic_create_mapped(). D3D11 and <br>
D3D9 have something similar as well. It's always to copy to/from CPU <br>
from/to GPU. Just using planes could work. (VT even does the plane <br>
copying manually)<br>
<br>
vmem, kms and fb display modules could also use an approach with just <br>
planes.<br>
<br>
That involves more patching and more testing. But now they are more <br>
visible. It's all the picture_NewFromResource() calls with a NULL gc.<br>
<br>
See branch <a href="https://code.videolan.org/robUx4/vlc/-/commits/picture-clean/2" rel="noreferrer noreferrer" target="_blank">https://code.videolan.org/robUx4/vlc/-/commits/picture-clean/2</a><br>
<br>
> Even if that use case materialises, picture_NewFromResource() cannot handle it<br>
> as it stands: it lacks provision for passing the file descriptor, mapping offset<br>
> and size. So that would need a whole new picture_New*() function in any case.<br>
> <br>
> -- <br>
> 雷米‧德尼-库尔蒙<br>
> <a href="http://www.remlab.net/" rel="noreferrer noreferrer" target="_blank">http://www.remlab.net/</a><br>
> <br>
> <br>
> <br>
> _______________________________________________<br>
> vlc-devel mailing list<br>
> To unsubscribe or modify your subscription options:<br>
> <a href="https://mailman.videolan.org/listinfo/vlc-devel" rel="noreferrer noreferrer" target="_blank">https://mailman.videolan.org/listinfo/vlc-devel</a><br>
> <br>
_______________________________________________<br>
vlc-devel mailing list<br>
To unsubscribe or modify your subscription options:<br>
<a href="https://mailman.videolan.org/listinfo/vlc-devel" rel="noreferrer noreferrer" target="_blank">https://mailman.videolan.org/listinfo/vlc-devel</a></blockquote></div></div></div>