[vlc-devel] [PATCH 2/2] vout: spu: remove useless picture allocation

[Steve Lhomme]

On 2019-06-05 11:15, Steve Lhomme wrote:
> On 2019-06-05 10:30, Thomas Guillem wrote:
>>
>> On Wed, Jun 5, 2019, at 10:25, Steve Lhomme wrote:
>>> It seems that this hack is done in case region_fmt doesn't match the
>>> region_picture format.
>>>
>>> I'm not sure changing this will not create side effects.
>>
>> Where could be the possible side effect ? The picture is always 
>> destroyed just after being allocated.
>> I dind't change the behavior of subpicture_region_New(), that is used 
>> by most modules and that will continue to allocate a picture if needed.
> 
> As the code after shows, the region has its own dimensions, offset, etc. 
> They probably come from region_fmt and are not assumed to be the same as 
> the underlying picture. This is ugly, but that's the state of the code 
> right now.

But then we don't use the picture with that dimension at all, so it 
should be fine.

>>>
>>> On 2019-06-05 9:54, Thomas Guillem wrote:
>>>> ---
>>>>    src/video_output/vout_subpictures.c | 5 ++---
>>>>    1 file changed, 2 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/src/video_output/vout_subpictures.c 
>>>> b/src/video_output/vout_subpictures.c
>>>> index 0cc102136d..bd07493ce1 100644
>>>> --- a/src/video_output/vout_subpictures.c
>>>> +++ b/src/video_output/vout_subpictures.c
>>>> @@ -1012,13 +1012,12 @@ static void SpuRenderRegion(spu_t *spu,
>>>>            }
>>>>        }
>>>> -    subpicture_region_t *dst = *dst_ptr = 
>>>> subpicture_region_New(&region_fmt);
>>>> +    subpicture_region_t *dst = *dst_ptr = 
>>>> subpicture_region_NewInternal(&region_fmt);
>>>>        if (dst) {
>>>>            dst->i_x       = x_offset;
>>>>            dst->i_y       = y_offset;
>>>>            dst->i_align   = 0;
>>>> -        if (dst->p_picture)
>>>> -            picture_Release(dst->p_picture);
>>>> +        assert(!dst->p_picture);
>>>>            dst->p_picture = picture_Hold(region_picture);
>>>>            int fade_alpha = 255;
>>>>            if (subpic->b_fade) {
>>>> -- 
>>>> 2.20.1
>>>>
>>>> _______________________________________________
>>>> 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
>> _______________________________________________
>> 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