Re: [FFmpeg-devel] [Patch] beautified + accelerated vf_fillborders – Please review
Ulf Zibis (12019-04-03): > In consideration of his in my judgement impolite 1-line comments it > seems unlikely to me that rebasing would be worth the effort. You are right, these comments are completely unacceptable. But that does not mean you should not strive to improve your patches. Regards, -- Nicolas George signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [Patch] beautified + accelerated vf_fillborders – Please review
On 4/3/19, Ulf Zibis wrote: > > Am 03.04.19 um 14:25 schrieb Carl Eugen Hoyos: >>> vf_fillborders_1.patch >> As explained, this patch is not ok, > I would say "determined". > >> There are two possibilities: >> Either you rebase your remaining patchset and wait for a >> review from Paul. > > In consideration of his in my judgement impolite 1-line comments it > seems unlikely to me that rebasing would be worth the effort. > >> Patches are big mess. >> You showed very little skills. >> You obviously lack git skills. > Even your question from 28.03.19, 23:22 CET is still open. > >> Or only send the patch that improves the filter performance. > > I'll consider that when I'm complete with my investigation with tuning. > I also could provide a final patch to rework the remaining indentations > which is much less work. Do not reindent code, it is waste of my and yours time. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [Patch] beautified + accelerated vf_fillborders – Please review
Am 03.04.19 um 14:25 schrieb Carl Eugen Hoyos: >> vf_fillborders_1.patch > As explained, this patch is not ok, I would say "determined". > There are two possibilities: > Either you rebase your remaining patchset and wait for a > review from Paul. In consideration of his in my judgement impolite 1-line comments it seems unlikely to me that rebasing would be worth the effort. > Patches are big mess. > You showed very little skills. > You obviously lack git skills. Even your question from 28.03.19, 23:22 CET is still open. > Or only send the patch that improves the filter performance. I'll consider that when I'm complete with my investigation with tuning. I also could provide a final patch to rework the remaining indentations which is much less work. -Ulf ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [Patch] beautified + accelerated vf_fillborders – Please review
2019-04-03 11:13 GMT+02:00, Ulf Zibis : > vf_fillborders_1.patch As explained, this patch is not ok, therefore the patchset as-is can not be applied. There are two possibilities: Either you rebase your remaining patchset and wait for a review from Paul. Or only send the patch that improves the filter performance. Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [Patch] beautified + accelerated vf_fillborders – Please review
Am 03.04.19 um 11:13 schrieb Ulf Zibis: > At my machine all patches work fine: > > ich@T500:~/Projects/ffmpeg/test$ git clone git://source.ffmpeg.org/ffmpeg . > Klone nach '.' ... > remote: Counting objects: 565208, done. > remote: Compressing objects: 100% (117011/117011), done. > remote: Total 565208 (delta 453761), reused 556701 (delta 447046) > Empfange Objekte: 100% (565208/565208), 100.39 MiB | 1.04 MiB/s, Fertig. > Löse Unterschiede auf: 100% (453761/453761), Fertig. > ich@T500:~/Projects/ffmpeg/test$ git checkout -b vf_fillborders > Zu neuem Branch 'vf_fillborders' gewechselt > [.] > ich@T500:~/Projects/ffmpeg/test$ git am vf_fillborders_1.patch > Wende an: avfilter/fillborders: corrected indentations on > line-continuation to 8 > [.] The similar you can do with the benchmark patches: $ git branch master $ git checkout -b vf_fillbd_benchmark $ git am vf_fillbd_benchmark_1.patch $ git am vf_fillbd_benchmark_2.patch [.] -Ulf ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [Patch] beautified + accelerated vf_fillborders – Please review
On 4/3/19, Ulf Zibis wrote: > > Am 03.04.19 um 00:32 schrieb Carl Eugen Hoyos: >>> So please throw away the old one and use the new >>> patch 11. >> That patch does not apply: > At my machine all patches work fine: > > ich@T500:~/Projects/ffmpeg/test$ git clone git://source.ffmpeg.org/ffmpeg . > Klone nach '.' ... > remote: Counting objects: 565208, done. > remote: Compressing objects: 100% (117011/117011), done. > remote: Total 565208 (delta 453761), reused 556701 (delta 447046) > Empfange Objekte: 100% (565208/565208), 100.39 MiB | 1.04 MiB/s, Fertig. > Löse Unterschiede auf: 100% (453761/453761), Fertig. > ich@T500:~/Projects/ffmpeg/test$ git checkout -b vf_fillborders > Zu neuem Branch 'vf_fillborders' gewechselt > ich@T500:~/Projects/ffmpeg/test$ git status > Auf Branch vf_fillborders > Unversionierte Dateien: > (benutzen Sie "git add ...", um die Änderungen zum Commit > vorzumerken) > > vf_fillborders_1.patch > vf_fillborders_10.patch > vf_fillborders_11.patch > vf_fillborders_12.patch > vf_fillborders_2.patch > vf_fillborders_3.patch > vf_fillborders_4.patch > vf_fillborders_5.patch > vf_fillborders_6.patch > vf_fillborders_7.patch > vf_fillborders_8.patch > vf_fillborders_9.patch > > nichts zum Commit vorgemerkt, aber es gibt unversionierte Dateien > (benutzen Sie "git add" zum Versionieren) > ich@T500:~/Projects/ffmpeg/test$ git am vf_fillborders_1.patch > Wende an: avfilter/fillborders: corrected indentations on > line-continuation to 8 > ich@T500:~/Projects/ffmpeg/test$ git am vf_fillborders_2.patch > Wende an: avfilter/fillborders: added comments; removed separation in > commented blocks > ich@T500:~/Projects/ffmpeg/test$ git am vf_fillborders_3.patch > Wende an: avfilter/fillborders: named more descriptive > ich@T500:~/Projects/ffmpeg/test$ git am vf_fillborders_4.patch > Wende an: avfilter/fillborders: moved fillborders_options[] more up, > needed for STOP_TIMER(testcase); > ich@T500:~/Projects/ffmpeg/test$ git am vf_fillborders_5.patch > Wende an: avfilter/fillborders: removed obsolete includes > ich@T500:~/Projects/ffmpeg/test$ git am vf_fillborders_6.patch > Wende an: avfilter/fillborders: reduced scope of local variables, also > saves code lines > ich@T500:~/Projects/ffmpeg/test$ git am vf_fillborders_7.patch > Wende an: avfilter/fillborders:renamed config_props; avoid needless > calculations there and order more logical > ich@T500:~/Projects/ffmpeg/test$ git am vf_fillborders_8.patch > Wende an: avfilter/fillborders: aligned pointer to array addressing style > ich@T500:~/Projects/ffmpeg/test$ git am vf_fillborders_9.patch > Wende an: avfilter/fillborders: enhanced readability; > ich@T500:~/Projects/ffmpeg/test$ git am vf_fillborders_10.patch > Wende an: avfilter/fillborders: shortened linesize name to allow more > 1-line code; removed braces > ich@T500:~/Projects/ffmpeg/test$ git am vf_fillborders_11.patch > Wende an: avfilter/fillborders: move definitions to their context, also > to reduce their scope > ich@T500:~/Projects/ffmpeg/test$ git am vf_fillborders_12.patch > Wende an: avfilter/fillborders: moved multiplication with linesize out > of for loop for performance; > ich@T500:~/Projects/ffmpeg/test$ > >> The patch wants to remove "enum" from line 27, but that is an include in >> current FFmpeg. > If you already have applied the old version of vf_fillborders_11.patch > you may first remove it from the repository with: > git reset --hard HEAD~1 > > ... and then apply the new patch again. You obviously lack git skills. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [Patch] beautified + accelerated vf_fillborders – Please review
Am 03.04.19 um 00:32 schrieb Carl Eugen Hoyos: >> So please throw away the old one and use the new >> patch 11. > That patch does not apply: At my machine all patches work fine: ich@T500:~/Projects/ffmpeg/test$ git clone git://source.ffmpeg.org/ffmpeg . Klone nach '.' ... remote: Counting objects: 565208, done. remote: Compressing objects: 100% (117011/117011), done. remote: Total 565208 (delta 453761), reused 556701 (delta 447046) Empfange Objekte: 100% (565208/565208), 100.39 MiB | 1.04 MiB/s, Fertig. Löse Unterschiede auf: 100% (453761/453761), Fertig. ich@T500:~/Projects/ffmpeg/test$ git checkout -b vf_fillborders Zu neuem Branch 'vf_fillborders' gewechselt ich@T500:~/Projects/ffmpeg/test$ git status Auf Branch vf_fillborders Unversionierte Dateien: (benutzen Sie "git add ...", um die Änderungen zum Commit vorzumerken) vf_fillborders_1.patch vf_fillborders_10.patch vf_fillborders_11.patch vf_fillborders_12.patch vf_fillborders_2.patch vf_fillborders_3.patch vf_fillborders_4.patch vf_fillborders_5.patch vf_fillborders_6.patch vf_fillborders_7.patch vf_fillborders_8.patch vf_fillborders_9.patch nichts zum Commit vorgemerkt, aber es gibt unversionierte Dateien (benutzen Sie "git add" zum Versionieren) ich@T500:~/Projects/ffmpeg/test$ git am vf_fillborders_1.patch Wende an: avfilter/fillborders: corrected indentations on line-continuation to 8 ich@T500:~/Projects/ffmpeg/test$ git am vf_fillborders_2.patch Wende an: avfilter/fillborders: added comments; removed separation in commented blocks ich@T500:~/Projects/ffmpeg/test$ git am vf_fillborders_3.patch Wende an: avfilter/fillborders: named more descriptive ich@T500:~/Projects/ffmpeg/test$ git am vf_fillborders_4.patch Wende an: avfilter/fillborders: moved fillborders_options[] more up, needed for STOP_TIMER(testcase); ich@T500:~/Projects/ffmpeg/test$ git am vf_fillborders_5.patch Wende an: avfilter/fillborders: removed obsolete includes ich@T500:~/Projects/ffmpeg/test$ git am vf_fillborders_6.patch Wende an: avfilter/fillborders: reduced scope of local variables, also saves code lines ich@T500:~/Projects/ffmpeg/test$ git am vf_fillborders_7.patch Wende an: avfilter/fillborders:renamed config_props; avoid needless calculations there and order more logical ich@T500:~/Projects/ffmpeg/test$ git am vf_fillborders_8.patch Wende an: avfilter/fillborders: aligned pointer to array addressing style ich@T500:~/Projects/ffmpeg/test$ git am vf_fillborders_9.patch Wende an: avfilter/fillborders: enhanced readability; ich@T500:~/Projects/ffmpeg/test$ git am vf_fillborders_10.patch Wende an: avfilter/fillborders: shortened linesize name to allow more 1-line code; removed braces ich@T500:~/Projects/ffmpeg/test$ git am vf_fillborders_11.patch Wende an: avfilter/fillborders: move definitions to their context, also to reduce their scope ich@T500:~/Projects/ffmpeg/test$ git am vf_fillborders_12.patch Wende an: avfilter/fillborders: moved multiplication with linesize out of for loop for performance; ich@T500:~/Projects/ffmpeg/test$ > The patch wants to remove "enum" from line 27, but that is an include in > current FFmpeg. If you already have applied the old version of vf_fillborders_11.patch you may first remove it from the repository with: git reset --hard HEAD~1 ... and then apply the new patch again. -Ulf ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [Patch] beautified + accelerated vf_fillborders – Please review
2019-04-03 0:25 GMT+02:00, Ulf Zibis : > So please throw away the old one and use the new > patch 11. That patch does not apply: The patch wants to remove "enum" from line 27, but that is an include in current FFmpeg. Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [Patch] beautified + accelerated vf_fillborders – Please review
Am 02.04.19 um 23:33 schrieb Carl Eugen Hoyos: >> I again could enhance the performance up to 20 %. >> >> Patch 11: Correction of version from 28.03.19 22:01 CET. Fixed compiler >> warning. >> Patch 12: Moved multiplication with linesize out of for loop for >> performance; side effect: reduces footprint again. > Does not apply / patches to change patches are not ok. Sorry, I do not have the original commit anymore, because I have amended the change to it. So I can't provide a revert patch on base of the original patch 11. So please throw away the old one and use the new patch 11. -Ulf ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [Patch] beautified + accelerated vf_fillborders – Please review
2019-04-02 22:26 GMT+02:00, Ulf Zibis : > Hi again, > > Am 28.03.19 um 22:01 schrieb Ulf Zibis: >> As you can see from the benchmark log included in the >> vf_fillbd_benchmark_9.patch I have attained a performance gain up to 45 >> %. >> It is remarkable, that in several cases the processing of 16-bit planes >> is often faster as of 8-bit planes of same image dimension. >> >> Regards, >> >> -Ulf > > I again could enhance the performance up to 20 %. > > Patch 11: Correction of version from 28.03.19 22:01 CET. Fixed compiler > warning. > Patch 12: Moved multiplication with linesize out of for loop for > performance; side effect: reduces footprint again. Does not apply / patches to change patches are not ok. Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [Patch] beautified + accelerated vf_fillborders – Please review
Am 01.04.19 um 22:08 schrieb Carl Eugen Hoyos: >> Can you please tell me more detailed, what is wrong with the indentations? > It’s correct as it is now. You are sure? line 125: there is a double space line 130: the indentation is not aligned with the upper square bracket lines 310..318: the code is hard to read without highlighting the line-continuation by extra indentation All other cases are surly matter of taste, but I prefer unified line-continuation indentation of 8. This has the advantage, that renaming a variable, e.g. "ptr" -> "data", does not entail an additional correction of the indentations each time and all line-continuations are clearly distinguishable from new lines. -Ulf ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [Patch] beautified + accelerated vf_fillborders – Please review
> Am 01.04.2019 um 21:39 schrieb Ulf Zibis : > > Hi Carl Eugen, > > Am 28.03.19 um 22:45 schrieb Carl Eugen Hoyos: >>> Here they are, my new set of patches. >> Patch 1 is wrong. > > Can you please tell me more detailed, what is wrong with the indentations? It’s correct as it is now, please don’t change it. Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [Patch] beautified + accelerated vf_fillborders – Please review
Hi Carl Eugen, Am 28.03.19 um 22:45 schrieb Carl Eugen Hoyos: >> Here they are, my new set of patches. > Patch 1 is wrong. Can you please tell me more detailed, what is wrong with the indentations? -Ulf ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [Patch] beautified + accelerated vf_fillborders – Please review
2019-03-28 23:18 GMT+01:00, Paul B Mahol : > On 3/28/19, Carl Eugen Hoyos wrote: >> 2019-03-28 22:45 GMT+01:00, Carl Eugen Hoyos : >> >>> Patch 1 is wrong. >>> >>> I don't understand the benchmarks >> >> Ok, numer 9 looks like a good idea, either send only this patch or wait >> for another comment. > > Patches are big mess. Until this is fixed I not going to apply this mess. More important is probably if you agree to the variable renaming and code moving or not. If you don't, sending only patch 9 (which, if the benchmarks are correct, indeed shows a large speedup) could be resent. Not sure where you see a "mess" in the latest patchset, it looks quite clean once patch 1 is removed... Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [Patch] beautified + accelerated vf_fillborders – Please review
On 3/28/19, Carl Eugen Hoyos wrote: > 2019-03-28 22:45 GMT+01:00, Carl Eugen Hoyos : > >> Patch 1 is wrong. >> >> I don't understand the benchmarks > > Ok, numer 9 looks like a good idea, either send only this patch or wait > for another comment. Patches are big mess. Until this is fixed I not going to apply this mess. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [Patch] beautified + accelerated vf_fillborders – Please review
2019-03-28 22:45 GMT+01:00, Carl Eugen Hoyos : > Patch 1 is wrong. > > I don't understand the benchmarks Ok, numer 9 looks like a good idea, either send only this patch or wait for another comment. Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [Patch] beautified + accelerated vf_fillborders – Please review
2019-03-28 22:01 GMT+01:00, Ulf Zibis : > Hi again, > > Am 25.03.19 um 12:31 schrieb Ulf Zibis: >>> There are two patches "1", one with wrong indentation. >> I intentionally have provided 2 patches with the same number, one for >> the code base an one with additions for the benchmark. I've catched the >> wrong indentation, hopefully at the place you meant. >> >> I'm preparing a new set of patches to follow your advice. >> >>> Do I read the results correctly that for all patches some cases >>> get faster and others get slower? >> Correct. I'm wondering about the cases, where it gets such slower. So >> I'm interested in an answer from you experienced developers to >> understand this. Maybe a compiler option would help. > > Here they are, my new set of patches. Patch 1 is wrong. I don't understand the benchmarks (actually benchmark) you attached but before sending another patch, you may want to wait for a comment from Paul, he maintains this filter. Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [Patch] beautified + accelerated vf_fillborders – Please review
Am 26.03.19 um 17:12 schrieb Carl Eugen Hoyos: > I was under the impression that we exchanged all > these emails today only because you still hadn't > found a way to measure the performance of your > patch. As I had written, I found a way with "-vf loop=loop=1024:size=1:start=0", but I was curious how I could use the shorter options -loop or -stream_loop from your suggestion of 19.03.19, 17:31 CET. This does not mean, that I unlike your new suggestion with "-f rawvideo ". > I hoped you had already tested the functional correctness. Until this state of changes yes. But it is more convenient in my IDE configuration to have only 1 script for both purposes. Thanks for all your help -Ulf ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [Patch] beautified + accelerated vf_fillborders – Please review
2019-03-26 23:33 GMT+01:00, Ulf Zibis : > > Am 26.03.19 um 16:32 schrieb Carl Eugen Hoyos: Please elaborate. >>> It seems I'm doing something wrong: >>> >>> ich@T500:~/Projects/ffmpeg/dev$ ./ffmpeg-p7b -y -stream_loop 1024 >>> -i /dev/zero -vf fillborders=25:25:25:25:mirror -f null - >> $ ffmpeg -f rawvideo -s hd1080 -i /dev/zero -vf ... -t 1000 -f null - > Thanks. With "-t 1000" it loops endless Unlikely. Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [Patch] beautified + accelerated vf_fillborders – Please review
Am 26.03.19 um 16:32 schrieb Carl Eugen Hoyos: >>> Please elaborate. >> It seems I'm doing something wrong: >> >> ich@T500:~/Projects/ffmpeg/dev$ ./ffmpeg-p7b -y -stream_loop 1024 >> -i /dev/zero -vf fillborders=25:25:25:25:mirror -f null - > $ ffmpeg -f rawvideo -s hd1080 -i /dev/zero -vf ... -t 1000 -f null - Thanks. With "-t 1000" it loops endless, but it works fine with: $ ffmpeg -f rawvideo -s 300x200 -i /dev/zero -vf ... -frames 1024 -f null - -Ulf ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [Patch] beautified + accelerated vf_fillborders – Please review
On 3/26/19, Ulf Zibis wrote: > > Am 26.03.19 um 17:39 schrieb Carl Eugen Hoyos: >> >>> 1.) There may be a shortcut in CPU architecture for copying nulls in >>> series (fillborders.c essentially does that) and more important ... >> I am curious: >> Which architecture are you thinking about that interprets >> FFmpeg's inner structure? > I was inspired of your suspicion. ;-) From Java code I know, that such > things happen as cause of the JIT "just in time compiler" optimization, > don't know, if modern C compilers assemble similar effects. > >>> 2.) Additionally I want to test on different ... >>> - number of planes >>> - color model /resolution >>> - bit depth >> Use the input option -pix_fmt > > Ok, I'll look on that. > > And I'm still curious to read something on my initial question > (following your suggestion from 19.03.19, 17:31 CET to use "-loop"): > ... I ask, because I want to understand the purpose of the shorter > options "-loop number" and "-stream_loop number" (or how to apply them > correctly in the command line to get the wanted effect on single picture > input). If you want to work on ffmpeg, you certainly need to learn how to use. You showed very little skills. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [Patch] beautified + accelerated vf_fillborders – Please review
Am 26.03.19 um 17:39 schrieb Carl Eugen Hoyos: > >> 1.) There may be a shortcut in CPU architecture for copying nulls in >> series (fillborders.c essentially does that) and more important ... > I am curious: > Which architecture are you thinking about that interprets > FFmpeg's inner structure? I was inspired of your suspicion. ;-) From Java code I know, that such things happen as cause of the JIT "just in time compiler" optimization, don't know, if modern C compilers assemble similar effects. >> 2.) Additionally I want to test on different ... >> - number of planes >> - color model /resolution >> - bit depth > Use the input option -pix_fmt Ok, I'll look on that. And I'm still curious to read something on my initial question (following your suggestion from 19.03.19, 17:31 CET to use "-loop"): ... I ask, because I want to understand the purpose of the shorter options "-loop number" and "-stream_loop number" (or how to apply them correctly in the command line to get the wanted effect on single picture input). -Ulf ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [Patch] beautified + accelerated vf_fillborders – Please review
2019-03-26 17:36 GMT+01:00, Ulf Zibis : > > Am 26.03.19 um 17:20 schrieb Carl Eugen Hoyos: >> 2019-03-26 17:17 GMT+01:00, Ulf Zibis : >>> Am 26.03.19 um 16:32 schrieb Carl Eugen Hoyos: 2019-03-26 16:28 GMT+01:00, Ulf Zibis : > Am 26.03.19 um 16:10 schrieb Carl Eugen Hoyos: >> 2019-03-26 15:56 GMT+01:00, Ulf Zibis : >> >>> I'm trying to benchmark -vf fillborders (added the timer >>> code in vf_fillborders.c), so Carl Eugen's suggestion >>> to use /dev/zero as input would not make sense. >> Please elaborate. > It seems I'm doing something wrong: > > ich@T500:~/Projects/ffmpeg/dev$ ./ffmpeg-p7b -y -stream_loop 1024 > -i /dev/zero -vf fillborders=25:25:25:25:mirror -f null - $ ffmpeg -f rawvideo -s hd1080 -i /dev/zero -vf ... -t 1000 -f null - It may be that the performance of the filter cannot be tested like this, I don't know. >>> I suspect, you are right on not suitable for performance test. >> (I did not claim that, on the contrary.) >> Why not? > 1.) There may be a shortcut in CPU architecture for copying nulls in > series (fillborders.c essentially does that) and more important ... I am curious: Which architecture are you thinking about that interprets FFmpeg's inner structure? > 2.) Additionally I want to test on different ... > - number of planes > - color model /resolution > - bit depth Use the input option -pix_fmt Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [Patch] beautified + accelerated vf_fillborders – Please review
Am 26.03.19 um 17:20 schrieb Carl Eugen Hoyos: > 2019-03-26 17:17 GMT+01:00, Ulf Zibis : >> Am 26.03.19 um 16:32 schrieb Carl Eugen Hoyos: >>> 2019-03-26 16:28 GMT+01:00, Ulf Zibis : Am 26.03.19 um 16:10 schrieb Carl Eugen Hoyos: > 2019-03-26 15:56 GMT+01:00, Ulf Zibis : > >> I'm trying to benchmark -vf fillborders (added the timer >> code in vf_fillborders.c), so Carl Eugen's suggestion >> to use /dev/zero as input would not make sense. > Please elaborate. It seems I'm doing something wrong: ich@T500:~/Projects/ffmpeg/dev$ ./ffmpeg-p7b -y -stream_loop 1024 -i /dev/zero -vf fillborders=25:25:25:25:mirror -f null - >>> $ ffmpeg -f rawvideo -s hd1080 -i /dev/zero -vf ... -t 1000 -f null - >>> >>> It may be that the performance of the filter cannot be >>> tested like this, I don't know. >> I suspect, you are right on not suitable for performance test. > (I did not claim that, on the contrary.) > Why not? 1.) There may be a shortcut in CPU architecture for copying nulls in series (fillborders.c essentially does that) and more important ... 2.) Additionally I want to test on different ... - number of planes - color model /resolution - bit depth -Ulf ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [Patch] beautified + accelerated vf_fillborders – Please review
2019-03-26 17:17 GMT+01:00, Ulf Zibis : > > Am 26.03.19 um 16:32 schrieb Carl Eugen Hoyos: >> 2019-03-26 16:28 GMT+01:00, Ulf Zibis : >>> Am 26.03.19 um 16:10 schrieb Carl Eugen Hoyos: 2019-03-26 15:56 GMT+01:00, Ulf Zibis : > I'm trying to benchmark -vf fillborders (added the timer > code in vf_fillborders.c), so Carl Eugen's suggestion > to use /dev/zero as input would not make sense. Please elaborate. >>> It seems I'm doing something wrong: >>> >>> ich@T500:~/Projects/ffmpeg/dev$ ./ffmpeg-p7b -y -stream_loop 1024 >>> -i /dev/zero -vf fillborders=25:25:25:25:mirror -f null - >> $ ffmpeg -f rawvideo -s hd1080 -i /dev/zero -vf ... -t 1000 -f null - >> >> It may be that the performance of the filter cannot be >> tested like this, I don't know. > I suspect, you are right on not suitable for performance test. (I did not claim that, on the contrary.) Why not? Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [Patch] beautified + accelerated vf_fillborders – Please review
Am 26.03.19 um 16:32 schrieb Carl Eugen Hoyos: > 2019-03-26 16:28 GMT+01:00, Ulf Zibis : >> Am 26.03.19 um 16:10 schrieb Carl Eugen Hoyos: >>> 2019-03-26 15:56 GMT+01:00, Ulf Zibis : >>> I'm trying to benchmark -vf fillborders (added the timer code in vf_fillborders.c), so Carl Eugen's suggestion to use /dev/zero as input would not make sense. >>> Please elaborate. >> It seems I'm doing something wrong: >> >> ich@T500:~/Projects/ffmpeg/dev$ ./ffmpeg-p7b -y -stream_loop 1024 >> -i /dev/zero -vf fillborders=25:25:25:25:mirror -f null - > $ ffmpeg -f rawvideo -s hd1080 -i /dev/zero -vf ... -t 1000 -f null - > > It may be that the performance of the filter cannot be > tested like this, I don't know. Thanks for your help! I suspect, you are right on not suitable for performance test. ... and for sure not for algorithmic tests. Unfortunately my initial question is still open: ... but I ask, because I want to understand the purpose of the shorter options "-loop number" and "-stream_loop number" (or how to apply them correctly in the command line to get the wanted effect). -Ulf ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [Patch] beautified + accelerated vf_fillborders – Please review
2019-03-26 17:09 GMT+01:00, Ulf Zibis : > > Am 26.03.19 um 16:34 schrieb Carl Eugen Hoyos: >> 2019-03-26 16:23 GMT+01:00, Ulf Zibis : >>> Am 26.03.19 um 16:00 schrieb Nicolas George: Using the "color" filter source may be a little more efficient, and is much more convenient. >>> With >>> ffplay -f lavfi color=green >>> I only see a monotone picture. This is not apropriate >>> to test the fillborders filter with mode=mirror. >> Why not? > Well, it may be good for the performance test I was under the impression that we exchanged all these emails today only because you still hadn't found a way to measure the performance of your patch. I hoped you had already tested the functional correctness. Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [Patch] beautified + accelerated vf_fillborders – Please review
Am 26.03.19 um 16:34 schrieb Carl Eugen Hoyos: > 2019-03-26 16:23 GMT+01:00, Ulf Zibis : >> Am 26.03.19 um 16:00 schrieb Nicolas George: >>> Using the "color" filter source may be a little more >>> efficient, and is much more convenient. >> With >> ffplay -f lavfi color=green >> I only see a monotone picture. This is not apropriate >> to test the fillborders filter with mode=mirror. > Why not? Well, it may be good for the performance test, but can't test the algorithmic correctness of the tweaked vf_fillborders.c code. Additionally I want to test on different ... - number of planes - color model - bit depth -Ulf ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [Patch] beautified + accelerated vf_fillborders – Please review
Am 26.03.19 um 16:26 schrieb Nicolas George: > > Try testsrc2. Bad news: ich@T500:~/Projects/ffmpeg/dev$ ./ffmpeg-p7b testsrc2 -loop 1024 -vf fillborders=25:25:25:25:mirror -f null - ffmpeg version N-93458-g18429ce896 Copyright (c) 2000-2019 the FFmpeg developers built with gcc 7 (Ubuntu 7.3.0-27ubuntu1~18.04) configuration: libavutil 56. 26.100 / 56. 26.100 libavcodec 58. 47.105 / 58. 47.105 libavformat 58. 26.101 / 58. 26.101 libavdevice 58. 7.100 / 58. 7.100 libavfilter 7. 48.100 / 7. 48.100 libswscale 5. 4.100 / 5. 4.100 libswresample 3. 4.100 / 3. 4.100 [NULL @ 0x5636e78ba900] Unable to find a suitable output format for 'testsrc2' testsrc2: Invalid argument ich@T500:~/Projects/ffmpeg/dev$ ./ffmpeg-p7b -f lavfi testsrc2 -loop 1024 -vf fillborders=25:25:25:25:mirror -f null - ffmpeg version N-93458-g18429ce896 Copyright (c) 2000-2019 the FFmpeg developers built with gcc 7 (Ubuntu 7.3.0-27ubuntu1~18.04) configuration: libavutil 56. 26.100 / 56. 26.100 libavcodec 58. 47.105 / 58. 47.105 libavformat 58. 26.101 / 58. 26.101 libavdevice 58. 7.100 / 58. 7.100 libavfilter 7. 48.100 / 7. 48.100 libswscale 5. 4.100 / 5. 4.100 libswresample 3. 4.100 / 3. 4.100 [NULL @ 0x564b7e1fb940] Requested output format 'lavfi' is not a suitable output format testsrc2: Invalid argument Anyway this test video is not really appropriate for my tests with fillborders filter: 1.) The edges are mostly monotone, so I would hardly see an effect after mirroring the borders 2.) I still want to see the performance on single pictures. -Ulf ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [Patch] beautified + accelerated vf_fillborders – Please review
2019-03-26 16:23 GMT+01:00, Ulf Zibis : > > Am 26.03.19 um 16:00 schrieb Nicolas George: >> Using the "color" filter source may be a little more >> efficient, and is much more convenient. > With > ffplay -f lavfi color=green > I only see a monotone picture. This is not apropriate > to test the fillborders filter with mode=mirror. Why not? Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [Patch] beautified + accelerated vf_fillborders – Please review
2019-03-26 16:28 GMT+01:00, Ulf Zibis : > > Am 26.03.19 um 16:10 schrieb Carl Eugen Hoyos: >> 2019-03-26 15:56 GMT+01:00, Ulf Zibis : >> >>> I'm trying to benchmark -vf fillborders (added the timer >>> code in vf_fillborders.c), so Carl Eugen's suggestion >>> to use /dev/zero as input would not make sense. >> Please elaborate. > > It seems I'm doing something wrong: > > ich@T500:~/Projects/ffmpeg/dev$ ./ffmpeg-p7b -y -stream_loop 1024 > -i /dev/zero -vf fillborders=25:25:25:25:mirror -f null - $ ffmpeg -f rawvideo -s hd1080 -i /dev/zero -vf ... -t 1000 -f null - It may be that the performance of the filter cannot be tested like this, I don't know. Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [Patch] beautified + accelerated vf_fillborders – Please review
Ulf Zibis (12019-03-26): > It seems I'm doing something wrong: > > ich@T500:~/Projects/ffmpeg/dev$ ./ffmpeg-p7b -y -stream_loop 1024 -i > /dev/zero -vf fillborders=25:25:25:25:mirror -f null - Obviously. Please stop putting options randomly together and wasting everybody's time when they do not work. Instead take the necessary time to learn how ffmpeg works. Regards, -- Nicolas George signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [Patch] beautified + accelerated vf_fillborders – Please review
Am 26.03.19 um 16:10 schrieb Carl Eugen Hoyos: > 2019-03-26 15:56 GMT+01:00, Ulf Zibis : > >> I'm trying to benchmark -vf fillborders (added the timer >> code in vf_fillborders.c), so Carl Eugen's suggestion >> to use /dev/zero as input would not make sense. > Please elaborate. It seems I'm doing something wrong: ich@T500:~/Projects/ffmpeg/dev$ ./ffmpeg-p7b -y -stream_loop 1024 -i /dev/zero -vf fillborders=25:25:25:25:mirror -f null - ffmpeg version N-93458-g18429ce896 Copyright (c) 2000-2019 the FFmpeg developers built with gcc 7 (Ubuntu 7.3.0-27ubuntu1~18.04) configuration: libavutil 56. 26.100 / 56. 26.100 libavcodec 58. 47.105 / 58. 47.105 libavformat 58. 26.101 / 58. 26.101 libavdevice 58. 7.100 / 58. 7.100 libavfilter 7. 48.100 / 7. 48.100 libswscale 5. 4.100 / 5. 4.100 libswresample 3. 4.100 / 3. 4.100 /dev/zero: Invalid data found when processing input ich@T500:~/Projects/ffmpeg/dev$ ./ffmpeg-p7b -y -i /dev/zero -loop 1024 -vf fillborders=25:25:25:25:mirror -f null - ffmpeg version N-93458-g18429ce896 Copyright (c) 2000-2019 the FFmpeg developers built with gcc 7 (Ubuntu 7.3.0-27ubuntu1~18.04) configuration: libavutil 56. 26.100 / 56. 26.100 libavcodec 58. 47.105 / 58. 47.105 libavformat 58. 26.101 / 58. 26.101 libavdevice 58. 7.100 / 58. 7.100 libavfilter 7. 48.100 / 7. 48.100 libswscale 5. 4.100 / 5. 4.100 libswresample 3. 4.100 / 3. 4.100 /dev/zero: Invalid data found when processing input -Ulf ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [Patch] beautified + accelerated vf_fillborders – Please review
Ulf Zibis (12019-03-26): > With > ffplay -f lavfi color=green > I only see a monotone picture. This is not apropriate to test the > fillborders filter with mode=mirror. Ok. Then it is not suitable. And neither would be /dev/zero. > Also yuvtestsrc is not really helpfull on that. Try testsrc2. And if it is satisfactory, prepare a rawvideo clip with a lightweight muxer. Regards, -- Nicolas George signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [Patch] beautified + accelerated vf_fillborders – Please review
Am 26.03.19 um 16:00 schrieb Nicolas George: > Using the "color" filter source may be a little more efficient, and is > much more convenient. With ffplay -f lavfi color=green I only see a monotone picture. This is not apropriate to test the fillborders filter with mode=mirror. Also yuvtestsrc is not really helpfull on that. -Ulf ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [Patch] beautified + accelerated vf_fillborders – Please review
2019-03-26 15:56 GMT+01:00, Ulf Zibis : > I'm trying to benchmark -vf fillborders (added the timer > code in vf_fillborders.c), so Carl Eugen's suggestion > to use /dev/zero as input would not make sense. Please elaborate. Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [Patch] beautified + accelerated vf_fillborders – Please review
Ulf Zibis (12019-03-26): > Again only 1 runs (also with "-stream_loop 1024"). You are obviously doing something wrong. Regards, -- Nicolas George signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [Patch] beautified + accelerated vf_fillborders – Please review
Ulf Zibis (12019-03-26): > I'm trying to benchmark -vf fillborders (added the timer code in > vf_fillborders.c), so Carl Eugen's suggestion to use /dev/zero as input > would not make sense. I'll try with "-f null -". Using the "color" filter source may be a little more efficient, and is much more convenient. Regards, -- Nicolas George signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [Patch] beautified + accelerated vf_fillborders – Please review
Am 26.03.19 um 15:56 schrieb Ulf Zibis: > I'm trying to benchmark -vf fillborders (added the timer code in > vf_fillborders.c), so Carl Eugen's suggestion to use /dev/zero as input > would not make sense. I'll try with "-f null -". Again only 1 runs (also with "-stream_loop 1024"). -Ulf ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [Patch] beautified + accelerated vf_fillborders – Please review
Am 26.03.19 um 15:48 schrieb Nicolas George: > Ulf Zibis (12019-03-26): >> Do you mean the following option? Unfortunately I still see only 1 run. >> >> I know, that it works with "-vf -loop=loop=1024:size=1:start=0", but I >> ask, because I want to understand the purpose of the shorter option >> "-loop number". >> >> ./ffmpeg-p7b -y -i debug/8.jpg -loop 1024 -vf >> fillborders=25:25:25:25:mirror debug/ZZ_8_mirror-25-25-25-25.jpg > Are you trying to benchmark the JPEG encoder? If not, do not use the > JPEG encoder, use no encoder at all. > > Are you trying to benchmark the image2 muxer? If not, do not use the > image2 muxer, use no muxer at all. > > Are you trying to benchmark the JPEG decoder? If not, do not use the > JPEG decoder, use the "color" filter source, or, if the test requires > non-trivial content to be relevant, prepare a rawvideo input. Thanks for your hints. I'm trying to benchmark -vf fillborders (added the timer code in vf_fillborders.c), so Carl Eugen's suggestion to use /dev/zero as input would not make sense. I'll try with "-f null -". -Ulf ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [Patch] beautified + accelerated vf_fillborders – Please review
Am 26.03.19 um 15:42 schrieb Ulf Zibis: > Hi again, > > Am 19.03.19 um 17:31 schrieb Carl Eugen Hoyos: >>> 122670 decicycles in fillborders=0:0:5:5:mirror 3p-8bit-1x1, >>> 1 runs, 0 skips >> One run is not good. >> Either use the loop option to filter the same frame again and >> again or feed a video to ffmpeg. > Do you mean the following option? Unfortunately I still see only 1 run. > > I know, that it works with "-vf -loop=loop=1024:size=1:start=0", but I > ask, because I want to understand the purpose of the shorter option > "-loop number". Also "-stream_loop 1024" doesn't work as I would expect. -Ulf ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [Patch] beautified + accelerated vf_fillborders – Please review
Ulf Zibis (12019-03-26): > Do you mean the following option? Unfortunately I still see only 1 run. > > I know, that it works with "-vf -loop=loop=1024:size=1:start=0", but I > ask, because I want to understand the purpose of the shorter option > "-loop number". > > ./ffmpeg-p7b -y -i debug/8.jpg -loop 1024 -vf > fillborders=25:25:25:25:mirror debug/ZZ_8_mirror-25-25-25-25.jpg Are you trying to benchmark the JPEG encoder? If not, do not use the JPEG encoder, use no encoder at all. Are you trying to benchmark the image2 muxer? If not, do not use the image2 muxer, use no muxer at all. Are you trying to benchmark the JPEG decoder? If not, do not use the JPEG decoder, use the "color" filter source, or, if the test requires non-trivial content to be relevant, prepare a rawvideo input. Most of all: use common sense! Regards, -- Nicolas George signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [Patch] beautified + accelerated vf_fillborders – Please review
2019-03-26 15:42 GMT+01:00, Ulf Zibis : > Do you mean the following option? Unfortunately I still see only 1 run. > > I know, that it works with "-vf -loop=loop=1024:size=1:start=0", but I > ask, because I want to understand the purpose of the shorter option > "-loop number". > ./ffmpeg-p7b -y -i debug/8.jpg -loop 1024 -vf loop is an input option, consider using rawvideo (possibly /dev/zero) as input to increase performance (and reduce measurement error). > fillborders=25:25:25:25:mirror debug/ZZ_8_mirror-25-25-25-25.jpg and output to "-f null -". Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [Patch] beautified + accelerated vf_fillborders – Please review
Hi again, Am 19.03.19 um 17:31 schrieb Carl Eugen Hoyos: >> 122670 decicycles in fillborders=0:0:5:5:mirror 3p-8bit-1x1, >> 1 runs, 0 skips > One run is not good. > Either use the loop option to filter the same frame again and > again or feed a video to ffmpeg. Do you mean the following option? Unfortunately I still see only 1 run. I know, that it works with "-vf -loop=loop=1024:size=1:start=0", but I ask, because I want to understand the purpose of the shorter option "-loop number". ./ffmpeg-p7b -y -i debug/8.jpg -loop 1024 -vf fillborders=25:25:25:25:mirror debug/ZZ_8_mirror-25-25-25-25.jpg ffmpeg version N-93458-g18429ce896 Copyright (c) 2000-2019 the FFmpeg developers built with gcc 7 (Ubuntu 7.3.0-27ubuntu1~18.04) configuration: libavutil 56. 26.100 / 56. 26.100 libavcodec 58. 47.105 / 58. 47.105 libavformat 58. 26.101 / 58. 26.101 libavdevice 58. 7.100 / 58. 7.100 libavfilter 7. 48.100 / 7. 48.100 libswscale 5. 4.100 / 5. 4.100 libswresample 3. 4.100 / 3. 4.100 Input #0, image2, from 'debug/8.jpg': Duration: 00:00:00.04, start: 0.00, bitrate: 39119 kb/s Stream #0:0: Video: mjpeg (Lossless), gray(bt470bg/unknown/unknown), 640x480 [SAR 96:96 DAR 4:3], lossless, 25 tbr, 25 tbn, 25 tbc Stream mapping: Stream #0:0 -> #0:0 (mjpeg (native) -> mjpeg (native)) Press [q] to stop, [?] for help [swscaler @ 0x560c9b036400] deprecated pixel format used, make sure you did set range correctly 968130 decicycles in fillborders=25:25:25:25:mirror 1p-8bit-0x0, 1 runs, 0 skips Output #0, image2, to 'debug/ZZ_8_mirror-25-25-25-25.jpg': Metadata: encoder : Lavf58.26.101 Stream #0:0: Video: mjpeg, yuvj444p(pc), 640x480 [SAR 1:1 DAR 4:3], q=2-31, 200 kb/s, 25 fps, 25 tbn, 25 tbc Metadata: encoder : Lavc58.47.105 mjpeg Side data: cpb: bitrate max/min/avg: 0/0/20 buffer size: 0 vbv_delay: -1 frame= 1 fps=0.0 q=6.0 Lsize=N/A time=00:00:00.04 bitrate=N/A speed=1.69x video:34kB audio:0kB subtitle:0kB other streams:0kB global headers:0kB muxing overhead: unknown ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [Patch] beautified + accelerated vf_fillborders – Please review
Am 24.03.19 um 00:40 schrieb Carl Eugen Hoyos: > There are two patches "1", one with wrong indentation. I intentionally have provided 2 patches with the same number, one for the code base an one with additions for the benchmark. I've catched the wrong indentation, hopefully at the place you meant. > Iiuc, the patch mixes the following: > Renaming of a variable > Moving of blocks > Adding commented code that is removed in a later patch > Replacing constants with harder to read calculations This is an argument. There are several places in the code using "* 2". At some places this is an algorithmic purpose, at others to serve the 16-bit logic. I wanted to set this in clear view. > Adding comments > > I don't maintain the file but in general not all of these are > acceptable and it any case should be separate changes. > > This also removes outcommented code which should > not be part of a performance enhancement. > How much "slight enhancement" were you able to measure? > Iiuc, some cases get slower, no? > Some people prefer that patches like this do not change the > indentation to make it more readable. > > Again: Please do not mix functional and cosmetic changes. I'm preparing a new set of patches to follow your advice. > Do I read the results correctly that for all patches some cases > get faster and others get slower? Correct. I'm wondering about the cases, where it gets such slower. So I'm interested in an answer from you experienced developers to understand this. Maybe a compiler option would help. -Ulf ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [Patch] beautified + accelerated vf_fillborders – Please review
Hi again, Am 19.03.19 um 21:44 schrieb Ulf Zibis: > Am 19.03.19 um 17:31 schrieb Carl Eugen Hoyos: >>> 122670 decicycles in fillborders=0:0:5:5:mirror 3p-8bit-1x1, >>> 1 runs, 0 skips >> One run is not good. >> Either use the loop option to filter the same frame again and >> again or feed a video to ffmpeg. > With: > ./ffmpeg -y -v error -stream_loop 100 -i input.jpg -vf > fillborders=5:5:5:5:mirror -f null - > I still see only 1 run. What I'm doing wrong? As I was not able to find a loop option I used -stream_loop. Now I'm wondering, that it doesn't work as I expect. Do I misinterpret the purpose of -stream_loop? -Ulf ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [Patch] beautified + accelerated vf_fillborders – Please review
2019-03-24 0:13 GMT+01:00, Ulf Zibis : > Hi again, > > Am 19.03.19 um 17:31 schrieb Carl Eugen Hoyos: >> One run is not good. >> Either use the loop option to filter the same frame again and >> again or feed a video to ffmpeg. > > I have new patches. > > Patch 1 is just a little renaming and a preparation for the > benchmark timer code. There are two patches "1", one with wrong indentation. Iiuc, the patch mixes the following: Renaming of a variable Moving of blocks Adding commented code that is removed in a later patch Replacing constants with harder to read calculations Adding comments I don't maintain the file but in general not all of these are acceptable and it any case should be separate changes. > Patch 2 is a slight enhancement in performance for cases, > where only top and bottom borders are filled. This also removes outcommented code which should not be part of a performance enhancement. How much "slight enhancement" were you able to measure? Iiuc, some cases get slower, no? Some people prefer that patches like this do not change the indentation to make it more readable. > Patch 3 beautifies the code an really enhances the performance. Again: Please do not mix functional and cosmetic changes. > See the results included in the benchmark patch Do I read the results correctly that for all patches some cases get faster and others get slower? Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [Patch] beautified + accelerated vf_fillborders – Please review
Hi again, Am 19.03.19 um 17:31 schrieb Carl Eugen Hoyos: > One run is not good. > Either use the loop option to filter the same frame again and > again or feed a video to ffmpeg. I have new patches. Patch 1 is just a little renaming and a preparation for the benchmark timer code. Patch 2 is a slight enhancement in performance for cases, where only top and bottom borders are filled. Patch 3 beautifies the code an really enhances the performance. See the results included in the benchmark patch -Ulf >From d78d496c0ed7ba83bf113d3f9cf5d68ce62ce4eb Mon Sep 17 00:00:00 2001 From: Ulf Zibis Date: 14.03.2019, 19:34:03 avfilter/fillborders: added comments; named more descriptive; corrected indentations; moved fillborders_options[] more up, needed for STOP_TIMER(testcase); diff --git a/libavfilter/vf_fillborders.c b/libavfilter/vf_fillborders.c index 1344587..ce768d4 100644 --- a/libavfilter/vf_fillborders.c +++ b/libavfilter/vf_fillborders.c @@ -19,14 +19,16 @@ */ #include "libavutil/colorspace.h" -#include "libavutil/common.h" #include "libavutil/opt.h" #include "libavutil/pixdesc.h" -#include "avfilter.h" #include "drawutils.h" -#include "formats.h" #include "internal.h" +/* +#include "libavutil/common.h" +#include "avfilter.h" +#include "formats.h" #include "video.h" +*/ enum { Y, U, V, A }; enum { R, G, B }; @@ -53,6 +55,22 @@ void (*fillborders)(struct FillBordersContext *s, AVFrame *frame); } FillBordersContext; + +#define OFFSET(x) offsetof(FillBordersContext, x) +#define FLAGS AV_OPT_FLAG_VIDEO_PARAM|AV_OPT_FLAG_FILTERING_PARAM + +static const AVOption fillborders_options[] = { +{ "left", "set the left fill border", OFFSET(left), AV_OPT_TYPE_INT, {.i64=0}, 0, INT_MAX,FLAGS }, +{ "right", "set the right fill border", OFFSET(right), AV_OPT_TYPE_INT, {.i64=0}, 0, INT_MAX,FLAGS }, +{ "top","set the top fill border",OFFSET(top),AV_OPT_TYPE_INT, {.i64=0}, 0, INT_MAX,FLAGS }, +{ "bottom", "set the bottom fill border", OFFSET(bottom), AV_OPT_TYPE_INT, {.i64=0}, 0, INT_MAX,FLAGS }, +{ "mode", "set the fill borders mode", OFFSET(mode), AV_OPT_TYPE_INT, {.i64=FM_SMEAR}, 0, FM_NB_MODES-1, FLAGS, "mode" }, +{ "smear", NULL, 0, AV_OPT_TYPE_CONST, {.i64=FM_SMEAR}, 0, 0, FLAGS, "mode" }, +{ "mirror", NULL, 0, AV_OPT_TYPE_CONST, {.i64=FM_MIRROR}, 0, 0, FLAGS, "mode" }, +{ "fixed", NULL, 0, AV_OPT_TYPE_CONST, {.i64=FM_FIXED}, 0, 0, FLAGS, "mode" }, +{ "color", "set the color for the fixed mode", OFFSET(rgba_color), AV_OPT_TYPE_COLOR, {.str = "black"}, .flags = FLAGS }, +{ NULL } +}; static int query_formats(AVFilterContext *ctx) { @@ -87,27 +105,28 @@ int p, y; for (p = 0; p < s->nb_planes; p++) { -uint8_t *ptr = frame->data[p]; +uint8_t *data = frame->data[p]; int linesize = frame->linesize[p]; +/* fill left and right borders from top to bottom border */ for (y = s->borders[p].top; y < s->planeheight[p] - s->borders[p].bottom; y++) { -memset(ptr + y * linesize, - *(ptr + y * linesize + s->borders[p].left), - s->borders[p].left); -memset(ptr + y * linesize + s->planewidth[p] - s->borders[p].right, - *(ptr + y * linesize + s->planewidth[p] - s->borders[p].right - 1), - s->borders[p].right); +memset(data + y * linesize, +*(data + y * linesize + s->borders[p].left), +s->borders[p].left); +memset(data + y * linesize + s->planewidth[p] - s->borders[p].right, +*(data + y * linesize + s->planewidth[p] - s->borders[p].right - 1), +s->borders[p].right); } +/* fill top and bottom borders */ for (y = 0; y < s->borders[p].top; y++) { -memcpy(ptr + y * linesize, - ptr + s->borders[p].top * linesize, s->planewidth[p]); +memcpy(data + y * linesize, +data + s->borders[p].top * linesize, s->planewidth[p]); } - for (y = s->planeheight[p] - s->borders[p].bottom; y < s->planeheight[p]; y++) { -memcpy(ptr + y * linesize, - ptr + (s->planeheight[p] - s->borders[p].bottom - 1) * linesize, - s->planewidth[p]); +memcpy(data + y * linesize, +data + (s->planeheight[p] - s->borders[p].bottom - 1) * linesize, +s->planewidth[p]); } } } @@ -117,29 +136,29 @@ int p, y, x; for (p = 0; p < s->nb_planes; p++) { -uint16_t *ptr = (uint16_t *)frame->data[p]; -int linesize = frame->linesize[p] / 2; +uint16_t *data = (uint16_t *)frame->data[p]; +int linesize = frame->linesize[p] / sizeof(uint16_t); +/* fill left and right borders from top to bottom border */ for (y = s->borders[p].top; y < s->planehei
Re: [FFmpeg-devel] [Patch] beautified + accelerated vf_fillborders – Please review
Am 19.03.19 um 17:31 schrieb Carl Eugen Hoyos: > This does not look like a command line but to avoid the encoding > time, "-f null -" can be used. > >> 122670 decicycles in fillborders=0:0:5:5:mirror 3p-8bit-1x1, >> 1 runs, 0 skips > One run is not good. > Either use the loop option to filter the same frame again and > again or feed a video to ffmpeg. With: ./ffmpeg -y -v error -stream_loop 100 -i input.jpg -vf fillborders=5:5:5:5:mirror -f null - I still see only 1 run. What I'm doing wrong? -Ulf ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [Patch] beautified + accelerated vf_fillborders – Please review
Am 19.03.19 um 17:31 schrieb Carl Eugen Hoyos: >> $ debug/fillborders.sh >> Test[0] ==> 3-plane 8-bit YUV-colour:CYD_1005.jpg <== >> ./ffmpeg-p1 : CYD_1005.jpg --> ZZ_CYD_1005_mirror-0-0-5-5.jpg > This does not look like a command line The command line is in the script file debug/fillborders.sh. This echo comment line with ./ffmpeg-p1 means that the following runs were done with the build from patch 1 and with ./ffmpeg-2 from patch 2 for comparison. > but to avoid the encoding > time, "-f null -" can be used. You mean this as answer for using the -benchmark option? Thanks for the hint. But the CPU time for the decoding would still be there, which i'm afraid, it will too much overflow the little CPU time for the filter. >> 122670 decicycles in fillborders=0:0:5:5:mirror 3p-8bit-1x1, >> 1 runs, 0 skips > One run is not good. I did 6 runs for each command line pattern by loop in debug/fillborders.sh (included in vf_fillbd_benchmark_2.patch). > Either use the loop option to filter the same frame again and > again or feed a video to ffmpeg. Ok, I'll try this. -Ulf ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [Patch] beautified + accelerated vf_fillborders – Please review
2019-03-19 15:57 GMT+01:00, Ulf Zibis : > $ debug/fillborders.sh > Test[0] ==> 3-plane 8-bit YUV-colour:CYD_1005.jpg <== > ./ffmpeg-p1 : CYD_1005.jpg --> ZZ_CYD_1005_mirror-0-0-5-5.jpg This does not look like a command line but to avoid the encoding time, "-f null -" can be used. > 122670 decicycles in fillborders=0:0:5:5:mirror 3p-8bit-1x1, > 1 runs, 0 skips One run is not good. Either use the loop option to filter the same frame again and again or feed a video to ffmpeg. Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [Patch] beautified + accelerated vf_fillborders – Please review
Hi again, Am 12.03.19 um 00:37 schrieb Carl Eugen Hoyos: > 2019-03-12 0:25 GMT+01:00, Moritz Barsnick : >> Ideally, you use the START_TIMER/STOP_TIMER macros to >> profile the actual functions you changed. (Check this mailing list's >> archives for some examples, and play with the code.) > But this should not be needed if time (the command) and / or > benchmark (the FFmpeg option) show clear improvements. With the benchmark option I can not see the time for the filter, just for the de/encoding, and as I assume, that this filter is much faster than the de/encoding around it, I suspect, the overall time will be helpful. So I have "played" with the START_TIMER/STOP_TIMER macros. Now I'm kind of helpless, as the numbers I get are varying in wide range. It seems, that my changes help a little for e.g. "-vf fillborders:0:0:5:5:mirror". This is what I expected by bypassing the code loops for the right/left borders, when there is nothing to do, but the timer results are "noisy". I attach the patches for the first 2 chunks again, and too the patches for my timed version. Hopefully you have the time to play a little with that and can give me hints, how I could get more reliable numbers. (I just had closed all other applications like Firefox, Transmission etc. before running the benchmarks) -Ulf == $ debug/fillborders.sh Test[0] ==> 3-plane 8-bit YUV-colour: CYD_1005.jpg <== ./ffmpeg-p1 : CYD_1005.jpg --> ZZ_CYD_1005_mirror-0-0-5-5.jpg 122670 decicycles in fillborders=0:0:5:5:mirror 3p-8bit-1x1, 1 runs, 0 skips 133020 decicycles in fillborders=0:0:5:5:mirror 3p-8bit-1x1, 1 runs, 0 skips 119430 decicycles in fillborders=0:0:5:5:mirror 3p-8bit-1x1, 1 runs, 0 skips 118350 decicycles in fillborders=0:0:5:5:mirror 3p-8bit-1x1, 1 runs, 0 skips 124740 decicycles in fillborders=0:0:5:5:mirror 3p-8bit-1x1, 1 runs, 0 skips 122130 decicycles in fillborders=0:0:5:5:mirror 3p-8bit-1x1, 1 runs, 0 skips ./ffmpeg-p2 : CYD_1005.jpg --> ZZ_CYD_1005_mirror-0-0-5-5.jpg 118800 decicycles in fillborders=0:0:5:5:mirror 3p-8bit-1x1, 1 runs, 0 skips 123840 decicycles in fillborders=0:0:5:5:mirror 3p-8bit-1x1, 1 runs, 0 skips 121500 decicycles in fillborders=0:0:5:5:mirror 3p-8bit-1x1, 1 runs, 0 skips 135090 decicycles in fillborders=0:0:5:5:mirror 3p-8bit-1x1, 1 runs, 0 skips 126270 decicycles in fillborders=0:0:5:5:mirror 3p-8bit-1x1, 1 runs, 0 skips 125730 decicycles in fillborders=0:0:5:5:mirror 3p-8bit-1x1, 1 runs, 0 skips ./ffmpeg-p1 : CYD_1005.jpg --> ZZ_CYD_1005_mirror-5-5-0-0.jpg 557730 decicycles in fillborders=5:5:0:0:mirror 3p-8bit-1x1, 1 runs, 0 skips 614880 decicycles in fillborders=5:5:0:0:mirror 3p-8bit-1x1, 1 runs, 0 skips 598410 decicycles in fillborders=5:5:0:0:mirror 3p-8bit-1x1, 1 runs, 0 skips 545940 decicycles in fillborders=5:5:0:0:mirror 3p-8bit-1x1, 1 runs, 0 skips 591030 decicycles in fillborders=5:5:0:0:mirror 3p-8bit-1x1, 1 runs, 0 skips 566910 decicycles in fillborders=5:5:0:0:mirror 3p-8bit-1x1, 1 runs, 0 skips ./ffmpeg-p2 : CYD_1005.jpg --> ZZ_CYD_1005_mirror-5-5-0-0.jpg 542430 decicycles in fillborders=5:5:0:0:mirror 3p-8bit-1x1, 1 runs, 0 skips 567900 decicycles in fillborders=5:5:0:0:mirror 3p-8bit-1x1, 1 runs, 0 skips 490050 decicycles in fillborders=5:5:0:0:mirror 3p-8bit-1x1, 1 runs, 0 skips 579330 decicycles in fillborders=5:5:0:0:mirror 3p-8bit-1x1, 1 runs, 0 skips 521370 decicycles in fillborders=5:5:0:0:mirror 3p-8bit-1x1, 1 runs, 0 skips 890370 decicycles in fillborders=5:5:0:0:mirror 3p-8bit-1x1, 1 runs, 0 skips ./ffmpeg-p1 : CYD_1005.jpg --> ZZ_CYD_1005_mirror-5-5-5-5.jpg 576540 decicycles in fillborders=5:5:5:5:mirror 3p-8bit-1x1, 1 runs, 0 skips 597060 decicycles in fillborders=5:5:5:5:mirror 3p-8bit-1x1, 1 runs, 0 skips 599940 decicycles in fillborders=5:5:5:5:mirror 3p-8bit-1x1, 1 runs, 0 skips 621900 decicycles in fillborders=5:5:5:5:mirror 3p-8bit-1x1, 1 runs, 0 skips 588870 decicycles in fillborders=5:5:5:5:mirror 3p-8bit-1x1, 1 runs, 0 skips 606600 decicycles in fillborders=5:5:5:5:mirror 3p-8bit-1x1, 1 runs, 0 skips ./ffmpeg-p2 : CYD_1005.jpg --> ZZ_CYD_1005_mirror-5-5-5-5.jpg 522090 decicycles in fillborders=5:5:5:5:mirror 3p-8bit-1x1, 1 runs, 0 skips 655650 decicycles in fillborders=5:5:5:5:mirror 3p-8bit-1x1, 1 runs, 0 skips 609660 decicycles in fillborders=5:5:5:5:mirror 3p-8bit-1x1, 1 runs, 0 skips 600300 decicycles in fillborders=5:5:5:5:mirror 3p-8bit-1x1, 1 runs, 0 skips 561510 decicycles in fillborders=5:5:5:5:mirror 3p-8bit-1x1, 1 runs, 0 skips 630090 decicycles in fillborders=5
Re: [FFmpeg-devel] [Patch] beautified + accelerated vf_fillborders – Please review
On Fri, Mar 15, 2019 at 01:08:33AM +0100, Ulf Zibis wrote: [...] > static void fixed_borders16(FillBordersContext *s, AVFrame *frame) > { > -int p, y, x; > - > -for (p = 0; p < s->nb_planes; p++) { > +for (int p = 0; p < s->nb_planes; p++) { > uint16_t *data = (uint16_t *)frame->data[p]; > -int linesize = frame->linesize[p] / sizeof(uint16_t); > +int lz = frame->linesize[p] / sizeof(uint16_t); > int width = s->planewidth[p]; > -int height = s->planeheight[p]; > +int height = s->planeheight[p] * lz; > int left = s->borders[p].left; > -int right = s->borders[p].right; > -int top = s->borders[p].top; > -int bottom = s->borders[p].bottom; > -uint16_t fill = s->fill[p] << (s->depth - 8); > +int right = width - s->borders[p].right; > +int top = s->borders[p].top * lz; > +int bottom = height - s->borders[p].bottom * lz; > +int fill_sz = MAX(MAX(left, right), top!=0 || height-bottom!=0 ? > width : 0); this would be FFMAX src/libavfilter/vf_fillborders.c: In function ‘fixed_borders16’: src/libavfilter/vf_fillborders.c:239:9: error: implicit declaration of function ‘MAX’ [-Werror=implicit-function-declaration] int fill_sz = MAX(MAX(left, right), top!=0 || height-bottom!=0 ? width : 0); [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Modern terrorism, a quick summary: Need oil, start war with country that has oil, kill hundread thousand in war. Let country fall into chaos, be surprised about raise of fundamantalists. Drop more bombs, kill more people, be surprised about them taking revenge and drop even more bombs and strip your own citizens of their rights and freedoms. to be continued signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [Patch] beautified + accelerated vf_fillborders – Please review
2019-03-15 1:08 GMT+01:00, Ulf Zibis : > Can you give a rating if a performance win could be expected compaired > to the original code from your experienced knowledge without a benchmark? Just post the numbers that your tests produced. Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [Patch] beautified + accelerated vf_fillborders – Please review
Am 11.03.19 um 23:29 schrieb Lou Logan: > Commit message title prefix for filter patches are usually in the form > of: > > avfilter/fillborders: > or > lavfi/fillborders: > > Trailing whitespace. This should always be avoided. > > Use av_malloc. I now have separted the changes into 4 patches, and mergerd your hints. So you can clearly see, which calculations I have skipped or moved out of inner for loops. Can you give a rating if a performance win could be expected compaired to the original code from your experienced knowledge without a benchmark? -Ulf >From c51360f3b4be0dca597190da5c2128b45e9ee31b Mon Sep 17 00:00:00 2001 From: Ulf Zibis Date: 14.03.2019, 19:34:03 avfilter/fillborders: added comments; named more descriptive; corrected indentations; diff --git a/libavfilter/vf_fillborders.c b/libavfilter/vf_fillborders.c index 1344587..820aa2d 100644 --- a/libavfilter/vf_fillborders.c +++ b/libavfilter/vf_fillborders.c @@ -87,26 +87,27 @@ int p, y; for (p = 0; p < s->nb_planes; p++) { -uint8_t *ptr = frame->data[p]; +uint8_t *data = frame->data[p]; int linesize = frame->linesize[p]; +/* fill left and right borders from top to bottom border */ for (y = s->borders[p].top; y < s->planeheight[p] - s->borders[p].bottom; y++) { -memset(ptr + y * linesize, - *(ptr + y * linesize + s->borders[p].left), +memset(data + y * linesize, + *(data + y * linesize + s->borders[p].left), s->borders[p].left); -memset(ptr + y * linesize + s->planewidth[p] - s->borders[p].right, - *(ptr + y * linesize + s->planewidth[p] - s->borders[p].right - 1), +memset(data + y * linesize + s->planewidth[p] - s->borders[p].right, + *(data + y * linesize + s->planewidth[p] - s->borders[p].right - 1), s->borders[p].right); } +/* fill top and bottom borders */ for (y = 0; y < s->borders[p].top; y++) { -memcpy(ptr + y * linesize, - ptr + s->borders[p].top * linesize, s->planewidth[p]); +memcpy(data + y * linesize, + data + s->borders[p].top * linesize, s->planewidth[p]); } - for (y = s->planeheight[p] - s->borders[p].bottom; y < s->planeheight[p]; y++) { -memcpy(ptr + y * linesize, - ptr + (s->planeheight[p] - s->borders[p].bottom - 1) * linesize, +memcpy(data + y * linesize, + data + (s->planeheight[p] - s->borders[p].bottom - 1) * linesize, s->planewidth[p]); } } @@ -117,29 +118,29 @@ int p, y, x; for (p = 0; p < s->nb_planes; p++) { -uint16_t *ptr = (uint16_t *)frame->data[p]; -int linesize = frame->linesize[p] / 2; +uint16_t *data = (uint16_t *)frame->data[p]; +int linesize = frame->linesize[p] / sizeof(uint16_t); +/* fill left and right borders from top to bottom border */ for (y = s->borders[p].top; y < s->planeheight[p] - s->borders[p].bottom; y++) { for (x = 0; x < s->borders[p].left; x++) { -ptr[y * linesize + x] = *(ptr + y * linesize + s->borders[p].left); +data[y * linesize + x] = *(data + y * linesize + s->borders[p].left); } - for (x = 0; x < s->borders[p].right; x++) { -ptr[y * linesize + s->planewidth[p] - s->borders[p].right + x] = - *(ptr + y * linesize + s->planewidth[p] - s->borders[p].right - 1); +data[y * linesize + s->planewidth[p] - s->borders[p].right + x] = + *(data + y * linesize + s->planewidth[p] - s->borders[p].right - 1); } } +/* fill top and bottom borders */ for (y = 0; y < s->borders[p].top; y++) { -memcpy(ptr + y * linesize, - ptr + s->borders[p].top * linesize, s->planewidth[p] * 2); +memcpy(data + y * linesize, + data + s->borders[p].top * linesize, s->planewidth[p] * sizeof(uint16_t)); } - for (y = s->planeheight[p] - s->borders[p].bottom; y < s->planeheight[p]; y++) { -memcpy(ptr + y * linesize, - ptr + (s->planeheight[p] - s->borders[p].bottom - 1) * linesize, - s->planewidth[p] * 2); +memcpy(data + y * linesize, + data + (s->planeheight[p] - s->borders[p].bottom - 1) * linesize, + s->planewidth[p] * sizeof(uint16_t)); } } } @@ -149,29 +150,29 @@ int p, y, x; for (p = 0; p < s->nb_planes; p++) { -uint8_t *ptr = frame->data[p]; +uint8_t *data = frame->data[p]; int linesize = frame->linesize[p]; +/* fill left and right borders from top to bottom border */ for (y = s->borders[p].top; y < s->planeheight[p] - s->b
Re: [FFmpeg-devel] [Patch] beautified + accelerated vf_fillborders – Please review
2019-03-12 0:25 GMT+01:00, Moritz Barsnick : > On Mon, Mar 11, 2019 at 23:23:15 +0100, Ulf Zibis wrote: >> >> I believe, it's faster because of: >> > Please post some numbers if your patch is supposed to >> > speed up the filter. >> >> Hm, I have no clue how to do this. I thing the listed arguments speak >> for themselves. Is there a kind of harness, template or framework for >> this? > > Well, belief is not enough. ;-) In the simplest case, you show command > lines of some examples using the filter (with various inputs, > parameters) and their filtering/encoding performance before and after > the patch. > Ideally, you use the START_TIMER/STOP_TIMER macros to > profile the actual functions you changed. (Check this mailing list's > archives for some examples, and play with the code.) But this should not be needed if time (the command) and / or benchmark (the FFmpeg option) show clear improvements. Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [Patch] beautified + accelerated vf_fillborders – Please review
On Mon, Mar 11, 2019 at 23:23:15 +0100, Ulf Zibis wrote: > >> I believe, it's faster because of: > > Please post some numbers if your patch is supposed to > > speed up the filter. > > Hm, I have no clue how to do this. I thing the listed arguments speak > for themselves. Is there a kind of harness, template or framework for this? Well, belief is not enough. ;-) In the simplest case, you show command lines of some examples using the filter (with various inputs, parameters) and their filtering/encoding performance before and after the patch. Ideally, you use the START_TIMER/STOP_TIMER macros to profile the actual functions you changed. (Check this mailing list's archives for some examples, and play with the code.) Cheers, Moritz ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [Patch] beautified + accelerated vf_fillborders – Please review
On Mon, 11 Mar 2019 23:07:37 +0100 Ulf Zibis wrote: > From 74dda304bf7a0a31873518187438815d08533934 Mon Sep 17 00:00:00 2001 > From: Ulf Zibis > Date: 11.03.2019, 23:04:15 > > Beautified + accelerated Commit message title prefix for filter patches are usually in the form of: avfilter/fillborders: or lavfi/fillborders: The commit message body ideally should have a concise but informative summary that describes the patch. See 'git log' for examples. > diff --git a/libavfilter/vf_fillborders.c b/libavfilter/vf_fillborders.c > index 1344587..a7a074b 100644 > --- a/libavfilter/vf_fillborders.c > +++ b/libavfilter/vf_fillborders.c > @@ -1,5 +1,7 @@ > /* > * Copyright (c) 2017 Paul B Mahol > + * Trailing whitespace. This should always be avoided. > + * Contributors: Ulf Zibis We don't add a "Contributors" line. The git log covers that. [...] > static void fixed_borders16(FillBordersContext *s, AVFrame *frame) > { > -int p, y, x; > +for (int p = 0; p < s->nb_planes; p++) { > +uint16_t *data = (uint16_t *)frame->data[p]; > +int lz = frame->linesize[p] / sizeof(uint16_t); > +int width = s->planewidth[p]; > +int height = s->planeheight[p] * lz; > +int left = s->borders[p].left; > +int right = width - s->borders[p].right; > +int top = s->borders[p].top * lz; > +int bottom = height - s->borders[p].bottom * lz; > +int fill_sz = MAX(MAX(left, right), top!=0 || height-bottom!=0 ? > width : 0); > +uint16_t *fill = malloc(fill_sz * sizeof(uint16_t)); Use av_malloc. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [Patch] beautified + accelerated vf_fillborders – Please review
Am 11.03.19 um 23:08 schrieb Carl Eugen Hoyos: > You are not supposed to mix cosmetic changes > like removing braces or moving variable declarations > with actual code changes. Hm difficult, because the code changes are dependent from different variables. But I'll give it a try to make some separated patches. >> I believe, it's faster because of: > Please post some numbers if your patch is supposed to > speed up the filter. Hm, I have no clue how to do this. I thing the listed arguments speak for themselves. Is there a kind of harness, template or framework for this? Thanks for your first review. -Ulf ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [Patch] beautified + accelerated vf_fillborders – Please review
2019-03-11 22:59 GMT+01:00, Ulf Zibis : > I have made some refactoring to vf_fillborders.c. You are not supposed to mix cosmetic changes like removing braces or moving variable declarations with actual code changes. > My motivation came from using this filter as a template > for a new filter. Refactoring the code was a good exercise > to understand FFmpeg's data models. > > I think, the code is now much better readable and > I believe, it's faster because of: Please post some numbers if your patch is supposed to speed up the filter. Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [Patch] beautified + accelerated vf_fillborders – Please review
Here is attached the "commit" patch, if you like this more. -Ulf Am 11.03.19 um 22:59 schrieb Ulf Zibis: > Hi, > > I have made some refactoring to vf_fillborders.c. > > My motivation came from using this filter as a template for a new > filter. Refactoring the code was a good exercise to understand FFmpeg's > data models. > > I think, the code is now much better readable and I believe, it's faster > because of: > - more use of memset() and memcpy() > - less calculations in the inner for loops > - skip the looping for right/left filling when there is nothing to do > > I would be appreciated, if one would review my proposed changes. > > Hopefully I've used the right format for the patch. Please honour, that > I'm new in FFmpeg development. > > -Ulf > > > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel >From 74dda304bf7a0a31873518187438815d08533934 Mon Sep 17 00:00:00 2001 From: Ulf Zibis Date: 11.03.2019, 23:04:15 Beautified + accelerated diff --git a/libavfilter/vf_fillborders.c b/libavfilter/vf_fillborders.c index 1344587..a7a074b 100644 --- a/libavfilter/vf_fillborders.c +++ b/libavfilter/vf_fillborders.c @@ -1,5 +1,7 @@ /* * Copyright (c) 2017 Paul B Mahol + * + * Contributors: Ulf Zibis * * This file is part of FFmpeg. * @@ -18,6 +20,7 @@ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA */ +#include #include "libavutil/colorspace.h" #include "libavutil/common.h" #include "libavutil/opt.h" @@ -84,187 +87,176 @@ static void smear_borders8(FillBordersContext *s, AVFrame *frame) { -int p, y; +for (int p = 0; p < s->nb_planes; p++) { +uint8_t *data = frame->data[p]; +int lz = frame->linesize[p]; +int width = s->planewidth[p]; +int height = s->planeheight[p] * lz; +int left = s->borders[p].left; +int right = width - s->borders[p].right; +int top = s->borders[p].top * lz; +int bottom = height - s->borders[p].bottom * lz; -for (p = 0; p < s->nb_planes; p++) { -uint8_t *ptr = frame->data[p]; -int linesize = frame->linesize[p]; +/* fill left and right borders from top to bottom border */ +if (left != 0 || right != width) // in case skip for performance +for (int y = top; y < bottom; y += lz) { +memset(data + y, *(data + y + left), left); +memset(data + y + right, *(data + y + right - 1), width - right); +} -for (y = s->borders[p].top; y < s->planeheight[p] - s->borders[p].bottom; y++) { -memset(ptr + y * linesize, - *(ptr + y * linesize + s->borders[p].left), - s->borders[p].left); -memset(ptr + y * linesize + s->planewidth[p] - s->borders[p].right, - *(ptr + y * linesize + s->planewidth[p] - s->borders[p].right - 1), - s->borders[p].right); -} - -for (y = 0; y < s->borders[p].top; y++) { -memcpy(ptr + y * linesize, - ptr + s->borders[p].top * linesize, s->planewidth[p]); -} - -for (y = s->planeheight[p] - s->borders[p].bottom; y < s->planeheight[p]; y++) { -memcpy(ptr + y * linesize, - ptr + (s->planeheight[p] - s->borders[p].bottom - 1) * linesize, - s->planewidth[p]); -} +/* fill top and bottom borders */ +for (uint8_t *y = data + top; y > data; ) +memcpy(y -= lz, data + top, width); +for (uint8_t *y = data + bottom; y < data + height; y += lz) +memcpy(y, data + bottom - lz, width); } } static void smear_borders16(FillBordersContext *s, AVFrame *frame) { -int p, y, x; +for (int p = 0; p < s->nb_planes; p++) { +uint16_t *data = (uint16_t *)frame->data[p]; +int lz = frame->linesize[p] / sizeof(uint16_t); +int width = s->planewidth[p]; +int height = s->planeheight[p] * lz; +int left = s->borders[p].left; +int right = width - s->borders[p].right; +int top = s->borders[p].top * lz; +int bottom = height - s->borders[p].bottom * lz; -for (p = 0; p < s->nb_planes; p++) { -uint16_t *ptr = (uint16_t *)frame->data[p]; -int linesize = frame->linesize[p] / 2; - -for (y = s->borders[p].top; y < s->planeheight[p] - s->borders[p].bottom; y++) { -for (x = 0; x < s->borders[p].left; x++) { -ptr[y * linesize + x] = *(ptr + y * linesize + s->borders[p].left); +/* fill left and right borders from top to bottom border */ +if (left != 0 || right != width) // in case skip for performance +for (int y = top; y < bottom; y += lz) { +for (int x = left; x >= 0; x--) +data[y + x] = data[y + left]; +for (int x = right; x < width; x+
[FFmpeg-devel] [Patch] beautified + accelerated vf_fillborders – Please review
Hi, I have made some refactoring to vf_fillborders.c. My motivation came from using this filter as a template for a new filter. Refactoring the code was a good exercise to understand FFmpeg's data models. I think, the code is now much better readable and I believe, it's faster because of: - more use of memset() and memcpy() - less calculations in the inner for loops - skip the looping for right/left filling when there is nothing to do I would be appreciated, if one would review my proposed changes. Hopefully I've used the right format for the patch. Please honour, that I'm new in FFmpeg development. -Ulf diff --git a/libavfilter/vf_fillborders.c b/libavfilter/vf_fillborders.c index 1344587..a7a074b 100644 --- a/libavfilter/vf_fillborders.c +++ b/libavfilter/vf_fillborders.c @@ -1,5 +1,7 @@ /* * Copyright (c) 2017 Paul B Mahol + * + * Contributors: Ulf Zibis * * This file is part of FFmpeg. * @@ -18,6 +20,7 @@ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA */ +#include #include "libavutil/colorspace.h" #include "libavutil/common.h" #include "libavutil/opt.h" @@ -84,187 +87,176 @@ static void smear_borders8(FillBordersContext *s, AVFrame *frame) { -int p, y; +for (int p = 0; p < s->nb_planes; p++) { +uint8_t *data = frame->data[p]; +int lz = frame->linesize[p]; +int width = s->planewidth[p]; +int height = s->planeheight[p] * lz; +int left = s->borders[p].left; +int right = width - s->borders[p].right; +int top = s->borders[p].top * lz; +int bottom = height - s->borders[p].bottom * lz; -for (p = 0; p < s->nb_planes; p++) { -uint8_t *ptr = frame->data[p]; -int linesize = frame->linesize[p]; +/* fill left and right borders from top to bottom border */ +if (left != 0 || right != width) // in case skip for performance +for (int y = top; y < bottom; y += lz) { +memset(data + y, *(data + y + left), left); +memset(data + y + right, *(data + y + right - 1), width - right); +} -for (y = s->borders[p].top; y < s->planeheight[p] - s->borders[p].bottom; y++) { -memset(ptr + y * linesize, - *(ptr + y * linesize + s->borders[p].left), - s->borders[p].left); -memset(ptr + y * linesize + s->planewidth[p] - s->borders[p].right, - *(ptr + y * linesize + s->planewidth[p] - s->borders[p].right - 1), - s->borders[p].right); -} - -for (y = 0; y < s->borders[p].top; y++) { -memcpy(ptr + y * linesize, - ptr + s->borders[p].top * linesize, s->planewidth[p]); -} - -for (y = s->planeheight[p] - s->borders[p].bottom; y < s->planeheight[p]; y++) { -memcpy(ptr + y * linesize, - ptr + (s->planeheight[p] - s->borders[p].bottom - 1) * linesize, - s->planewidth[p]); -} +/* fill top and bottom borders */ +for (uint8_t *y = data + top; y > data; ) +memcpy(y -= lz, data + top, width); +for (uint8_t *y = data + bottom; y < data + height; y += lz) +memcpy(y, data + bottom - lz, width); } } static void smear_borders16(FillBordersContext *s, AVFrame *frame) { -int p, y, x; +for (int p = 0; p < s->nb_planes; p++) { +uint16_t *data = (uint16_t *)frame->data[p]; +int lz = frame->linesize[p] / sizeof(uint16_t); +int width = s->planewidth[p]; +int height = s->planeheight[p] * lz; +int left = s->borders[p].left; +int right = width - s->borders[p].right; +int top = s->borders[p].top * lz; +int bottom = height - s->borders[p].bottom * lz; -for (p = 0; p < s->nb_planes; p++) { -uint16_t *ptr = (uint16_t *)frame->data[p]; -int linesize = frame->linesize[p] / 2; - -for (y = s->borders[p].top; y < s->planeheight[p] - s->borders[p].bottom; y++) { -for (x = 0; x < s->borders[p].left; x++) { -ptr[y * linesize + x] = *(ptr + y * linesize + s->borders[p].left); +/* fill left and right borders from top to bottom border */ +if (left != 0 || right != width) // in case skip for performance +for (int y = top; y < bottom; y += lz) { +for (int x = left; x >= 0; x--) +data[y + x] = data[y + left]; +for (int x = right; x < width; x++) +data[y + x] = data[y + right - 1]; } -for (x = 0; x < s->borders[p].right; x++) { -ptr[y * linesize + s->planewidth[p] - s->borders[p].right + x] = - *(ptr + y * linesize + s->planewidth[p] - s->borders[p].right - 1); -} -} - -for (y = 0; y < s->borders[p].top; y++) { -memcpy(ptr + y * linesize, -