Re: [FFmpeg-devel] [PATCH] Fix loss of precision for silencedetecton large files
On Wednesday, March 27, 2019, 11:19:23 AM PDT, Michael Niedermayer wrote: > this breaks make fate> > also if fate is updated it should be ensured it still checks enough precission> and that it does produce the same results (fate passes) on all relevant> platforms. This change may bring non significant differences into significance> > thanks OK so I spent some more time familiarizing myself with the code and tests, and it's clear that making the change the way I did would probably break dozens of tests, because the function I changed, av_ts_make_time_string, is referenced (by its convenience macro av_ts2timestr) in many files (I count 21) and dozens of lines of code, in libavfilter, fftools, and libavformat. As an alternative, I tried making a modified & renamed copy of av_ts_make_time_string/av_ts2timestr, and referencing that copy in af_silencedetect.c, leaving other references to the original function unchanged. In this case, FATE fails just a single test, filter-metadata-silencedetect. I then modified the reference output file (tests/ref/fate/filter-metadata-silencedetect) to match the changed output, and all tests then pass. So at this point I could use some guidance from the wizards as to the best way to proceed. Not knowing any of the history or thinking that went into the design, I don't know if it makes the best sense to use the "%.6g" format for all those other timestamps, but I feel strongly that it's a poor choice for silencedetect (and probably other *detect filters), for the reason I explained originally. Is there possibly another way to get the result I want without modifying the code? Or I can imagine maybe a format specifier as a filter parameter -- is there any precedent for this? Open to suggestions. Thanks all. Allan ___ 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] Fix loss of precision for silencedetecton large files
On 27.03.2019 23:13, Allan Cady via ffmpeg-devel wrote: On Tue, Mar 26, 2019 at 10:07:10PM +, Allan Cady via ffmpeg-devel wrote: When the silencedetect filter is run against very large files, the output timestamps gradually lose precision as the scan proceeds further into the file. This is because the output is formatted (in libavutil/timestamp.h) as "%.6g", which limits the total field length. Eventually, for offsets greater than 10 seconds (about 28 hours), fractions of a second disappear altogether, and the timestamps are logged as whole integers. This is insufficient precision for my purposes. I propose changing the format to "%.3f", which will give millisecond precision for all timestamps regardless of offset. I think it would be nice if some of this description would be included into the patch commit message. The header line is misleading in focusing on silencedetect when the change is actually done in (a)metadata filter. Also usual commit header line convention is to start with the affected module (like "avutil/timestamp: ..."). First glance it looks like there are pre-built reference output files in the tests that may need to be re-generated, and that may be all that's required. I'll continue digging to try to understand what's going on. You might want to look into "make fate GEN=1". See http://ffmpeg.org/fate.html#FATE-makefile-targets-and-variables for details. Regards, Tobias ___ 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] Fix loss of precision for silencedetecton large files
2019-03-27 23:13 GMT+01:00, Allan Cady via ffmpeg-devel : > On Tue, Mar 26, 2019 at 10:07:10PM +, Allan Cady via ffmpeg-devel > wrote: > >> When the silencedetect filter is run against very large files, the >> output timestamps gradually lose precision as the scan proceeds >> further into the file. This is because the output is formatted (in >> libavutil/timestamp.h) as "%.6g", which limits the total field length. >> Eventually, for offsets greater than 10 seconds (about 28 hours), >> fractions of a second disappear altogether, and the timestamps >> are logged as whole integers. This is insufficient precision for >> my purposes. >> >> I propose changing the format to "%.3f", which will give millisecond >> precision for all timestamps regardless of offset. >> >> Patch file is attached. >> >> EXAMPLE >> >> For a sample scan on a file that's about 35 hours (126000 seconds) length: >> >> $ ffmpeg -i input.mp3 -filter_complex >> silencedetect=n=-30dB:d=3,ametadata=mode=print:file=silence-out.txt -f >> null - >> >> The output looks like this near the beginning: >> >> frame:83085 pts:47856431 pts_time:2170.36 >> lavfi.silence_start=2166.86 >> frame:83139 pts:47887535 pts_time:2171.77 >> lavfi.silence_end=2171.77 >> lavfi.silence_duration=4.91061 >> >> Further on we get this: >> >> frame:2450348 pts:1411399919 pts_time:64009.1 >> lavfi.silence_start=64005.6 >> frame:2450371 pts:1411413167 pts_time:64009.7 >> lavfi.silence_end=64009.7 >> lavfi.silence_duration=4.10082 >> >> Eventually after it passes 10 seconds: >> >> frame:4738029 pts:2729104175 pts_time:123769 >> lavfi.silence_start=123765 >> frame:4738055 pts:2729119151 pts_time:123770 >> lavfi.silence_end=123770 >> lavfi.silence_duration=4.17918 >> >> The start and end times are now in whole integers (seconds). >> >> After making this patch and building the code myself, I now get this >> output near the end: >> >> frame:4738029 pts:2729104175 pts_time:123768.897 >> lavfi.silence_start=123765.411 >> frame:4738055 pts:2729119151 pts_time:123769.576 >> lavfi.silence_end=123769.584 >> lavfi.silence_duration=4.173 >> >> This gives me the output I want. >> >> Thank you, >> >> Allan Cady >> Seattle WA > >> timestamp.h |2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> a997e2f7d02d14a3761cf389b8096e55f3670fb5 >> 0001-Fix-loss-of-precision-for-silencedetect-on-large-fil.patch >> From 59b82d49516926173ab03944a73fd7fc9e5d7bcc Mon Sep 17 00:00:00 2001 >> From: Allan Cady >> Date: Tue, 26 Mar 2019 14:11:03 -0700 >> Subject: [PATCH] Fix loss of precision for silencedetect on large files > > > On Wednesday, March 27, 2019, 11:19:23 AM PDT, Michael Niedermayer > wrote: > > this breaks make fate > > also if fate is updated it should be ensured it still checks enough > precission > and that it does produce the same results (fate passes) on all relevant > platforms. This change may bring non significant differences into > significance > > thanks > > [...] > -- > MichaelGnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > Thank you Michael for moving this forward. I've been able to reproduce the > failure for myself and I'm starting to look into how big a deal it will be. > First glance it looks like there are pre-built reference output files in the > tests that may need to be re-generated, and that may be all that's required. > I'll continue digging to try to understand what's going on. > > I'm brand new here and not very experienced with open source > collaboration... any hints how to proceed or corrections if I break protocol > would be appreciated. So far so good though it seems. I didn't look at this specific test but if the reference files are downloaded you cannot replace them (in the repository), you can only add new reference files. Please fix your quoting, Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] Fix loss of precision for silencedetecton large files
On Tue, Mar 26, 2019 at 10:07:10PM +, Allan Cady via ffmpeg-devel wrote: > When the silencedetect filter is run against very large files, the > output timestamps gradually lose precision as the scan proceeds > further into the file. This is because the output is formatted (in > libavutil/timestamp.h) as "%.6g", which limits the total field length. > Eventually, for offsets greater than 10 seconds (about 28 hours), > fractions of a second disappear altogether, and the timestamps > are logged as whole integers. This is insufficient precision for > my purposes. > > I propose changing the format to "%.3f", which will give millisecond > precision for all timestamps regardless of offset. > > Patch file is attached. > > EXAMPLE > > For a sample scan on a file that's about 35 hours (126000 seconds) length: > > $ ffmpeg -i input.mp3 -filter_complex > silencedetect=n=-30dB:d=3,ametadata=mode=print:file=silence-out.txt -f null - > > The output looks like this near the beginning: > > frame:83085 pts:47856431 pts_time:2170.36 > lavfi.silence_start=2166.86 > frame:83139 pts:47887535 pts_time:2171.77 > lavfi.silence_end=2171.77 > lavfi.silence_duration=4.91061 > > Further on we get this: > > frame:2450348 pts:1411399919 pts_time:64009.1 > lavfi.silence_start=64005.6 > frame:2450371 pts:1411413167 pts_time:64009.7 > lavfi.silence_end=64009.7 > lavfi.silence_duration=4.10082 > > Eventually after it passes 10 seconds: > > frame:4738029 pts:2729104175 pts_time:123769 > lavfi.silence_start=123765 > frame:4738055 pts:2729119151 pts_time:123770 > lavfi.silence_end=123770 > lavfi.silence_duration=4.17918 > > The start and end times are now in whole integers (seconds). > > After making this patch and building the code myself, I now get this > output near the end: > > frame:4738029 pts:2729104175 pts_time:123768.897 > lavfi.silence_start=123765.411 > frame:4738055 pts:2729119151 pts_time:123769.576 > lavfi.silence_end=123769.584 > lavfi.silence_duration=4.173 > > This gives me the output I want. > > Thank you, > > Allan Cady > Seattle WA > timestamp.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > a997e2f7d02d14a3761cf389b8096e55f3670fb5 > 0001-Fix-loss-of-precision-for-silencedetect-on-large-fil.patch > From 59b82d49516926173ab03944a73fd7fc9e5d7bcc Mon Sep 17 00:00:00 2001 > From: Allan Cady > Date: Tue, 26 Mar 2019 14:11:03 -0700 > Subject: [PATCH] Fix loss of precision for silencedetect on large files On Wednesday, March 27, 2019, 11:19:23 AM PDT, Michael Niedermayer wrote: this breaks make fate also if fate is updated it should be ensured it still checks enough precission and that it does produce the same results (fate passes) on all relevant platforms. This change may bring non significant differences into significance thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Thank you Michael for moving this forward. I've been able to reproduce the failure for myself and I'm starting to look into how big a deal it will be. First glance it looks like there are pre-built reference output files in the tests that may need to be re-generated, and that may be all that's required. I'll continue digging to try to understand what's going on. I'm brand new here and not very experienced with open source collaboration... any hints how to proceed or corrections if I break protocol would be appreciated. So far so good though it seems. ___ 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".
