[PATCH 1/2] fftools/ffmpeg_filter: Fix audio_drift_threshold check
Variant 1 Found-by: mkver Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- fftools/ffmpeg_filter.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c index 0845c631a5..048815170a 100644 --- a/fftools/ffmpeg_filter.c +++ b/fftools/ffmpeg_filter.c @@ -891,7 +891,7 @@ static int configure_input_audio_filter(FilterGraph *fg, InputFilter *ifilter, char args[256] = {0}; av_strlcatf(args, sizeof(args), "async=%d", audio_sync_method); - if (audio_drift_threshold != 0.1) + if (audio_drift_threshold != (float)0.1) av_strlcatf(args, sizeof(args), ":min_hard_comp=%f", audio_drift_threshold); if (!fg->reconfiguration) av_strlcatf(args, sizeof(args), ":first_pts=0"); -- 2.17.1
Variant 2 Found-by: mkver Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- fftools/ffmpeg_filter.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c index 0845c631a5..a204c4f65f 100644 --- a/fftools/ffmpeg_filter.c +++ b/fftools/ffmpeg_filter.c @@ -18,6 +18,7 @@ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA */ +#include <float.h> #include <stdint.h> #include "ffmpeg.h" @@ -891,7 +892,7 @@ static int configure_input_audio_filter(FilterGraph *fg, InputFilter *ifilter, char args[256] = {0}; av_strlcatf(args, sizeof(args), "async=%d", audio_sync_method); - if (audio_drift_threshold != 0.1) + if (fabs(audio_drift_threshold - 0.1) > FLT_EPSILON) av_strlcatf(args, sizeof(args), ":min_hard_comp=%f", audio_drift_threshold); if (!fg->reconfiguration) av_strlcatf(args, sizeof(args), ":first_pts=0"); -- 2.17.1
Variant 2 is less bad, but the whole check seems hacky to me, since it seems to make assumptions about swr defaults Won't setting this unconditionally have the same effect? -- Anton Khirnov
On Thu, Jun 30, 2022 at 10:55:46AM +0200, Anton Khirnov wrote:
Variant 2 is less bad, but the whole check seems hacky to me, since it seems to make assumptions about swr defaults
Won't setting this unconditionally have the same effect?
it has the same effect but its not so nice to the user to recommand extra arguments which make no difference thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Republics decline into democracies and democracies degenerate into despotisms. -- Aristotle
Quoting Michael Niedermayer (2022-07-01 21:53:00)
On Thu, Jun 30, 2022 at 10:55:46AM +0200, Anton Khirnov wrote:
Variant 2 is less bad, but the whole check seems hacky to me, since it seems to make assumptions about swr defaults
Won't setting this unconditionally have the same effect?
it has the same effect but its not so nice to the user to recommand extra arguments which make no difference
Sorry, I don't follow. What is recommending any arguments to the user here? -- Anton Khirnov
On Sat, Jul 02, 2022 at 10:38:26AM +0200, Anton Khirnov wrote:
Quoting Michael Niedermayer (2022-07-01 21:53:00)
On Thu, Jun 30, 2022 at 10:55:46AM +0200, Anton Khirnov wrote:
Variant 2 is less bad, but the whole check seems hacky to me, since it seems to make assumptions about swr defaults
Won't setting this unconditionally have the same effect?
it has the same effect but its not so nice to the user to recommand extra arguments which make no difference
Sorry, I don't follow. What is recommending any arguments to the user here?
i meant this thing here: ./ffmpeg -i matrixbench_mpeg2.mpg -async 1 -f null - 2>&1 | grep async -async is forwarded to lavfi similarly to -af aresample=async=1:min_hard_comp=0.100000:first_pts=0. vs. -async is forwarded to lavfi similarly to -af aresample=async=1:first_pts=0. [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Observe your enemies, for they first find out your faults. -- Antisthenes
Quoting Michael Niedermayer (2022-07-02 14:56:50)
On Sat, Jul 02, 2022 at 10:38:26AM +0200, Anton Khirnov wrote:
Quoting Michael Niedermayer (2022-07-01 21:53:00)
On Thu, Jun 30, 2022 at 10:55:46AM +0200, Anton Khirnov wrote:
Variant 2 is less bad, but the whole check seems hacky to me, since it seems to make assumptions about swr defaults
Won't setting this unconditionally have the same effect?
it has the same effect but its not so nice to the user to recommand extra arguments which make no difference
Sorry, I don't follow. What is recommending any arguments to the user here?
i meant this thing here: ./ffmpeg -i matrixbench_mpeg2.mpg -async 1 -f null - 2>&1 | grep async
-async is forwarded to lavfi similarly to -af aresample=async=1:min_hard_comp=0.100000:first_pts=0.
vs.
-async is forwarded to lavfi similarly to -af aresample=async=1:first_pts=0.
I don't see a problem - why would the user care how it is forwarded? -- Anton Khirnov
On Tue, Jul 05, 2022 at 06:58:28PM +0200, Anton Khirnov wrote:
Quoting Michael Niedermayer (2022-07-02 14:56:50)
On Sat, Jul 02, 2022 at 10:38:26AM +0200, Anton Khirnov wrote:
Quoting Michael Niedermayer (2022-07-01 21:53:00)
On Thu, Jun 30, 2022 at 10:55:46AM +0200, Anton Khirnov wrote:
Variant 2 is less bad, but the whole check seems hacky to me, since it seems to make assumptions about swr defaults
Won't setting this unconditionally have the same effect?
it has the same effect but its not so nice to the user to recommand extra arguments which make no difference
Sorry, I don't follow. What is recommending any arguments to the user here?
i meant this thing here: ./ffmpeg -i matrixbench_mpeg2.mpg -async 1 -f null - 2>&1 | grep async
-async is forwarded to lavfi similarly to -af aresample=async=1:min_hard_comp=0.100000:first_pts=0.
vs.
-async is forwarded to lavfi similarly to -af aresample=async=1:first_pts=0.
I don't see a problem - why would the user care how it is forwarded?
The user may want to perform the equivalent operation inside a filter chain/graph or with a tool different from ffmpeg or even via the API its really a very minor thing if these defaults are displayed too, it just feels a bit cleaner to skip them thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB If you drop bombs on a foreign country and kill a hundred thousand innocent people, expect your government to call the consequence "unprovoked inhuman terrorist attacks" and use it to justify dropping more bombs and killing more people. The technology changed, the idea is old.
Quoting Michael Niedermayer (2022-07-06 00:36:25)
On Tue, Jul 05, 2022 at 06:58:28PM +0200, Anton Khirnov wrote:
Quoting Michael Niedermayer (2022-07-02 14:56:50)
On Sat, Jul 02, 2022 at 10:38:26AM +0200, Anton Khirnov wrote:
Quoting Michael Niedermayer (2022-07-01 21:53:00)
On Thu, Jun 30, 2022 at 10:55:46AM +0200, Anton Khirnov wrote:
Variant 2 is less bad, but the whole check seems hacky to me, since it seems to make assumptions about swr defaults
Won't setting this unconditionally have the same effect?
it has the same effect but its not so nice to the user to recommand extra arguments which make no difference
Sorry, I don't follow. What is recommending any arguments to the user here?
i meant this thing here: ./ffmpeg -i matrixbench_mpeg2.mpg -async 1 -f null - 2>&1 | grep async
-async is forwarded to lavfi similarly to -af aresample=async=1:min_hard_comp=0.100000:first_pts=0.
vs.
-async is forwarded to lavfi similarly to -af aresample=async=1:first_pts=0.
I don't see a problem - why would the user care how it is forwarded?
The user may want to perform the equivalent operation inside a filter chain/graph or with a tool different from ffmpeg or even via the API its really a very minor thing if these defaults are displayed too, it just feels a bit cleaner to skip them
Then the correct thing to do is retrieve the swr default value via the AVOption API. Though IMO it's extra complexity for marginal gain. I also wonder if this option should exist at all, given that it does nothing but set a swr option. It is also global, so you can't have different values for different streams. -- Anton Khirnov
participants (2)
-
Anton Khirnov -
Michael Niedermayer