Re: [FFmpeg-devel] [PATCH] fate.sh: Allow overriding what targets to make for running the tests
On Fri, 1 Dec 2023, Rémi Denis-Courmont wrote: Le 30 novembre 2023 23:13:59 GMT+02:00, "Martin Storsjö" a écrit : On Thu, 30 Nov 2023, Rémi Denis-Courmont wrote: In other words, is publishing on the FATE website worth making the tests coverage and/or the build time worse? By making the test coverage worse, you mean if I'd be doing the full testing of many combinations already, and I'd stop doing that in order to do this lesser testing instead? If I'd be doing it (I currently don't) I guess that would be my concern, not others? No. The point is that this is adding a small hack that works for one specific case for a short while (testing Armv8 IMM8 and DP), but is known not to be sufficient anyway (for SVE, PAuth, RVV, etc). I'll reiterate the question from the bottom of the mail, that you didn't respond to. Would you be ok with a setup, where a FATE instance optionally can run a subset of tests instead of the full suite, but run them multiple times with e.g. different QEMU settings? That would allow repeating checkasm for all the interesting cases - and if one really wanted to spend a lot of CPU time on it, also could run the full FATE suite in all those configurations. // Martin ___ 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] fate.sh: Allow overriding what targets to make for running the tests
Le 30 novembre 2023 23:13:59 GMT+02:00, "Martin Storsjö" a écrit : >On Thu, 30 Nov 2023, Rémi Denis-Courmont wrote: > >> You can already test it properly as things stand, and reporting is trivial, >> just not to the FATE website. The question is whether this is worth adding >> to FATE. > >More public test coverage is better than less, isn't it? That's a false dichotomy. >> In other words, is publishing on the FATE website worth making the tests >> coverage and/or the build time worse? > >By making the test coverage worse, you mean if I'd be doing the full testing >of many combinations already, and I'd stop doing that in order to do this >lesser testing instead? If I'd be doing it (I currently don't) I guess that >would be my concern, not others? No. The point is that this is adding a small hack that works for one specific case for a short while (testing Armv8 IMM8 and DP), but is known not to be sufficient anyway (for SVE, PAuth, RVV, etc). In the end, it's all about not adding inadequate interfaces and supporting/publishing bad solutions. It's certainly not as bad as if it were a public C API, but that doesn't make it good. Normally "insufficient" interfaces don't get merged for a variety of reasons. >>> Again, for SVE, I'd rather have testing with 1 config (the default, which >>> is longer vectors than one usually encounters in HW) rather than none at >>> all. It won't catch every theoretical issue but practically would catch >>> many things at least. >> >> I find that statement very misleading. This is not a question of testing 1 >> config vs 0. It's a question of testing 1 configuration vs all of them(*), >> and reporting that one vs reporting all of them elsewhere than >> FATE.ffmpeg.org. Until/unless somebody does the missing integration. > >Currently I test 0 of these configurations. I would like to test 1 such >config, and publish those results on the FATE website. I don't currently test >any form of "all configs". And if I wanted to make a private setup for testing >"all configs", I really don't see how it would be mutually exclusive with the >publicly posted test results from the one config? > >>> And in order to actually test BTI, one has to link with a sysroot that >>> also was built with BTI enabled - I currently use a sysroot extracted from >>> fedora for that. (And my tests for it use -Wl,-z,force-bti.) >> >> I can readily believe how much of a PITA that would be to set up. I can also >> believe that glibc won't allow masking the guarded page bit in mmap()/ >> mprotect(). >> >> That does not mean you need different builds to test each of the 4 possible >> combinations (or 3 if you ignore the case of BTI without PAC, which does not >> exist in real hardware). Once you have that build, you can test it with >> whichever QEMU CPU settings. > >I didn't mean to imply that one would have to do separate builds for all of >those. I currently don't do any testing with builds with >-mbranch-protection=standard (and with different sysroots), but I was >considering adding one such build, with the fedora sysroot - and only test one >single configuration with it (with QEMU's defaults of all features enabled). > > >So, to spell out your objection in simpler terms. You are firmly against >anybody posting test results on FATE that only include checkasm but not the >rest of the tests, because you consider that this can be misleading/confusing >to people reading the test results - is that right? > >Or would such a setup be acceptable to you, if someone would implement a way >of running the tests (either the full set or only a subset such as chckasm) >multiple times with different QEMU configurations, with the same build of >ffmpeg, within the same FATE run? > >// Martin >___ >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". ___ 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] avfilter: merge loudnorm filter functionality into f_ebur128.c
Hi, On Thu, Nov 30, 2023 at 2:43 PM Paul B Mahol wrote: > But how you could refactor code if one filter shares nothing with another > filter code? > > Its not possible. You all seem to not understand problem at all. I get that this patch swaps the libebur128 loudness computation with the f_ebur128 code. But it also changes the behavior of the AGC and limiter, right? It looks like a whole new algorithm. That's what I mean by new filter behavior. Thanks, Kyle ___ 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] avfilter: merge loudnorm filter functionality into f_ebur128.c
On Thu, Nov 30, 2023 at 11:20 PM Kyle Swanson wrote: > Hi, > > On Thu, Nov 30, 2023 at 1:36 PM Paul B Mahol wrote: > > > > Loudnorm filter is big pile of hacks, I wanted to move forward and I > > improved it. > > > > I received nothing in return just some politics talks. > > > > I will apply this soon unless technical comments arise. > > > > Why would I spend on this more my precious time? For no real gain as I > will > > again > > receive nothing in return except some useless comment and no single > > technical aspect. > > Please don't merge this as-is. I'm sure there are good changes/fixes > here that we should merge, but you need to help your reviewers > understand what is going on. One big commit that combines both > refactoring across files with introduction of new filter behavior is > very hard to review. That's why I'm suggesting we do this in two > steps, I think Anton's suggestion is the same. > But how you could refactor code if one filter shares nothing with another filter code? Its not possible. You all seem to not understand problem at all. > > Thanks, > Kyle > ___ > 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". > ___ 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] avfilter: merge loudnorm filter functionality into f_ebur128.c
Hi, On Thu, Nov 30, 2023 at 1:36 PM Paul B Mahol wrote: > > Loudnorm filter is big pile of hacks, I wanted to move forward and I > improved it. > > I received nothing in return just some politics talks. > > I will apply this soon unless technical comments arise. > > Why would I spend on this more my precious time? For no real gain as I will > again > receive nothing in return except some useless comment and no single > technical aspect. Please don't merge this as-is. I'm sure there are good changes/fixes here that we should merge, but you need to help your reviewers understand what is going on. One big commit that combines both refactoring across files with introduction of new filter behavior is very hard to review. That's why I'm suggesting we do this in two steps, I think Anton's suggestion is the same. Thanks, Kyle ___ 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] avfilter: merge loudnorm filter functionality into f_ebur128.c
On Thu, Nov 30, 2023 at 8:39 PM Kyle Swanson wrote: > Hi, > > On Thu, Nov 30, 2023 at 6:11 AM Paul B Mahol wrote: > > > > I tested this a lot, and its great move forward. > > > > Make more useful testing and review next time, I'm sure you are quite > > capable person. > > Paul, I agree with Anton. Refactoring the code (i.e. changing filter > behavior) and combining it with f_ebur128.c all at once makes the diff > very hard to review. I suggest an incremental approach, let's address > the issues you've identified with the loudnorm filter one by one, and > then a second stage where we see about combining with f_ebur128.c. > Loudnorm filter is big pile of hacks, I wanted to move forward and I improved it. I received nothing in return just some politics talks. I will apply this soon unless technical comments arise. Why would I spend on this more my precious time? For no real gain as I will again receive nothing in return except some useless comment and no single technical aspect. > > Thanks, > Kyle > ___ > 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". > ___ 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] [ANNOUNCE] upcoming vote: TC/CC elections
On Thu, Nov 30, 2023 at 12:01:25AM +, Cosmin Stejerean via ffmpeg-devel wrote: > > > > On Nov 29, 2023, at 3:14 PM, Michael Niedermayer > > wrote: > > > > If you give Jerry a weight of 10 and give Tom a weight of 9, that means > > you prefer Jerry over Tom because 10 > 9 > > If you give Spike a weight of 20 that would mean you not only prefer Spike > > over Tom OR Jerry but also over Tom AND Jerry. Because 20 > 10 + 9 > > > > OTOH if you give Spike a weight of 18 that would mean you prefer Spike over > > Tom OR Jerry but you prefer Tom AND Jerry over Spike. > > Because: 9 < 10< 18 < 9 + 10 > > Tom < Jerry < Spike < Tom and Jerry > > Is this last example the kind of preference that people are likely to want to > express in practice? It seems much harder to reason about and much more > likely to lead to mistakes. > > Given a list of say 7 candidates running for 5 positions that's 21 possible > combinations and in theory weights would have to be assigned such that the > sum for each one of those 21 combinations is correctly ranked by order of > preference. > > I think the simplicity of the simpler ranked choice voting might outweigh the > benefit of expressing complex preferences with the sum of weights. There are two things here assigning purely order vs assigning weights by the voter and one winner vs multi winners which represent the electorate proportionally You cannot simply use a one winner system and pull more winners out of it. That doesnt result in a group that proportionally represents teh electorate To see this, consider a simple 2 party world (which seems oddly common) one party we call the blue and one the yellow party Candidates now could be blue, yellow or maybe gray for the middle ground now if you have 51% blue voters your commitee will have 5 blue candidates most likely because 51% of voters prefer the blue over gray and yellow OTOH with 51% yellow voters your commitee will have 5 yellow candidates You get the same effect with simpler vote systems ask a room if they want a blue or yellow candidate. They say "blue" and so the most popular blue candidate is added. Now you ask the room again, what will they say ? obviously the same as its the same people still and so whoever has 51% controlls 100% of the seats in the commitee Proportional voting systems avoid this by trying to select a committee that is representative for more than just the majority If the weights are too complex, just use something simpler just make a scale from 0 to 5, from hate to love give every candidate you love a 5 and everyone you hate a 0 and if you want use the numbers between for intermediates thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The smallest minority on earth is the individual. Those who deny individual rights cannot claim to be defenders of minorities. - Ayn Rand 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] fate.sh: Allow overriding what targets to make for running the tests
On Thu, 30 Nov 2023, Rémi Denis-Courmont wrote: You can already test it properly as things stand, and reporting is trivial, just not to the FATE website. The question is whether this is worth adding to FATE. More public test coverage is better than less, isn't it? In other words, is publishing on the FATE website worth making the tests coverage and/or the build time worse? By making the test coverage worse, you mean if I'd be doing the full testing of many combinations already, and I'd stop doing that in order to do this lesser testing instead? If I'd be doing it (I currently don't) I guess that would be my concern, not others? And whether I want to spend my cpu cycles on testing for a public FATE config, even if it only tests a limited amount, in addition what tests I (hypothetically) run privately, wasting more CPU on building ffmpeg, that's also my concern and not others? not to mention confusing the existing website users with weirdly incomplete test results. FWIW, there are a bunch of otherwise weird, limited configs as well, e.g. http://fate.ffmpeg.org/history.cgi?slot=x86_64-archlinux-gcc-disableavcodec. Although there, the reason for the limited number of tests is clearly visible. Again, for SVE, I'd rather have testing with 1 config (the default, which is longer vectors than one usually encounters in HW) rather than none at all. It won't catch every theoretical issue but practically would catch many things at least. I find that statement very misleading. This is not a question of testing 1 config vs 0. It's a question of testing 1 configuration vs all of them(*), and reporting that one vs reporting all of them elsewhere than FATE.ffmpeg.org. Until/unless somebody does the missing integration. Currently I test 0 of these configurations. I would like to test 1 such config, and publish those results on the FATE website. I don't currently test any form of "all configs". And if I wanted to make a private setup for testing "all configs", I really don't see how it would be mutually exclusive with the publicly posted test results from the one config? And in order to actually test BTI, one has to link with a sysroot that also was built with BTI enabled - I currently use a sysroot extracted from fedora for that. (And my tests for it use -Wl,-z,force-bti.) I can readily believe how much of a PITA that would be to set up. I can also believe that glibc won't allow masking the guarded page bit in mmap()/ mprotect(). That does not mean you need different builds to test each of the 4 possible combinations (or 3 if you ignore the case of BTI without PAC, which does not exist in real hardware). Once you have that build, you can test it with whichever QEMU CPU settings. I didn't mean to imply that one would have to do separate builds for all of those. I currently don't do any testing with builds with -mbranch-protection=standard (and with different sysroots), but I was considering adding one such build, with the fedora sysroot - and only test one single configuration with it (with QEMU's defaults of all features enabled). So, to spell out your objection in simpler terms. You are firmly against anybody posting test results on FATE that only include checkasm but not the rest of the tests, because you consider that this can be misleading/confusing to people reading the test results - is that right? Or would such a setup be acceptable to you, if someone would implement a way of running the tests (either the full set or only a subset such as chckasm) multiple times with different QEMU configurations, with the same build of ffmpeg, within the same FATE run? // Martin ___ 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 13/13 v2] fftools/ffmpeg: convert to a threaded architecture
On Thu, Nov 30, 2023 at 02:34:59PM +0100, Anton Khirnov wrote: > Quoting Michael Niedermayer (2023-11-30 14:08:26) > > On Sat, Nov 25, 2023 at 09:32:06PM +0100, Anton Khirnov wrote: > > > Change the main loop and every component (demuxers, decoders, filters, > > > encoders, muxers) to use the previously added transcode scheduler. Every > > > instance of every such component was already running in a separate > > > thread, but now they can actually run in parallel. > > > > > > Changes the results of ffmpeg-fix_sub_duration_heartbeat - tested by > > > JEEB to be more correct and deterministic. > > > --- > > > fftools/ffmpeg.c | 374 + > > > fftools/ffmpeg.h | 97 +-- > > > fftools/ffmpeg_dec.c | 321 ++-- > > > fftools/ffmpeg_demux.c| 268 --- > > > fftools/ffmpeg_enc.c | 368 ++--- > > > fftools/ffmpeg_filter.c | 722 +- > > > fftools/ffmpeg_mux.c | 324 ++-- > > > fftools/ffmpeg_mux.h | 24 +- > > > fftools/ffmpeg_mux_init.c | 88 +-- > > > fftools/ffmpeg_opt.c | 6 +- > > > .../fate/ffmpeg-fix_sub_duration_heartbeat| 36 +- > > > 11 files changed, 598 insertions(+), 2030 deletions(-) > > > > I tried > > ./ffmpeg -f lavfi -i testsrc2 -bsf:v noise -bitexact -t 2 /tmp/.y4m > > > > with merged ffmpeg_threading into master > > > > and it gets stuck > > > > Stream #0:0 -> #0:0 (wrapped_avframe (native) -> wrapped_avframe (native)) > > Press [q] to stop, [?] for help > > [noise @ 0x55e8fbaea340] Wrapped AVFrame noising is unsupported > > [vost#0:0/wrapped_avframe @ 0x55e8fbae9840] Error initializing bitstream > > filter: noise > > [vf#0:0 @ 0x55e8fbaea880] Error sending frames to consumers: Not yet > > implemented in FFmpeg, patches welcome > > [vf#0:0 @ 0x55e8fbaea880] Task finished with error code: -1163346256 (Not > > yet implemented in FFmpeg, patches welcome) > > [vf#0:0 @ 0x55e8fbaea880] Terminating thread with return code -1163346256 > > (Not yet implemented in FFmpeg, patches welcome) > > Sorry, seems I forgot to update my branch. Did that now, it should not > get suck anymore. this still gets stuck: ./ffmpeg -y -i mm-short.mpg -af apad -shortest /tmp/.nut [...] thx -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB I know you won't believe me, but the highest form of Human Excellence is to question oneself and others. -- Socrates 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] [ANNOUNCE] upcoming vote: TC/CC elections
On 2023-11-30 00:14 +0100, Michael Niedermayer wrote: > On Tue, Nov 28, 2023 at 02:23:13PM +0100, Anton Khirnov wrote: > > For the record, I've edited the vote description to make it more clear. > > It now looks like this: > > > > Five people from the list below will become the members of the Technical > > Committee (TC). Assign weights to each person according to how much you > > want them to be in the committee (higher weight = higher preference). > > > > > The system will assume you want to maximise the sum of weights of > > selected candidates. E.g. if X is given a weight of 10 and Y and Z have > > weights 8 and 6 respectively, then the voting algorithm will assume you > > prefer a committee with both Y and Z over one with X, because 14 > 10. > > However, giving Y and Z weight of 4 and 2 instead would have expressed > > that X is preferred to a combination of Y and Z, because 6 < 10. > > My try in cooking word soup: > > The system will assume you want to maximise the sum of weights of the > selected candidates. > > Some examples: > If you give Jerry a weight of 10 and give Tom a weight of 9, that means > you prefer Jerry over Tom because 10 > 9 > If you give Spike a weight of 20 that would mean you not only prefer Spike > over Tom OR Jerry but also over Tom AND Jerry. Because 20 > 10 + 9 > > OTOH if you give Spike a weight of 18 that would mean you prefer Spike over > Tom OR Jerry but you prefer Tom AND Jerry over Spike. > Because: 9 < 10< 18 < 9 + 10 > Tom < Jerry < Spike < Tom and Jerry This is similar to the improved notice from Anton, but IMHO makes it a bit more tangible by using person names and by also explaining it in a longer way with AND, OR, and more spacing. Not sure how easy it is to add to the voting and get a proper layout/formatting like in this mail with fixed width though. Also not sure if combined-weights mode and the way it makes voting itself harder (at least the first time one is confronted with it) is worth to keep for the next vote. I personally have no strong opinion. This is not complaint. Alexander ___ 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 1/2] checkasm: test for abs_pow34
Okay, I splited and attached Rémi Denis-Courmont 于2023年11月30日周四 23:31写道: > Le tiistaina 28. marraskuuta 2023, 18.59.38 EET flow gg a écrit : > > > > Since nobody else commented, I shall note that you should probably split > the > underlying lavc changes into a separate preliminary patch. > > -- > レミ・デニ-クールモン > http://www.remlab.net/ > > > > ___ > 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". > From 95716fe7798ef207bb7924dff81970a45c358173 Mon Sep 17 00:00:00 2001 From: sunyuechi Date: Fri, 1 Dec 2023 04:21:53 +0800 Subject: [PATCH 1/3] lvac/aacenc: add ff_aac_dsp_init This is for clarity and use in testing, consistent with other parts of the code. --- libavcodec/aacenc.c | 24 ++-- libavcodec/aacenc.h | 1 + 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/libavcodec/aacenc.c b/libavcodec/aacenc.c index 5e6a255a8f..443b25e25a 100644 --- a/libavcodec/aacenc.c +++ b/libavcodec/aacenc.c @@ -1381,16 +1381,7 @@ static av_cold int aac_encode_init(AVCodecContext *avctx) ff_lpc_init(>lpc, 2*avctx->frame_size, TNS_MAX_ORDER, FF_LPC_TYPE_LEVINSON); s->random_state = 0x1f2e3d4c; -s->abs_pow34 = abs_pow34_v; -s->quant_bands = quantize_bands; - -#if ARCH_X86 -ff_aac_dsp_init_x86(s); -#endif - -#if HAVE_MIPSDSP -ff_aac_coder_init_mips(s); -#endif + ff_aac_dsp_init(s); ff_af_queue_init(avctx, >afq); @@ -1444,3 +1435,16 @@ const FFCodec ff_aac_encoder = { AV_SAMPLE_FMT_NONE }, .p.priv_class = _class, }; + +void ff_aac_dsp_init(AACEncContext *s){ +s->abs_pow34 = abs_pow34_v; +s->quant_bands = quantize_bands; + +#if ARCH_X86 +ff_aac_dsp_init_x86(s); +#endif + +#if HAVE_MIPSDSP +ff_aac_coder_init_mips(s); +#endif +} diff --git a/libavcodec/aacenc.h b/libavcodec/aacenc.h index b030c652ae..09dd8639be 100644 --- a/libavcodec/aacenc.h +++ b/libavcodec/aacenc.h @@ -154,6 +154,7 @@ typedef struct AACEncContext { } buffer; } AACEncContext; +void ff_aac_dsp_init(AACEncContext *s); void ff_aac_dsp_init_x86(AACEncContext *s); void ff_aac_coder_init_mips(AACEncContext *c); void ff_quantize_band_cost_cache_init(struct AACEncContext *s); -- 2.43.0 From 3c5b83a744f86107cdb58ad6e288c027cfa8c3cd Mon Sep 17 00:00:00 2001 From: sunyuechi Date: Tue, 28 Nov 2023 14:08:12 +0800 Subject: [PATCH 2/3] checkasm: test for abs_pow34 --- tests/checkasm/Makefile| 1 + tests/checkasm/aacencdsp.c | 70 ++ tests/checkasm/checkasm.c | 3 ++ tests/checkasm/checkasm.h | 1 + tests/fate/checkasm.mak| 3 +- 5 files changed, 77 insertions(+), 1 deletion(-) create mode 100644 tests/checkasm/aacencdsp.c diff --git a/tests/checkasm/Makefile b/tests/checkasm/Makefile index 8bc241d29b..da209dd7ad 100644 --- a/tests/checkasm/Makefile +++ b/tests/checkasm/Makefile @@ -22,6 +22,7 @@ AVCODECOBJS-$(CONFIG_VIDEODSP) += videodsp.o # decoders/encoders AVCODECOBJS-$(CONFIG_AAC_DECODER) += aacpsdsp.o \ sbrdsp.o +AVCODECOBJS-$(CONFIG_AAC_ENCODER) += aacencdsp.o AVCODECOBJS-$(CONFIG_ALAC_DECODER) += alacdsp.o AVCODECOBJS-$(CONFIG_DCA_DECODER) += synth_filter.o AVCODECOBJS-$(CONFIG_EXR_DECODER) += exrdsp.o diff --git a/tests/checkasm/aacencdsp.c b/tests/checkasm/aacencdsp.c new file mode 100644 index 00..684c775862 --- /dev/null +++ b/tests/checkasm/aacencdsp.c @@ -0,0 +1,70 @@ +/* + * Copyright (c) 2023 Institue of Software Chinese Academy of Sciences (ISCAS). + * + * This file is part of FFmpeg. + * + * FFmpeg is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * FFmpeg is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with FFmpeg; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#include + +#include "libavutil/mem.h" +#include "libavutil/mem_internal.h" + +#include "libavcodec/aacenc.h" + +#include "checkasm.h" + +#define randomize_float(buf, len) \ +do {\ +int i; \ +for (i = 0; i < len; i++) { \ +float f = (float)rnd() /
Re: [FFmpeg-devel] [PATCH] avfilter: merge loudnorm filter functionality into f_ebur128.c
Hi, On Thu, Nov 30, 2023 at 6:11 AM Paul B Mahol wrote: > > I tested this a lot, and its great move forward. > > Make more useful testing and review next time, I'm sure you are quite > capable person. Paul, I agree with Anton. Refactoring the code (i.e. changing filter behavior) and combining it with f_ebur128.c all at once makes the diff very hard to review. I suggest an incremental approach, let's address the issues you've identified with the loudnorm filter one by one, and then a second stage where we see about combining with f_ebur128.c. Thanks, Kyle ___ 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] fate.sh: Allow overriding what targets to make for running the tests
Le torstaina 30. marraskuuta 2023, 18.28.39 EET Martin Storsjö a écrit : > On Thu, 30 Nov 2023, Rémi Denis-Courmont wrote: > > Le torstaina 30. marraskuuta 2023, 17.34.31 EET Martin Storsjö a écrit : > >> Yeah, I wouldn't reuse an existing build here. For the setup I have in > >> mind, one build doesn't take too horribly long (either on an old desktop > >> x86 machine, or a moderate aarch64 server) - so it's not ideal but not a > >> dealbreaker anyway (while running all of fate with qemu takes one > >> magnitude longer). > > > > Well it's pretty much a deal breaker for Armv9 and RV. I can understand > > wanting to build on a comfy x86 server, but doing different builds just to > > change QEMU CPU flags is IMO inept. > > Yes. But for doing one single run with QEMU, I don't mind. You can already test it properly as things stand, and reporting is trivial, just not to the FATE website. The question is whether this is worth adding to FATE. In other words, is publishing on the FATE website worth making the tests coverage and/or the build time worse? not to mention confusing the existing website users with weirdly incomplete test results. > Again, for SVE, I'd rather have testing with 1 config (the default, which > is longer vectors than one usually encounters in HW) rather than none at > all. It won't catch every theoretical issue but practically would catch > many things at least. I find that statement very misleading. This is not a question of testing 1 config vs 0. It's a question of testing 1 configuration vs all of them(*), and reporting that one vs reporting all of them elsewhere than FATE.ffmpeg.org. Until/unless somebody does the missing integration. (*) at least those that QEMU supports > Are you volunteering to write FATE integration to run checkasm multiple > times with different QEMU settings, so I can wait for that instead of > having much improved public test coverage right now? Of course I will not volunteer, given that the RISE project already has an outstanding RfP which will likely require this done professionally: https://hubs.la/Q029hwpS0 (That does not mean that I would have volunteered otherwise, just that the question is moot as far as I am concerned and for the time being.) > > Sure, we could just build once and run several times checkasm with a > > separate script, as I already pointed out. But then this patch is > > completely unnecessary. > > Indeed, that's trivial to do for a private testing setup. > > >> For the other setup I intended to test, to test AArch64 PAC and BTI, I > >> would do a separate build with -mbranch-protection=standard anyway. > > > > That does not make much sense to me. PAC and BTI should be enabled by > > default in compatibility mode (for ARMv8.0-8.2 builds) or > > noncompatibility mode (for ARMv8.3+ builds). > > Maybe it should - but it currently isn't. That's really up to whoever set up the AArch64 builds to fix their build flags TBH (I believe that the assembler is already sorted). And at least for PAuth, that should be sufficient, as support from the C runtime is not required. > And in order to actually test BTI, one has to link with a sysroot that > also was built with BTI enabled - I currently use a sysroot extracted from > fedora for that. (And my tests for it use -Wl,-z,force-bti.) I can readily believe how much of a PITA that would be to set up. I can also believe that glibc won't allow masking the guarded page bit in mmap()/ mprotect(). That does not mean you need different builds to test each of the 4 possible combinations (or 3 if you ignore the case of BTI without PAC, which does not exist in real hardware). Once you have that build, you can test it with whichever QEMU CPU settings. Surely Fedora, of all distros, is not going to treat Armv8.5-BTI as a distinct arch from AArch64 whilst Arm made sure it was both backward-compatible and runtime-tunable. -- レミ・デニ-クールモン http://www.remlab.net/ ___ 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 v2 1/3] avfilter/vf_bwdif: consider chroma subsampling when enforcing minimum dimensions
On Nov 30, 2023, at 04:37, Thomas Mundt wrote: Am Do., 30. Nov. 2023 um 01:23 Uhr schrieb Cosmin Stejerean via ffmpeg-devel mailto:ffmpeg-devel@ffmpeg.org> >: From: Cosmin Stejerean mailto:cos...@cosmin.at> > Fixes #10688 Signed-off-by: Cosmin Stejerean mailto:cos...@cosmin.at> > --- libavfilter/vf_bwdif.c | 13 ++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/libavfilter/vf_bwdif.c b/libavfilter/vf_bwdif.c index 137cd5ef13..80aa85a48b 100644 --- a/libavfilter/vf_bwdif.c +++ b/libavfilter/vf_bwdif.c @@ -191,12 +191,19 @@ static int config_props(AVFilterLink *link) return ret; } - if (link->w < 3 || link->h < 4) { - av_log(ctx, AV_LOG_ERROR, "Video of less than 3 columns or 4 lines is not supported\n"); + const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(link->format); + + int h = link->h; + int w = link->w; + int h_chroma = AV_CEIL_RSHIFT(h, desc->log2_chroma_h); + int w_chroma = AV_CEIL_RSHIFT(w, desc->log2_chroma_w); + + if (w < 3 || w_chroma < 3 || h < 4 || h_chroma < 4) { + av_log(ctx, AV_LOG_ERROR, "Video with planes less than 3 columns or 4 lines is not supported\n"); return AVERROR(EINVAL); } - yadif->csp = av_pix_fmt_desc_get(link->format); + yadif->csp = desc; yadif->filter = filter; ff_bwdif_init_filter_line(>dsp, yadif->csp->comp[0].depth); I think mixed declarations are not allowed. Also log2_chroma_w/h should never be negative, so why not just do: if (AV_CEIL_RSHIFT(link->w, yadif->csp->log2_chroma_w) < 3 || AV_CEIL_RSHIFT(link->h, yadif->csp->log2_chroma_h) < 4) Thank you for the prompt feedback, makes a lot of sense to me, will update in v3. - Cosmin ___ 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] fate.sh: Allow overriding what targets to make for running the tests
On Thu, 30 Nov 2023, Rémi Denis-Courmont wrote: Le torstaina 30. marraskuuta 2023, 17.34.31 EET Martin Storsjö a écrit : Yeah, I wouldn't reuse an existing build here. For the setup I have in mind, one build doesn't take too horribly long (either on an old desktop x86 machine, or a moderate aarch64 server) - so it's not ideal but not a dealbreaker anyway (while running all of fate with qemu takes one magnitude longer). Well it's pretty much a deal breaker for Armv9 and RV. I can understand wanting to build on a comfy x86 server, but doing different builds just to change QEMU CPU flags is IMO inept. Yes. But for doing one single run with QEMU, I don't mind. Again, for SVE, I'd rather have testing with 1 config (the default, which is longer vectors than one usually encounters in HW) rather than none at all. It won't catch every theoretical issue but practically would catch many things at least. Are you volunteering to write FATE integration to run checkasm multiple times with different QEMU settings, so I can wait for that instead of having much improved public test coverage right now? Sure, we could just build once and run several times checkasm with a separate script, as I already pointed out. But then this patch is completely unnecessary. Indeed, that's trivial to do for a private testing setup. For the other setup I intended to test, to test AArch64 PAC and BTI, I would do a separate build with -mbranch-protection=standard anyway. That does not make much sense to me. PAC and BTI should be enabled by default in compatibility mode (for ARMv8.0-8.2 builds) or noncompatibility mode (for ARMv8.3+ builds). Maybe it should - but it currently isn't. And in order to actually test BTI, one has to link with a sysroot that also was built with BTI enabled - I currently use a sysroot extracted from fedora for that. (And my tests for it use -Wl,-z,force-bti.) // Martin ___ 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] fate.sh: Allow overriding what targets to make for running the tests
Le torstaina 30. marraskuuta 2023, 17.34.31 EET Martin Storsjö a écrit : > Yeah, I wouldn't reuse an existing build here. For the setup I have in > mind, one build doesn't take too horribly long (either on an old desktop > x86 machine, or a moderate aarch64 server) - so it's not ideal but not a > dealbreaker anyway (while running all of fate with qemu takes one > magnitude longer). Well it's pretty much a deal breaker for Armv9 and RV. I can understand wanting to build on a comfy x86 server, but doing different builds just to change QEMU CPU flags is IMO inept. Sure, we could just build once and run several times checkasm with a separate script, as I already pointed out. But then this patch is completely unnecessary. > For the other setup I intended to test, to test AArch64 PAC and BTI, I > would do a separate build with -mbranch-protection=standard anyway. That does not make much sense to me. PAC and BTI should be enabled by default in compatibility mode (for ARMv8.0-8.2 builds) or noncompatibility mode (for ARMv8.3+ builds). The resulting code should be tested with and without PAC and with and without BTI. Separate builds only might make sense if you want to do something more fancy with PAC, requiring the non-HINT instructions, but then that is beyond "standard" branch protection. For BTI, there are no reasons whatsoever to make separate builds; it's a literal waste of time and energy. -- レミ・デニ-クールモン http://www.remlab.net/ ___ 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".
[FFmpeg-devel] [PATCH] examples/transcode: fix log message
From: Zhao Zhili 'encoder' can be audio or video encoder. --- doc/examples/transcode.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/examples/transcode.c b/doc/examples/transcode.c index ed6ac9fa03..a544ec0340 100644 --- a/doc/examples/transcode.c +++ b/doc/examples/transcode.c @@ -196,7 +196,7 @@ static int open_output_file(const char *filename) /* Third parameter can be used to pass settings to encoder */ ret = avcodec_open2(enc_ctx, encoder, NULL); if (ret < 0) { -av_log(NULL, AV_LOG_ERROR, "Cannot open video encoder for stream #%u\n", i); +av_log(NULL, AV_LOG_ERROR, "Cannot open %s encoder for stream #%u\n", encoder->name, i); return ret; } ret = avcodec_parameters_from_context(out_stream->codecpar, enc_ctx); -- 2.34.1 ___ 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] fate.sh: Allow overriding what targets to make for running the tests
Le tiistaina 28. marraskuuta 2023, 16.21.55 EET Michael Niedermayer a écrit : > On Tue, Nov 28, 2023 at 09:27:08AM +0200, Rémi Denis-Courmont wrote: > > Le 28 novembre 2023 01:22:14 GMT+02:00, Michael Niedermayer a écrit : > > >On Mon, Nov 27, 2023 at 05:46:40PM +0200, Rémi Denis-Courmont wrote: > > >[...] > > > > > >> Also FWIW, RV broke due to misaligned accesses and illegal vector types > > >> that QEMU tolerated. That is rather an argument against QEMU than > > >> against this MR but still. > > > > > >has someone reported this to qemu ? > > >(seems like a bug) > > > > It's not a bug. The specification leaves those cases *undefined*. QEMU > > supports them because they can, and adding sanity checks would just slow > > stuff down. > > > > Also generally QEMU TCG policy seems to be maximize perf and > > compatibility, not formal correctness. > I think i read somewhere that recent qemu supposedly checks alignment on arm > more completely. But i couldnt quickly find a official statement about that As of 8.2.0-rc2, it most definitely does not: 8< static inline void gen_check_sp_alignment(DisasContext *s) { /* The AArch64 architecture mandates that (if enabled via PSTATE * or SCTLR bits) there is a check that SP is 16-aligned on every * SP-relative load or store (with an exception generated if it is not). * In line with general QEMU practice regarding misaligned accesses, * we omit these checks for the sake of guest program performance. * This function is provided as a hook so we can more easily add these * checks in future (possibly as a "favour catching guest program bugs * over speed" user selectable option). */ } >8 And this is an actual violation of the specification. In the RISC-V case, QEMU is not even violating the specification, just making a different choice than the only one currently commercially available hardware implementation. > But either way, qemu could emit such code optionally when it is used for > testing. Which is one of the things people use qemu for. That would be very true for system mode "soft-MMU" QEMU, but much more questionable for user mode. In any case, I don't make their policies. > So IMHO it would make sense for qemu to detect cases that are undefined > even if for no other reason than to emulate the hw more exactly. I would agree that optional flags would be sensible. But TBH, we don't even yet know how the IPs from other vendors than Alibaba/T-Head will behave. > If this is not done, qemu can be detected and code could refuse or > fail to run -- レミ・デニ-クールモン http://www.remlab.net/ ___ 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] [POC][PATCHSET] Add qrencodesrc source
> On Nov 30, 2023, at 03:07, Tomas Härdin wrote: > > tor 2023-11-30 klockan 01:49 +0100 skrev Stefano Sabatini: >> This is meant to introduce functionality to handle QR codes. > > Why? > The why seems to be answered below the section you quoted in the original email > QR codes are robust to lossy coding, therefore it should be possible to use > them to compare a generated video with a reference one (in case they cannot be compared frame-by-frame). - Cosmin ___ 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] fate.sh: Allow overriding what targets to make for running the tests
On Thu, 30 Nov 2023, Rémi Denis-Courmont wrote: Le 27 novembre 2023 23:55:18 GMT+02:00, "Martin Storsjö" a écrit : On Mon, 27 Nov 2023, Rémi Denis-Courmont wrote: Le maanantaina 27. marraskuuta 2023, 14.31.18 EET Martin Storsjö a écrit : This can be useful if doing testing of uncommon CPU extensions by running tests with QEMU (by configuring with e.g. "target_exec=qemu-aarch64"), by only running the checkasm tests, to get a reasonable test coverage without excessive test runtime. For the purpose of testing future or bleeding-edge CPU extensions on emulator, you would normally want to be able to actually filter those in. That is more of a matter of patching checkasm than FATE. Sorry, can you elaborate on what you mean with "filter those in" here? You're running all checkasm tests, not just those that require the emulator. But what's potentially much worse is that you're triggering a whole build, or it's not entirely clear from the description how you'd reuse an existing build. Yeah, I wouldn't reuse an existing build here. For the setup I have in mind, one build doesn't take too horribly long (either on an old desktop x86 machine, or a moderate aarch64 server) - so it's not ideal but not a dealbreaker anyway (while running all of fate with qemu takes one magnitude longer). For the other setup I intended to test, to test AArch64 PAC and BTI, I would do a separate build with -mbranch-protection=standard anyway. For Armv8, that's just bad. For RV, that's terrible, as we need to run the same checkasm with different emulator configuration (different $QEMU_CPU in the case of QEMU): one per vector length. Armv9 will potentially have the same problem if FFmpeg grows SVE(2) support. Yes, for SVE I would ideally like to test all vector lengths (I did such a setup for x264 recently, when someone was proposing some SVE codepaths). I don't have a neat idea for how to integrate that into FATE, and this patch doesn't buy us that indeed. But running tests with the default QEMU settings would at least test with a larger vector length than the usual, so it would provide at least some coverage. Not exhaustive, but at least something. So the setup I have in mind wouldn't cover all those cases, but it would at least fix some current gaps in testing coverage. I guess it's a case of whether we should let perfect be the enemy of good; adding this lets us easily add a fair bit of more coverage, in particular of the (new) handwritten asm. And it shouldn't get in the way of doing other better solutions at a later point. // Martin ___ 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 1/2] checkasm: test for abs_pow34
Le tiistaina 28. marraskuuta 2023, 18.59.38 EET flow gg a écrit : > Since nobody else commented, I shall note that you should probably split the underlying lavc changes into a separate preliminary patch. -- レミ・デニ-クールモン http://www.remlab.net/ ___ 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 4/9] avutil: introduce an Immersive Audio Model and Formats API
Quoting James Almer (2023-11-30 15:27:49) > > But this is the only boolean field. For now. Who's to say there will not be more in the future. -- Anton Khirnov ___ 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] fate.sh: Allow overriding what targets to make for running the tests
Le 27 novembre 2023 23:55:18 GMT+02:00, "Martin Storsjö" a écrit : >On Mon, 27 Nov 2023, Rémi Denis-Courmont wrote: > >> Le maanantaina 27. marraskuuta 2023, 14.31.18 EET Martin Storsjö a écrit : >>> This can be useful if doing testing of uncommon CPU extensions by >>> running tests with QEMU (by configuring with e.g. >>> "target_exec=qemu-aarch64"), by only running the checkasm tests, >>> to get a reasonable test coverage without excessive test runtime. >> >> For the purpose of testing future or bleeding-edge CPU extensions on >> emulator, you would normally want to be able to actually filter those in. >> That is more of a matter of patching checkasm than FATE. > >Sorry, can you elaborate on what you mean with "filter those in" here? You're running all checkasm tests, not just those that require the emulator. But what's potentially much worse is that you're triggering a whole build, or it's not entirely clear from the description how you'd reuse an existing build. For Armv8, that's just bad. For RV, that's terrible, as we need to run the same checkasm with different emulator configuration (different $QEMU_CPU in the case of QEMU): one per vector length. Armv9 will potentially have the same problem if FFmpeg grows SVE(2) support. > >> Considering the poor coverage of checkasm, I fear that this just gives the >> wrong impression, not to say a false sense of security. It feels misleading >> to encourage or support that paradigm into FATE, in light of that poor >> coverage. Afterall, if it's just about running checkasm, anybody can just >> run `make tests/checkasm/checkasm && tests/checkasm/checkasm`. > >Yes, anybody can run that - but having those results posted continuously >somewhere where other can see them can be valuable as well. > >Anyway, the concrete case I'm considering, is that we've got AArch64 code >merged, that uses the I8MM extensions. We don't have any FATE configuration >that continuously test that. Whenever there are patches, I do spin up a cloud >instance that supports this extension and test the patches there, but >inbetween that we're pretty much blind. > >While checkasm's coverage isn't fantastic, for this particular case I'm not >merging any AArch64 code for new extensions unless that code is covered by >checkasm. > >The other AArch64 feature that we do have code for, which also is untested, is >the assembly support for branch protection and pointer authentication. Also >this is testable pretty easily with QEMU. It's of course more interesting to >run the full fate suite, but if we're not looking for bugs in the compiler but >only for bugs in our assembly, then checkasm should cover most of it. > >Yes there's potential for QEMU bugs hiding real issues, but I'd rather have a >regular run of QEMU+checkasm than not have it tested at all. And I'm >volunteering HW+time for testing these cases with QEMU for whatever checkasm >covers, but I'm not volunteering it for full fate runs with QEMU. > >And sure, I can just run such configs privately, as I already do run a bunch >of various regular builds for projects I care about - but as we do have FATE >with the public status page, hooking it up to be reported there would feel >like added value for everybody. > >// Martin >___ >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". ___ 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] avfilter: merge loudnorm filter functionality into f_ebur128.c
On Thu, Nov 30, 2023 at 2:57 PM Anton Khirnov wrote: > Quoting Paul B Mahol (2023-11-30 15:01:23) > > On Thu, Nov 30, 2023 at 2:47 PM Anton Khirnov wrote: > > > > > Quoting Paul B Mahol (2023-11-30 13:48:16) > > > > On Thu, Nov 30, 2023 at 12:43 PM Anton Khirnov > > > wrote: > > > > > > > > > Quoting Paul B Mahol (2023-11-28 17:51:14) > > > > > > Major change: loudnorm no longer returns oversampled audio at > 192000 > > > Hz > > > > > > when doing dynamic processing. > > > > > > Oversampled audio is only used for true peak finding now. > > > > > > This was trivial improvement as possible with ebur128 code. > > > > > > Minor changes: numerous stability fixes > > > > > > > > > > This patch in unreviewable and should be split. > > > > > > > > > > > > > It cant be split, > > > > > > Why not? > > > > > > > Because it is new code and refactoring, > > Then it should be first refactoring, then new code. > I wrote it wrongly its more than refactoring as code shares no similar code between this two filters. > > > and I know it enough that I'm confident to claim so. > > Hubris usually implies more bugs. > I tested this a lot, and its great move forward. Make more useful testing and review next time, I'm sure you are quite capable person. > > -- > Anton Khirnov > ___ > 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". > ___ 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] avfilter: merge loudnorm filter functionality into f_ebur128.c
Quoting Paul B Mahol (2023-11-30 15:01:23) > On Thu, Nov 30, 2023 at 2:47 PM Anton Khirnov wrote: > > > Quoting Paul B Mahol (2023-11-30 13:48:16) > > > On Thu, Nov 30, 2023 at 12:43 PM Anton Khirnov > > wrote: > > > > > > > Quoting Paul B Mahol (2023-11-28 17:51:14) > > > > > Major change: loudnorm no longer returns oversampled audio at 192000 > > Hz > > > > > when doing dynamic processing. > > > > > Oversampled audio is only used for true peak finding now. > > > > > This was trivial improvement as possible with ebur128 code. > > > > > Minor changes: numerous stability fixes > > > > > > > > This patch in unreviewable and should be split. > > > > > > > > > > It cant be split, > > > > Why not? > > > > Because it is new code and refactoring, Then it should be first refactoring, then new code. > and I know it enough that I'm confident to claim so. Hubris usually implies more bugs. -- Anton Khirnov ___ 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] avfilter: merge loudnorm filter functionality into f_ebur128.c
On Thu, Nov 30, 2023 at 2:47 PM Anton Khirnov wrote: > Quoting Paul B Mahol (2023-11-30 13:48:16) > > On Thu, Nov 30, 2023 at 12:43 PM Anton Khirnov > wrote: > > > > > Quoting Paul B Mahol (2023-11-28 17:51:14) > > > > Major change: loudnorm no longer returns oversampled audio at 192000 > Hz > > > > when doing dynamic processing. > > > > Oversampled audio is only used for true peak finding now. > > > > This was trivial improvement as possible with ebur128 code. > > > > Minor changes: numerous stability fixes > > > > > > This patch in unreviewable and should be split. > > > > > > > It cant be split, > > Why not? > Because it is new code and refactoring, and I know it enough that I'm confident to claim so. > > -- > Anton Khirnov > ___ > 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". > ___ 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] avfilter: merge loudnorm filter functionality into f_ebur128.c
Quoting Paul B Mahol (2023-11-30 13:48:16) > On Thu, Nov 30, 2023 at 12:43 PM Anton Khirnov wrote: > > > Quoting Paul B Mahol (2023-11-28 17:51:14) > > > Major change: loudnorm no longer returns oversampled audio at 192000 Hz > > > when doing dynamic processing. > > > Oversampled audio is only used for true peak finding now. > > > This was trivial improvement as possible with ebur128 code. > > > Minor changes: numerous stability fixes > > > > This patch in unreviewable and should be split. > > > > It cant be split, Why not? -- Anton Khirnov ___ 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 13/13 v2] fftools/ffmpeg: convert to a threaded architecture
Quoting Michael Niedermayer (2023-11-30 14:08:26) > On Sat, Nov 25, 2023 at 09:32:06PM +0100, Anton Khirnov wrote: > > Change the main loop and every component (demuxers, decoders, filters, > > encoders, muxers) to use the previously added transcode scheduler. Every > > instance of every such component was already running in a separate > > thread, but now they can actually run in parallel. > > > > Changes the results of ffmpeg-fix_sub_duration_heartbeat - tested by > > JEEB to be more correct and deterministic. > > --- > > fftools/ffmpeg.c | 374 + > > fftools/ffmpeg.h | 97 +-- > > fftools/ffmpeg_dec.c | 321 ++-- > > fftools/ffmpeg_demux.c| 268 --- > > fftools/ffmpeg_enc.c | 368 ++--- > > fftools/ffmpeg_filter.c | 722 +- > > fftools/ffmpeg_mux.c | 324 ++-- > > fftools/ffmpeg_mux.h | 24 +- > > fftools/ffmpeg_mux_init.c | 88 +-- > > fftools/ffmpeg_opt.c | 6 +- > > .../fate/ffmpeg-fix_sub_duration_heartbeat| 36 +- > > 11 files changed, 598 insertions(+), 2030 deletions(-) > > I tried > ./ffmpeg -f lavfi -i testsrc2 -bsf:v noise -bitexact -t 2 /tmp/.y4m > > with merged ffmpeg_threading into master > > and it gets stuck > > Stream #0:0 -> #0:0 (wrapped_avframe (native) -> wrapped_avframe (native)) > Press [q] to stop, [?] for help > [noise @ 0x55e8fbaea340] Wrapped AVFrame noising is unsupported > [vost#0:0/wrapped_avframe @ 0x55e8fbae9840] Error initializing bitstream > filter: noise > [vf#0:0 @ 0x55e8fbaea880] Error sending frames to consumers: Not yet > implemented in FFmpeg, patches welcome > [vf#0:0 @ 0x55e8fbaea880] Task finished with error code: -1163346256 (Not yet > implemented in FFmpeg, patches welcome) > [vf#0:0 @ 0x55e8fbaea880] Terminating thread with return code -1163346256 > (Not yet implemented in FFmpeg, patches welcome) Sorry, seems I forgot to update my branch. Did that now, it should not get suck anymore. -- Anton Khirnov ___ 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 1/9] avutil/mem: add av_dynarray2_add_nofree
On 11/30/2023 7:39 AM, Anton Khirnov wrote: av_dynarray2_add_nofree This is getting ridiculous. Do we really need 4 separate dynarray_add functions? Wouldn't a single one with a flags argument do the job? Maybe, but i just wanted a av_dynarray2_add() that would not free the array on failure, as that'd mean leaks for my usecase. I'll just try to use av_calloc() everywhere instead. ___ 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 2/9] avcodec/get_bits: add get_leb()
On 11/30/2023 7:40 AM, Anton Khirnov wrote: add get_leb() Do you expect people to understand what this means? Will add "Read an unsigned integer coded as a variable number of little-endian bytes". ___ 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 13/13 v2] fftools/ffmpeg: convert to a threaded architecture
On Sat, Nov 25, 2023 at 09:32:06PM +0100, Anton Khirnov wrote: > Change the main loop and every component (demuxers, decoders, filters, > encoders, muxers) to use the previously added transcode scheduler. Every > instance of every such component was already running in a separate > thread, but now they can actually run in parallel. > > Changes the results of ffmpeg-fix_sub_duration_heartbeat - tested by > JEEB to be more correct and deterministic. > --- > fftools/ffmpeg.c | 374 + > fftools/ffmpeg.h | 97 +-- > fftools/ffmpeg_dec.c | 321 ++-- > fftools/ffmpeg_demux.c| 268 --- > fftools/ffmpeg_enc.c | 368 ++--- > fftools/ffmpeg_filter.c | 722 +- > fftools/ffmpeg_mux.c | 324 ++-- > fftools/ffmpeg_mux.h | 24 +- > fftools/ffmpeg_mux_init.c | 88 +-- > fftools/ffmpeg_opt.c | 6 +- > .../fate/ffmpeg-fix_sub_duration_heartbeat| 36 +- > 11 files changed, 598 insertions(+), 2030 deletions(-) I tried ./ffmpeg -f lavfi -i testsrc2 -bsf:v noise -bitexact -t 2 /tmp/.y4m with merged ffmpeg_threading into master and it gets stuck Stream #0:0 -> #0:0 (wrapped_avframe (native) -> wrapped_avframe (native)) Press [q] to stop, [?] for help [noise @ 0x55e8fbaea340] Wrapped AVFrame noising is unsupported [vost#0:0/wrapped_avframe @ 0x55e8fbae9840] Error initializing bitstream filter: noise [vf#0:0 @ 0x55e8fbaea880] Error sending frames to consumers: Not yet implemented in FFmpeg, patches welcome [vf#0:0 @ 0x55e8fbaea880] Task finished with error code: -1163346256 (Not yet implemented in FFmpeg, patches welcome) [vf#0:0 @ 0x55e8fbaea880] Terminating thread with return code -1163346256 (Not yet implemented in FFmpeg, patches welcome) [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The day soldiers stop bringing you their problems is the day you have stopped leading them. They have either lost confidence that you can help or concluded you do not care. Either case is a failure of leadership. - Colin Powell 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] lavc/dvdsubenc: only check canvas size when it is actually set
Probably ok ___ 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 2/9] avcodec/get_bits: add get_leb()
On Thu, Nov 30, 2023 at 11:40 AM Anton Khirnov wrote: > > add get_leb() > > Do you expect people to understand what this means? > get_leb() : get little-endian bits. > -- > Anton Khirnov > ___ > 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". > ___ 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] avfilter: merge loudnorm filter functionality into f_ebur128.c
On Thu, Nov 30, 2023 at 12:43 PM Anton Khirnov wrote: > Quoting Paul B Mahol (2023-11-28 17:51:14) > > Major change: loudnorm no longer returns oversampled audio at 192000 Hz > > when doing dynamic processing. > > Oversampled audio is only used for true peak finding now. > > This was trivial improvement as possible with ebur128 code. > > Minor changes: numerous stability fixes > > This patch in unreviewable and should be split. > It cant be split, its unreviewable only for those lacking advanced C and git skills. > -- > Anton Khirnov > ___ > 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". > ___ 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 v2 1/3] avfilter/vf_bwdif: consider chroma subsampling when enforcing minimum dimensions
Am Do., 30. Nov. 2023 um 01:23 Uhr schrieb Cosmin Stejerean via ffmpeg-devel : > From: Cosmin Stejerean > > Fixes #10688 > > Signed-off-by: Cosmin Stejerean > --- > libavfilter/vf_bwdif.c | 13 ++--- > 1 file changed, 10 insertions(+), 3 deletions(-) > > diff --git a/libavfilter/vf_bwdif.c b/libavfilter/vf_bwdif.c > index 137cd5ef13..80aa85a48b 100644 > --- a/libavfilter/vf_bwdif.c > +++ b/libavfilter/vf_bwdif.c > @@ -191,12 +191,19 @@ static int config_props(AVFilterLink *link) > return ret; > } > > -if (link->w < 3 || link->h < 4) { > -av_log(ctx, AV_LOG_ERROR, "Video of less than 3 columns or 4 > lines is not supported\n"); > +const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(link->format); > + > +int h = link->h; > +int w = link->w; > +int h_chroma = AV_CEIL_RSHIFT(h, desc->log2_chroma_h); > +int w_chroma = AV_CEIL_RSHIFT(w, desc->log2_chroma_w); > + > +if (w < 3 || w_chroma < 3 || h < 4 || h_chroma < 4) { > +av_log(ctx, AV_LOG_ERROR, "Video with planes less than 3 columns > or 4 lines is not supported\n"); > return AVERROR(EINVAL); > } > > -yadif->csp = av_pix_fmt_desc_get(link->format); > +yadif->csp = desc; > yadif->filter = filter; > ff_bwdif_init_filter_line(>dsp, yadif->csp->comp[0].depth); > > I think mixed declarations are not allowed. Also log2_chroma_w/h should never be negative, so why not just do: if (AV_CEIL_RSHIFT(link->w, yadif->csp->log2_chroma_w) < 3 || AV_CEIL_RSHIFT(link->h, yadif->csp->log2_chroma_h) < 4) Regards, Thomas ___ 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".
[FFmpeg-devel] [PATCH] avcodec/av1dec: Fix resolving zero divisor
Fixes: Out of array read Fixes: global-buffer-overflow-AV1 Found-by: "Leonelli, Matteo" Tested-by: "Wang, Fei W" Reviewed-by: "Wang, Fei W" Signed-off-by: Michael Niedermayer --- libavcodec/av1dec.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c index 6114cb78e65..4dcde234c6c 100644 --- a/libavcodec/av1dec.c +++ b/libavcodec/av1dec.c @@ -177,7 +177,7 @@ static uint8_t get_shear_params_valid(AV1DecContext *s, int idx) int16_t alpha, beta, gamma, delta, divf, divs; int64_t v, w; int32_t *param = >cur_frame.gm_params[idx][0]; -if (param[2] < 0) +if (param[2] <= 0) return 0; alpha = av_clip_int16(param[2] - (1 << AV1_WARPEDMODEL_PREC_BITS)); -- 2.17.1 ___ 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] swscale/x86/rgb_2_rgb: Add opaque pointer to missed definitions of ff_nv12ToUV
Quoting Alfred Wingate via ffmpeg-devel (2023-11-14 14:26:47) > Opaque parameters were previously added to the original definition of > ff_nv12ToUV, leading to gcc noticing a type mismatch with -Wlto-type-mismatch. > > https://git.ffmpeg.org/gitweb/ffmpeg.git/commit/f2de911818fbd7e73343803626b697fd0c968121 > https://bugs.gentoo.org/907484 > > Signed-off-by: Alfred Wingate > --- > libswscale/x86/rgb2rgb_template.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) Looks ok, will push soonish if noone objects. -- Anton Khirnov ___ 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] [POC][PATCHSET] Add qrencodesrc source
Tomas Härdin (12023-11-30): > Why? Why not? -- Nicolas George ___ 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] configure: fix linker error due to missing dependency
Quoting Michael Riedl (2023-11-15 13:40:45) > Add av1_parser to av1_decoder to fix undefined reference to > 'ff_av1_framerate'. This is wrong, rather you should add av1_parse.o to OBJS-$(CONFIG_AV1_DECODER) in libavcodec/Makefile -- Anton Khirnov ___ 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] avfilter: merge loudnorm filter functionality into f_ebur128.c
Quoting Paul B Mahol (2023-11-28 17:51:14) > Major change: loudnorm no longer returns oversampled audio at 192000 Hz > when doing dynamic processing. > Oversampled audio is only used for true peak finding now. > This was trivial improvement as possible with ebur128 code. > Minor changes: numerous stability fixes This patch in unreviewable and should be split. -- Anton Khirnov ___ 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] [POC][PATCHSET] Add qrencodesrc source
On Thu, Nov 30, 2023 at 12:07 PM Tomas Härdin wrote: > tor 2023-11-30 klockan 01:49 +0100 skrev Stefano Sabatini: > > This is meant to introduce functionality to handle QR codes. > > Why? > For such trivial functionality using external library is unacceptable. > > /Tomas > ___ > 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". > ___ 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] [POC][PATCHSET] Add qrencodesrc source
tor 2023-11-30 klockan 01:49 +0100 skrev Stefano Sabatini: > This is meant to introduce functionality to handle QR codes. Why? /Tomas ___ 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] fate.sh: Allow overriding what targets to make for running the tests
On Mon, 27 Nov 2023, Martin Storsjö wrote: Anyway, the concrete case I'm considering, is that we've got AArch64 code merged, that uses the I8MM extensions. We don't have any FATE configuration that continuously test that. Whenever there are patches, I do spin up a cloud instance that supports this extension and test the patches there, but inbetween that we're pretty much blind. While checkasm's coverage isn't fantastic, for this particular case I'm not merging any AArch64 code for new extensions unless that code is covered by checkasm. The other AArch64 feature that we do have code for, which also is untested, is the assembly support for branch protection and pointer authentication. Also this is testable pretty easily with QEMU. It's of course more interesting to run the full fate suite, but if we're not looking for bugs in the compiler but only for bugs in our assembly, then checkasm should cover most of it. So if you don't mind, I'd like to go ahead and push this, and set up those instances so that we do get some continuous testing of the corners of our aarch64 assembly that we otherwise only test occasionally. // Martin ___ 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 4/9] avutil: introduce an Immersive Audio Model and Formats API
Quoting James Almer (2023-11-26 02:28:53) > diff --git a/libavutil/iamf.h b/libavutil/iamf.h > new file mode 100644 > index 00..1f4919efdb > --- /dev/null > +++ b/libavutil/iamf.h > +enum AVIAMFAudioElementType { > +AV_IAMF_AUDIO_ELEMENT_TYPE_CHANNEL, > +AV_IAMF_AUDIO_ELEMENT_TYPE_SCENE, 'audio' in the names is redundant and makes already long identifiers unnecessarily longer > +}; > + > +/** > + * @defgroup lavf_iamf_params Parameter Definition > + * @{ > + * Parameters as defined in section 3.6.1 and 3.8 of what? > +/** > + * Mix Gain Parameter Data as defined in section 3.8.1 > + * > + * Subblocks in AVIAMFParamDefinition use this struct when the value or > + * @ref AVIAMFParamDefinition.param_definition_type param_definition_type is > + * AV_IAMF_PARAMETER_DEFINITION_MIX_GAIN. > + */ > +typedef struct AVIAMFMixGainParameterData { Does 'ParameterData' at the end really serve any purpose? > +const AVClass *av_class; > + > +// AVOption enabled fields > +unsigned int subblock_duration; > +enum AVIAMFAnimationType animation_type; > +AVRational start_point_value; > +AVRational end_point_value; > +AVRational control_point_value; > +unsigned int control_point_relative_time; All these should really be documented. Also, some vertical alignment would improve readability. > +/** > + * Parameters as defined in section 3.6.1 This really REALLY needs more documentation. > + */ > +typedef struct AVIAMFParamDefinition { > +const AVClass *av_class; > + > +size_t subblocks_offset; > +size_t subblock_size; > + > +enum AVIAMFParamDefinitionType param_definition_type; > +unsigned int num_subblocks; We use nb_foo generally. > +AVIAMFParamDefinition *av_iamf_param_definition_alloc(enum > AVIAMFParamDefinitionType param_definition_type, > +AVDictionary > **options, > +unsigned int > num_subblocks, AVDictionary **subblock_options, What are the dicts for? > + * > + * When audio_element_type is AV_IAMF_AUDIO_ELEMENT_TYPE_CHANNEL, this > + * corresponds to an Scalable Channel Layout layer as defined in section > 3.6.2. > + * For AV_IAMF_AUDIO_ELEMENT_TYPE_SCENE, it is an Ambisonics channel > + * layout as defined in section 3.6.3 > + */ > +typedef struct AVIAMFLayer { > +const AVClass *av_class; > + > +// AVOption enabled fields > +AVChannelLayout ch_layout; > + > +unsigned int recon_gain_is_present; Every time you dedicate 4 bytes to storing one bit, God kills a kitten. > +/** > + * Output gain flags as defined in section 3.6.2 It would be really really nice if people could understand the struct contents without some external document. > + * This field is defined only if audio_element_type is presumably the parent's audio_element_type > + * AV_IAMF_AUDIO_ELEMENT_TYPE_CHANNEL, must be 0 otherwise. > + */ > +unsigned int output_gain_flags; > +/** > + * Output gain as defined in section 3.6.2 > + * > + * Must be 0 if @ref output_gain_flags is 0. > + */ > +AVRational output_gain; > +/** > + * Ambisonics mode as defined in section 3.6.3 > + * > + * This field is defined only if audio_element_type is > + * AV_IAMF_AUDIO_ELEMENT_TYPE_SCENE, must be 0 otherwise. > + * > + * If 0, channel_mapping is defined implicitly (Ambisonic Order) > + * or explicitly (Custom Order with ambi channels) in @ref ch_layout. > + * If 1, @ref demixing_matrix must be set. > + */ > +enum AVIAMFAmbisonicsMode ambisonics_mode; > + > +// End of AVOption enabled fields What purpose does this comment serve? > +/** > + * Demixing matrix as defined in section 3.6.3 > + * > + * Set only if @ref ambisonics_mode == 1, must be NULL otherwise. > + */ > +AVRational *demixing_matrix; Who sets this? > +typedef struct AVIAMFAudioElement { > +const AVClass *av_class; > + > +AVIAMFLayer **layers; > +/** > + * Number of layers, or channel groups, in the Audio Element. > + * For audio_element_type AV_IAMF_AUDIO_ELEMENT_TYPE_SCENE, there > + * may be exactly 1. > + * > + * Set by av_iamf_audio_element_add_layer(), must not be > + * modified by any other code. > + */ > +unsigned int num_layers; > + > +unsigned int codec_config_id; ??? > +int av_iamf_audio_element_add_layer(AVIAMFAudioElement *audio_element, > AVDictionary **options); I would much prefer to have the caller call av_opt_set* manually rather than sprinkle AVDictionary function arguments everywhere. Do note that their usage in lavc and lavf APIs is out of necessity, not because it's very pretty. -- Anton Khirnov ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email
Re: [FFmpeg-devel] [PATCH 2/2] avcodec/amd: fix pict_type, match it with amf & ffmpeg. add sps pps when frame->keyframe
The idea itself looks good to me, but the patch is corrupted. Regarding the patch: 1) >>// Keyframe overrides previous assignment. It might be better instead of: setting the frame type -> if key frame then overwrite to do: check for key frame and if it's not, set other PICTURE_TYPE to avoid unnecessary overwriting (use AV_FRAME_FLAG_KEY instead of deprecated key_frame, as already noted under the second patch) 2) The patch includes logic for AV_CODEC_ID_H264 and AV_CODEC_ID_HEVC, but not for AV_CODEC_ID_AV1.( FORCE_INSERT_SEQUENCE_HEADER AMF_VIDEO_ENCODER_AV1_FORCE_FRAME_TYPE ) ___ 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 2/9] avcodec/get_bits: add get_leb()
> add get_leb() Do you expect people to understand what this means? -- Anton Khirnov ___ 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 1/9] avutil/mem: add av_dynarray2_add_nofree
> av_dynarray2_add_nofree This is getting ridiculous. Do we really need 4 separate dynarray_add functions? Wouldn't a single one with a flags argument do the job? -- Anton Khirnov ___ 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".
[FFmpeg-devel] [PATCH] lavc/dvdsubenc: only check canvas size when it is actually set
Fixes #10650 --- libavcodec/dvdsubenc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libavcodec/dvdsubenc.c b/libavcodec/dvdsubenc.c index d272b57675..06c2cf5e5a 100644 --- a/libavcodec/dvdsubenc.c +++ b/libavcodec/dvdsubenc.c @@ -376,7 +376,8 @@ static int encode_dvd_subtitles(AVCodecContext *avctx, x2 = vrect.x + vrect.w - 1; y2 = vrect.y + vrect.h - 1; -if (x2 > avctx->width || y2 > avctx->height) { +if ((avctx->width > 0 && x2 > avctx->width) || +(avctx->height > 0 && y2 > avctx->height)) { av_log(avctx, AV_LOG_ERROR, "canvas_size(%d:%d) is too small(%d:%d) for render\n", avctx->width, avctx->height, x2, y2); ret = AVERROR(EINVAL); -- 2.42.0 ___ 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] [ANNOUNCE] upcoming vote: TC/CC elections
Quoting Thilo Borgmann via ffmpeg-devel (2023-11-29 13:22:11) > > > On 28.11.23 21:30, Derek Buitenhuis wrote: > > On 11/28/2023 3:50 PM, Anton Khirnov wrote: > >> Calling things generically bad is the opposite of helpful. > > I cannot offer help on making a paragraph that I don't fully > > understand become more comprehensible, as that would require > > I understand it fully. > > > > But, I would again like to state these votes should be scrapped > > and redone. People literally voted the opposite of what they wanted > > to by accident, due to this. > > > > FWIW the type in of weights is one of the two options to do a > proportional representation for the vote. > The other is the one we had used so far, by ranking the candidates from > 1st to n-th. > > Both should serve our needs for proportional representation AFAICT and I > don't assume they'd give us different results of the vote. But maybe > Anton had a reason to pick one over the other. As per the official documentation https://civs1.civs.us/proportional.html Combined weights. In combined-weights mode, the voter gives a nonnegative weight to each candidate instead of ranking the candidates. The voter's goal is to maximize the sum of weights of selected candidates. This is an appropriate criterion for elections where the quality of all the candidates is is important to the voters, such as the election of an actual committee that will be voting on some issues. [...] Best candidate. In best-candidate mode, the voter's goal is to get a single very good candidate elected, and the quality of other elected candidates is a strictly secondary consideration. This is appropriate for an election where the voter really only cares about the best candidate[...] -- Anton Khirnov ___ 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] [ANNOUNCE] upcoming vote: TC/CC elections
Quoting Derek Buitenhuis (2023-11-28 21:30:12) > On 11/28/2023 3:50 PM, Anton Khirnov wrote: > > Calling things generically bad is the opposite of helpful. > > I cannot offer help on making a paragraph that I don't fully > understand become more comprehensible, as that would require > I understand it fully. Consider the following: 1) I am neither a UI nor a voting system expert. 2) Explaining complex topics (like sophisticated voting systems) in a clear and easy to understand way is a very rare and highly nontrivial skill. 3) If you want an explanation that can be understood with no effort from your side, the best I can give you is: higher number = more preference (which is in the text already). 4) As this is a collaborative project, you are very much welcome to read the official documentation and supply a better explanation yourself, or find someone else who does. 5) As you're doing neither of these, your "word soup" and "trainwreck" amount just to your being an asshole to no purpose. > But, I would again like to state these votes should be scrapped > and redone. People literally voted the opposite of what they wanted > to by accident, due to this. So far we have exactly one person to whom this happened, who has notably NOT asked to redo the vote yet. While I don't oppose redoing the vote in principle, I also don't think it's realistic to expect a vote with 50+ voters to run with zero issues. Voting fatigue is also a real thing, so we risk some people not voting at all in hypothetical repeat votes. -- Anton Khirnov ___ 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".