Re: [FFmpeg-devel] [PATCH] avfilter: Added siti filter

[Thilo Borgmann <thilo.borgmann@mail.de>]

Am 28.03.22 um 13:01 schrieb Thilo Borgmann:
> Am 22.03.22 um 09:36 schrieb Thilo Borgmann:
>> Am 18.03.22 um 15:04 schrieb Paul B Mahol:
>>> On 3/18/22, Thilo Borgmann <thilo.borgmann@mail.de> wrote:
>>>>
>>>>
>>>> On 12 Mar 2022, at 10:06, Thilo Borgmann wrote:
>>>>
>>>>> Am 09.03.22 um 18:31 schrieb Paul B Mahol:
>>>>>> On 3/8/22, Thilo Borgmann <thilo.borgmann@mail.de> wrote:
>>>>>>> Am 07.03.22 um 20:06 schrieb Paul B Mahol:
>>>>>>>> On 3/7/22, Thilo Borgmann <thilo.borgmann@mail.de> wrote:
>>>>>>>>> Am 06.03.22 um 22:25 schrieb Paul B Mahol:
>>>>>>>>>> On 3/6/22, Thilo Borgmann <thilo.borgmann@mail.de> wrote:
>>>>>>>>>>> Am 22.02.22 um 12:30 schrieb Thilo Borgmann:
>>>>>>>>>>>> Am 18.02.22 um 17:08 schrieb Paul B Mahol:
>>>>>>>>>>>>> On Sat, Feb 12, 2022 at 11:55 AM Thilo Borgmann
>>>>>>>>>>>>> <thilo.borgmann@mail.de>
>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>>> Am 31.01.22 um 12:55 schrieb James Almer:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> On 1/31/2022 8:53 AM, Anton Khirnov wrote:
>>>>>>>>>>>>>>>> Quoting Thilo Borgmann (2022-01-18 14:58:07)
>>>>>>>>>>>>>>>>>>> Violations of code style.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Enhanced.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Not enough. There are still many remaining, e.g.
>>>>>>>>>>>>>>>> * opening brace of a function definition should be on its own
>>>>>>>>>>>>>>>> line
>>>>>>>>>>>>>>>> * the context should generally be the first argument
>>>>>>>>>>>>>>>> * unsigned char* should be uint8_t*
>>>>>>>>>>>>>>>> * mixed declarations and code (the compiler should warn about
>>>>>>>>>>>>>>>> that)
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> I think someone said that clang (or some versions) is
>>>>>>>>>>>>>>> apparently
>>>>>>>>>>>>>>> not
>>>>>>>>>>>>>> warning about this, hence why so many of these end up being
>>>>>>>>>>>>>> missed
>>>>>>>>>>>>>> in
>>>>>>>>>>>>>> reviews or even by the patch author.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> This and all of Anton's comments in v3. Also removed some more
>>>>>>>>>>>>>> obviously
>>>>>>>>>>>>>> useless doubles.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> Why it uses doubles in so many places?
>>>>>>>>>>>>> Is there any real benefit in that, except extra slowdown?
>>>>>>>>>>>>
>>>>>>>>>>>> I guess because it's originating in some c&p Matlab code.
>>>>>>>>>>>> I did %s#double#float#g for v4, loosing some precision we can
>>>>>>>>>>>> ignore
>>>>>>>>>>>> IMHO.
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> v3:
>>>>>>>>>>>>
>>>>>>>>>>>> Total frames: 2
>>>>>>>>>>>>
>>>>>>>>>>>> Spatial Information:
>>>>>>>>>>>> Average: 165.451985
>>>>>>>>>>>> Max: 165.817542
>>>>>>>>>>>> Min: 165.086427
>>>>>>>>>>>>
>>>>>>>>>>>> Temporal Information:
>>>>>>>>>>>> Average: 1.007263
>>>>>>>>>>>> Max: 2.014525
>>>>>>>>>>>> Min: 0.000000
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> v4:
>>>>>>>>>>>>
>>>>>>>>>>>> Total frames: 2
>>>>>>>>>>>>
>>>>>>>>>>>> Spatial Information:
>>>>>>>>>>>> Average: 164.385895
>>>>>>>>>>>> Max: 164.742325
>>>>>>>>>>>> Min: 164.029480
>>>>>>>>>>>>
>>>>>>>>>>>> Temporal Information:
>>>>>>>>>>>> Average: 1.007241
>>>>>>>>>>>> Max: 2.014483
>>>>>>>>>>>> Min: 0.000000
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Ping.
>>>>>>>>>>
>>>>>>>>>> Into wrong section of changelog added entry.
>>>>>>>>>>
>>>>>>>>>> Useless cast of allocation results.
>>>>>>>>>>
>>>>>>>>>> Does filter changes pixels? If not, add metadata flag to appropriate
>>>>>>>>>> place.
>>>>>>>>>
>>>>>>>>> All addressed in v5, thx!
>>>>>>>>>
>>>>>>>>
>>>>>>>> Changelog entry is still in wrong, 5.0, section.
>>>>>>>
>>>>>>> Fixed in v6.
>>>>>>>
>>>>>>>>> Also added a FATE test for it.
>>>>>>>
>>>>>>>
>>>>>>
>>>>>> Could use fminf/ float functions instead of double variants.
>>>>>
>>>>> v7.
>>>>
>>>> Going to push soon if there are no more comments.
>>>
>>> Check that returned values are correct for bigger w/h, and that not
>>> values reach too high values for floats
>>> which may cause loss of precision in best case, eg. maybe you need to
>>> normalize pixel values from 0-255 to 0.f-1.f so mean/stddev  does not
>>> get bad results.
>>
>> Did the accumulators as doubles then, good?
>>
>> Also found another missing fmaxf(). V8 attached.
> 
> Ping.

Will apply soon.

-Thilo
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".