[vlc-devel] [PATCH 03/25] image: use es_format_Copy() instead of es_format_Init() where possible

[Steve Lhomme]

On Mon, Jul 10, 2017 at 5:09 PM, Francois Cartegnie <fcvlcdev at free.fr> wrote:
> Le 10/07/2017 à 16:52, Steve Lhomme a écrit :
>> We don't want to lose the palette. And if there's one there was potentially
>> a double free when calling es_format_Clean() on the copied structure.
>> ---
>>  src/misc/image.c | 9 +++------
>>  1 file changed, 3 insertions(+), 6 deletions(-)
>>
>> diff --git a/src/misc/image.c b/src/misc/image.c
>> index 20582ef1d6..c505b5bfcc 100644
>> --- a/src/misc/image.c
>> +++ b/src/misc/image.c
>> @@ -646,9 +646,8 @@ static decoder_t *CreateDecoder( vlc_object_t *p_this, video_format_t *fmt )
>>          return NULL;
>>
>>      p_dec->p_module = NULL;
>> -    es_format_Init( &p_dec->fmt_in, VIDEO_ES, fmt->i_chroma );
>> +    es_format_Copy( &p_dec->fmt_in, fmt );
>>      es_format_Init( &p_dec->fmt_out, VIDEO_ES, 0 );
>> -    p_dec->fmt_in.video = *fmt;
>>      p_dec->b_frame_drop_allowed = false;
>
> Init() should always be called before Copy(), even if that does nothing.
> I don't see the point of using an API if we need to be aware of what it
> does internally.
>
> Without Init -> Copy -> Clean cycles, we still write copy() internals
> aware code and will have the same issues if something changes in struct
> or code again.

I agree it's a mess.

> Defining an InitFrom() which is aware of and just does Copy() also an
> alternative.

I think that's what we should do. Given es_format_Copy() is a VLC_API
I don't know if we can remove/rename it. But es_format_InitFrom()
which does what copy does (copy the structures and pointed buffers)
would be clearer. For now we could just use a define or static inline
for that.

> Francois
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel