From: Thilo Borgmann <thilo.borgmann@mail.de>
To: ffmpeg-devel@ffmpeg.org
Subject: Re: [FFmpeg-devel] [PATCH] avfilter: Added siti filter
Date: Mon, 28 Mar 2022 13:01:58 +0200
Message-ID: <5faf3a41-14c8-3615-160a-e4a00ebdca62@mail.de> (raw)
In-Reply-To: <fd74c914-475c-887a-b76d-e71f02cee0d8@mail.de>
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.
-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".
next prev parent reply other threads:[~2022-03-28 11:02 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20210115045832.76405-1-borbarak@fb.com>
[not found] ` <MN2PR15MB2605F2669A411F794B6C2B40B5A70@MN2PR15MB2605.namprd15.prod.outlook.com>
[not found] ` <MR3WgJJ--3-2@lynne.ee>
[not found] ` <MN2PR15MB2605997AC36969E5639782C4B5A30@MN2PR15MB2605.namprd15.prod.outlook.com>
[not found] ` <MRNyTw0--3-2@lynne.ee>
2022-01-14 16:21 ` Thilo Borgmann
2022-01-15 8:25 ` Paul B Mahol
2022-01-15 8:27 ` Paul B Mahol
2022-01-18 13:58 ` Thilo Borgmann
2022-01-24 11:10 ` Thilo Borgmann
2022-01-31 10:12 ` Thilo Borgmann
2022-01-31 11:53 ` Anton Khirnov
2022-01-31 11:55 ` James Almer
2022-02-12 10:55 ` Thilo Borgmann
2022-02-15 8:54 ` Anton Khirnov
2022-02-15 9:10 ` Paul B Mahol
2022-02-15 9:19 ` Anton Khirnov
2022-02-22 11:26 ` Thilo Borgmann
2022-02-18 16:08 ` Paul B Mahol
2022-02-22 11:30 ` Thilo Borgmann
2022-03-06 20:39 ` Thilo Borgmann
2022-03-06 21:25 ` Paul B Mahol
2022-03-07 17:09 ` Thilo Borgmann
2022-03-07 19:06 ` Paul B Mahol
2022-03-08 12:25 ` Thilo Borgmann
2022-03-09 17:31 ` Paul B Mahol
2022-03-12 9:06 ` Thilo Borgmann
2022-03-18 13:56 ` Thilo Borgmann
2022-03-18 14:04 ` Paul B Mahol
2022-03-22 8:36 ` Thilo Borgmann
2022-03-28 11:01 ` Thilo Borgmann [this message]
2022-04-01 8:58 ` Thilo Borgmann
2022-04-01 18:54 ` Thilo Borgmann
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5faf3a41-14c8-3615-160a-e4a00ebdca62@mail.de \
--to=thilo.borgmann@mail.de \
--cc=ffmpeg-devel@ffmpeg.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
This inbox may be cloned and mirrored by anyone:
git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git
# If you have public-inbox 1.1+ installed, you may
# initialize and index your mirror using the following commands:
public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
ffmpegdev@gitmailbox.com
public-inbox-index ffmpegdev
Example config snippet for mirrors.
AGPL code for this site: git clone https://public-inbox.org/public-inbox.git