Re: [FFmpeg-devel] [PATCH 2/4] af_silencedetect : fix accuracy of silence_start

[Michael Niedermayer]

On Mon, Feb 05, 2018 at 02:20:46PM +, Gaullier Nicolas wrote:
> >this breaks fate
> >
> >If the changes are intended the reference must be updated by the patch 
> >causing the changes
> 
> Sorry, forgot... Now, I have some questions regarding fate tests:
> 
> 1) I would like to update the fate test itself :
> Currently, we have : silencedetect=d=-20dB
> I am considering changing it to : silencedetect=n=-30dB:d=.4
> The reason is that the usage would be more relevant (dB applying to noise + 
> duration set to a consistent value for this speech sample), easier to check 
> manually in a waveform editor, and that the coverage would be extended for 
> the new patches (silence_start=0 + log of silence_end at end of stream).
> Should I first publish the patch with only the fate results changed and later 
> on another patch to update the fate test with results changed again ?
> Personally, I would say a single patch with all three items (patch + fate 
> test update + fate result update) would be clearer, but I am not familiar 
> with ffmpeg usages, so I prefer asking...
> 

> 2) I just realized today that I could fix the accuracy of silence_end too, 
> even if it is clearly much less important compared to silence_start
> Do you think this should be handled by another patch, or should I better 
> group this patch with this one as they both deal with time accuracy and 
> affect fate results (I would rather go for the latter) ?

each independant change should be in a seperate patch


> 
> 3) Should I prepare a new fate test to cover the new "mono" mode (patch 1/4) ?

if you like, yes

more complete test coverage is always good

thanks

[...]

-- 
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
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH 2/4] af_silencedetect : fix accuracy of silence_start

[Gaullier Nicolas]

>this breaks fate
>
>If the changes are intended the reference must be updated by the patch causing 
>the changes

Sorry, forgot... Now, I have some questions regarding fate tests:

1) I would like to update the fate test itself :
Currently, we have : silencedetect=d=-20dB
I am considering changing it to : silencedetect=n=-30dB:d=.4
The reason is that the usage would be more relevant (dB applying to noise + 
duration set to a consistent value for this speech sample), easier to check 
manually in a waveform editor, and that the coverage would be extended for the 
new patches (silence_start=0 + log of silence_end at end of stream).
Should I first publish the patch with only the fate results changed and later 
on another patch to update the fate test with results changed again ?
Personally, I would say a single patch with all three items (patch + fate test 
update + fate result update) would be clearer, but I am not familiar with 
ffmpeg usages, so I prefer asking...

2) I just realized today that I could fix the accuracy of silence_end too, even 
if it is clearly much less important compared to silence_start
Do you think this should be handled by another patch, or should I better group 
this patch with this one as they both deal with time accuracy and affect fate 
results (I would rather go for the latter) ?

3) Should I prepare a new fate test to cover the new "mono" mode (patch 1/4) ?

Thank you!
Nicolas Gaullier
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 2/4] af_silencedetect : fix accuracy of silence_start

[Michael Niedermayer]

On Fri, Feb 02, 2018 at 11:01:20AM +, Gaullier Nicolas wrote:
> (attachement + email object fixed : sorry about that x2)
> Attached patch fixes accuracy of silence_start.
> The benefit is mostly noticeable when the silence starts at the very 
> beginning (ie. silence_start=0 exactly).
> Nicolas Gaullier

>  af_silencedetect.c |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 0847308d6116f153553a9dab288eecaf8804cfe6  
> 0002-avfilter-af_silencedetect-fix-silence_start-accuracy.patch
> From a8780b9c7132b80cd0f0f3cb99fc34bf38874f34 Mon Sep 17 00:00:00 2001
> From: nicolas gaullier 
> Date: Fri, 2 Feb 2018 11:14:47 +0100
> Subject: [PATCH 2/4] avfilter/af_silencedetect : fix silence_start accuracy

this breaks fate

make -j12 fate-filter-metadata-silencedetect
TESTfilter-metadata-silencedetect
--- ./tests/ref/fate/filter-metadata-silencedetect  2018-02-03 
00:37:40.522808340 +0100
+++ tests/data/fate/filter-metadata-silencedetect   2018-02-03 
02:57:28.310985048 +0100
@@ -4,9 +4,9 @@
 pkt_pts=960
 pkt_pts=1280
 pkt_pts=1600
-pkt_pts=1920|tag:lavfi.silence_start=0.02
+pkt_pts=1920|tag:lavfi.silence_start=0.0351875
 pkt_pts=2240
-pkt_pts=2560|tag:lavfi.silence_end=0.16|tag:lavfi.silence_duration=0.14
+pkt_pts=2560|tag:lavfi.silence_end=0.16|tag:lavfi.silence_duration=0.124813
 pkt_pts=2880
 pkt_pts=3200
 pkt_pts=3520


If the changes are intended the reference must be updated by the patch causing
the changes

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The misfortune of the wise is better than the prosperity of the fool.
-- Epicurus


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH 2/4] af_silencedetect : fix accuracy of silence_start

[Gaullier Nicolas]

(attachement + email object fixed : sorry about that x2)
Attached patch fixes accuracy of silence_start.
The benefit is mostly noticeable when the silence starts at the very beginning 
(ie. silence_start=0 exactly).
Nicolas Gaullier


0002-avfilter-af_silencedetect-fix-silence_start-accuracy.patch
Description: 0002-avfilter-af_silencedetect-fix-silence_start-accuracy.patch
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH 2/4] af_silencedetect : fix accuracy of silence_start)

[Gaullier Nicolas]

(email object fixed : sorry about that)
Attached patch fixes accuracy of silence_start.
The benefit is mostly noticeable when the silence starts at the very beginning 
(ie. silence_start=0 exactly).
Nicolas Gaullier
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel