[FFmpeg-devel] [PATCH] dnn_backend_native_layer_avgpool: Fix invalid assignment, use av_assert

[Andreas Rheinhardt]

Guo, Yejun:
> 
> 
>> -----Original Message-----
>> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
>> Andreas Rheinhardt
>> Sent: 2020年8月21日 22:21
>> To: ffmpeg-devel at ffmpeg.org
>> Subject: Re: [FFmpeg-devel] [PATCH] dnn_backend_native_layer_avgpool: Fix
>> invalid assignment, use av_assert
>>
>> Guo, Yejun:
>>>
>>>
>>>> -----Original Message-----
>>>> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
>>>> Andreas Rheinhardt
>>>> Sent: 2020年8月21日 19:47
>>>> To: ffmpeg-devel at ffmpeg.org
>>>> Cc: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
>>>> Subject: [FFmpeg-devel] [PATCH] dnn_backend_native_layer_avgpool: Fix
>>>> invalid assignment, use av_assert
>>>>
>>>> dnn_execute_layer_avg_pool() contains the following line:
>>>>
>>>> assert(avgpool_params->padding_method = VALID);
>>>>
>>>> This statement contains an assignment where obviously a comparison
>>>> was intended. Furthermore, *avgpool_params is const, so that the
>>>> attempted assignment leads to a compilation failure if asserts are
>>>> enabled (i.e. if DEBUG is defined which leads libavutil/internal.h to
>>>> not define NDEBUG). Moreover, the enumeration constant VALID actually
>>>> has the value 0, so that the assert would be triggered if a compiler
>>>> compiles this with asserts enabled. Finally, the statement uses assert()
>> directly instead of av_assert*().
>>>>
>>>> All these errors have been fixed.
>>>>
>>>> Thanks to ubitux for providing a FATE-box [1] where DEBUG is defined.
>>>>
>>>> [1]:
>>>> http://fate.ffmpeg.org/history.cgi?slot=x86_64-archlinux-gcc-ddebug
>>>>
>>>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
>>>> ---
>>>> I was unsure which assert level to use and therefore simply opted for 0.
>>>>
>>>>  libavfilter/dnn/dnn_backend_native_layer_avgpool.c | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/libavfilter/dnn/dnn_backend_native_layer_avgpool.c
>>>> b/libavfilter/dnn/dnn_backend_native_layer_avgpool.c
>>>> index d745c35b4a..8d4d8db98c 100644
>>>> --- a/libavfilter/dnn/dnn_backend_native_layer_avgpool.c
>>>> +++ b/libavfilter/dnn/dnn_backend_native_layer_avgpool.c
>>>> @@ -91,7 +91,7 @@ int dnn_execute_layer_avg_pool(DnnOperand
>>>> *operands, const int32_t *input_operan
>>>>          output_height = ceil(height / (kernel_strides * 1.0));
>>>>          output_width = ceil(width / (kernel_strides * 1.0));
>>>>      } else {
>>>> -        assert(avgpool_params->padding_method = VALID);
>>>> +        av_assert0(avgpool_params->padding_method == VALID);
>>>>          height_end = height - avgpool_params->kernel_size + 1;
>>>>          width_end = width - avgpool_params->kernel_size + 1;
>>>>          height_radius = 0;
>>>
>>> thanks for the patch, will push now.
>>
>> I actually push my patches myself. Too late now.
>>
> 
> sure, no problem for the push. 
> 
> Just found that my push command works, so I checked the repo at 
> https://git.ffmpeg.org/gitweb/ffmpeg.git/commit/128e6df1cd79076e3d5f51bbc88607b3d1c62689,
> looks that your push does not work for some reason. just fyi.

I had no problem with my push; it's just that you had already pushed
before I realized your approval and then it was of course too late (so I
never attempted to push this). But as has already been said: Too late now.

- Andreas