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

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

Am 01.04.22 um 10:58 schrieb Thilo Borgmann:
> 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.

Applied, thanks!

-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".