[vlc-devel] [PATCH] transform: inline error handling

[Steve Lhomme]

On 2021-01-08 10:34, Thomas Guillem wrote:
> Could we decide to always to the objres clean when destroying a vlc_object ?
> 
> I don't use this new vlc_obj_*alloc feature because I never know if the resource will be cleaned or not. There are still some module types without any clean.

Yes, that was my main concern as well when I saw the patch. I also 
always wonder wether I'm supposed to clean on error in the activate 
callback. If we can have a rule and stick to it, that would be helpful.

> On Fri, Jan 8, 2021, at 10:30, Alexandre Janniaux wrote:
>> Hi,
>>
>> On Fri, Jan 08, 2021 at 10:19:38AM +0100, Steve Lhomme wrote:
>>> On 2021-01-08 10:16, Alexandre Janniaux wrote:
>>>> Hi,
>>>>
>>>> On Fri, Jan 08, 2021 at 07:40:11AM +0100, Steve Lhomme wrote:
>>>>> On 2021-01-07 16:51, Alexandre Janniaux wrote:
>>>>>> ...and remove useless error label. The resources are released by the
>>>>>> vlc_objres mechanism already.
>>>>>
>>>>> Did you check all possible ways to create filters do release resources on
>>>>> Open error ? If so this should be done in other filters as well.
>>>>
>>>> I don't understand, this patch is local to the transform filter.
>>>> I don't change anything outside of it, and have checked that the
>>>> objres release is actually done (which it is, through module_need).
>>>
>>> So you understand that the key for this patch to work is that module_need
>>> calls do the cleaning.
>>
>> Ok, so I guess this is correct then.
>>
>>>> Regards,
>>>> --
>>>> Alexandre Janniaux
>>>> Videolabs
>>>>
>>>>>> ---
>>>>>>     modules/video_filter/transform.c | 13 +++++--------
>>>>>>     1 file changed, 5 insertions(+), 8 deletions(-)
>>>>>>
>>>>>> diff --git a/modules/video_filter/transform.c b/modules/video_filter/transform.c
>>>>>> index 473bd4f9fa..96397d0f7a 100644
>>>>>> --- a/modules/video_filter/transform.c
>>>>>> +++ b/modules/video_filter/transform.c
>>>>>> @@ -348,7 +348,7 @@ static int Open(filter_t *filter)
>>>>>>             default:
>>>>>>                 msg_Err(filter, "Unsupported pixel size %u (chroma %4.4s)",
>>>>>>                         chroma->pixel_size, (char *)&src->i_chroma);
>>>>>> -            goto error;
>>>>>> +            return VLC_EGENERIC;
>>>>>>         }
>>>>>>         for (unsigned i = 1; i < PICTURE_PLANE_MAX; i++)
>>>>>> @@ -367,7 +367,7 @@ static int Open(filter_t *filter)
>>>>>>                          != chroma->p[i].h.num * chroma->p[i].w.den) {
>>>>>>                             msg_Err(filter, "Format rotation not possible "
>>>>>>                                     "(chroma %4.4s)", (char *)&src->i_chroma);
>>>>>> -                        goto error;
>>>>>> +                        return VLC_EGENERIC;
>>>>>>                         }
>>>>>>                 }
>>>>>>             }
>>>>>> @@ -393,7 +393,7 @@ static int Open(filter_t *filter)
>>>>>>              dst->i_y_offset       != src_trans.i_y_offset)) {
>>>>>>                 msg_Err(filter, "Format change is not allowed");
>>>>>> -            goto error;
>>>>>> +            return VLC_EGENERIC;
>>>>>>         }
>>>>>>         else if(filter->b_allow_fmt_out_change) {
>>>>>> @@ -414,7 +414,7 @@ static int Open(filter_t *filter)
>>>>>>                 if (dsc_is_rotated(dsc)) {
>>>>>>                     msg_Err(filter, "Format rotation not possible (chroma %4.4s)",
>>>>>>                             (char *)&src->i_chroma);
>>>>>> -                goto error;
>>>>>> +                return VLC_EGENERIC;
>>>>>>                 }
>>>>>>                 /* fallthrough */
>>>>>>             case VLC_CODEC_YUYV:
>>>>>> @@ -423,7 +423,7 @@ static int Open(filter_t *filter)
>>>>>>                 break;
>>>>>>             case VLC_CODEC_NV12:
>>>>>>             case VLC_CODEC_NV21:
>>>>>> -            goto error;
>>>>>> +            return VLC_EGENERIC;
>>>>>>         }
>>>>>>         static const struct vlc_filter_operations filter_ops =
>>>>>> @@ -434,7 +434,4 @@ static int Open(filter_t *filter)
>>>>>>         filter->ops = &filter_ops;
>>>>>>         filter->p_sys           = sys;
>>>>>>         return VLC_SUCCESS;
>>>>>> -error:
>>>>>> -    vlc_obj_free(VLC_OBJECT(filter), sys);
>>>>>> -    return VLC_EGENERIC;
>>>>>>     }
>>>>>> --
>>>>>> 2.30.0
>>>>>>
>>>>>> _______________________________________________
>>>>>> 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
>> _______________________________________________
>> 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
>