[vlc-devel] [PATCH] picture_t: add the possibility to copy pixels another way than through planes

[Steve Lhomme]

On Mon, Apr 20, 2015 at 3:55 PM, Rémi Denis-Courmont <remi at remlab.net> wrote:
> Le 2015-04-20 16:27, Steve Lhomme a écrit :
>>
>> --
>> in the case of D3D9 surfaces, we copy pixels between the internal GPU
>> surfaces
>> ---
>>  include/vlc_picture.h | 1 +
>>  src/misc/picture.c    | 3 +++
>>  2 files changed, 4 insertions(+)
>>
>> diff --git a/include/vlc_picture.h b/include/vlc_picture.h
>> index d3e3b99..393f17f 100644
>> --- a/include/vlc_picture.h
>> +++ b/include/vlc_picture.h
>> @@ -98,6 +98,7 @@ struct picture_t
>>      /** Private data - the video output plugin might want to put
>> stuff here to
>>       * keep track of the picture */
>>      picture_sys_t * p_sys;
>> +    void (*pf_copy_private)( picture_t *p_dst, picture_t *p_src );
>>
>>      /** This way the picture_Release can be overloaded */
>>      struct
>> diff --git a/src/misc/picture.c b/src/misc/picture.c
>> index 1785d3d..8064346 100644
>> --- a/src/misc/picture.c
>> +++ b/src/misc/picture.c
>> @@ -360,6 +360,7 @@ void picture_CopyProperties( picture_t *p_dst,
>> const picture_t *p_src )
>>      p_dst->b_progressive = p_src->b_progressive;
>>      p_dst->i_nb_fields = p_src->i_nb_fields;
>>      p_dst->b_top_field_first = p_src->b_top_field_first;
>> +    p_dst->pf_copy_private = p_src->pf_copy_private;
>
>
> You can't know how the source and destination picture have been allocated
> the same way. So you do not know the meaning of the p_sys/gc.p_sys, nor the
> context. Thus, the generic parameters and the planes are the only thing that
> you can conceivably copy. So I don't see a point.

Pixel copying only occurs with picture_t having the same pixel format
and dimensions. If the pixel format is the same, then the copy method
should be compatible.

But you're right they copy callback might be propagated when the
properties are copied without actually copying the pixels. I'll check
if, in my particular case, this is needed.

> Also copying callbacks is dubious w.r.t. plugin instance life cycle.

Agreed. In my case the callback is set by the decoder and copied along
as picture data is copied. If the decoder is unloaded before the vout
or a filter that may crash.

As mentioned in the other thread, picture_t and picture_pool_t assume
too much about planes and memory being coming from memory. There could
be a "format handler" kind of module. Decoder/vout/filter modules who
use a certain pixel format load/release when they are set to handle
that format. That could also handle how pools are
initialized/allocated.