[FFmpeg-devel] [PATCH 1/2] fftools/ffmpeg_filter: Fix audio_drift_threshold check

[FFmpeg-devel] [PATCH 1/2] fftools/ffmpeg_filter: Fix audio_drift_threshold check

[Michael Niedermayer]

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

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 2/2] fftools/ffmpeg_filter: Fix audio_drift_threshold check

[Michael Niedermayer]

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

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 2/2] fftools/ffmpeg_filter: Fix audio_drift_threshold check

[Anton Khirnov]

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
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 2/2] fftools/ffmpeg_filter: Fix audio_drift_threshold check

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 555 bytes --]

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

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 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-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 2/2] fftools/ffmpeg_filter: Fix audio_drift_threshold check

[Anton Khirnov]

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
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 2/2] fftools/ffmpeg_filter: Fix audio_drift_threshold check

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 1057 bytes --]

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

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 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-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 2/2] fftools/ffmpeg_filter: Fix audio_drift_threshold check

[Anton Khirnov]

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
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 2/2] fftools/ffmpeg_filter: Fix audio_drift_threshold check

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 1803 bytes --]

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.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 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-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 2/2] fftools/ffmpeg_filter: Fix audio_drift_threshold check

[Anton Khirnov]

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
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".