diff mbox series

[FFmpeg-devel,01/14] Revert "avfilter/af_silenceremove: fix processing of periods > 1"

Message ID PR3PR03MB6665FF7FFF5FBAAA50288AE48FD59@PR3PR03MB6665.eurprd03.prod.outlook.com
State Withdrawn
Headers show
Series [FFmpeg-devel,01/14] Revert "avfilter/af_silenceremove: fix processing of periods > 1" | expand

Checks

Context Check Description
andriy/commit_msg_x86 warning The first line of the commit message must start with a context terminated by a colon and a space, for example "lavu/opt: " or "doc: ".
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished
andriy/commit_msg_ppc warning The first line of the commit message must start with a context terminated by a colon and a space, for example "lavu/opt: " or "doc: ".
andriy/make_ppc success Make finished
andriy/make_fate_ppc success Make fate finished

Commit Message

Andreas Rheinhardt Sept. 9, 2021, 3:50 p.m. UTC
This reverts commits 3b331468dae2e88ee6c87c257ac159ad662efcac
and 8a42ee6697317d0a30438df5905dfc0247cd28e7. They broke
the filter-silenceremove FATE-test, as they were pushed without
updating the reference file. Worse, the new output is not bitexact
across all arches, as the diff on aarch64 is different; see e.g.
http://fate.ffmpeg.org/report.cgi?slot=aarch64-linux-clang-10&time=20210909013452
and
http://fate.ffmpeg.org/report.cgi?time=20210909095918&slot=x86_64-archlinux-gcc-threads-16

So revert the commits for now until this can be properly addressed.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
I hope that this patch needn't be applied and include it here mainly
so that patchwork won't be red for the other patches of this patchset.

 libavfilter/af_silenceremove.c | 79 ++++++++++------------------------
 1 file changed, 23 insertions(+), 56 deletions(-)
diff mbox series

Patch

diff --git a/libavfilter/af_silenceremove.c b/libavfilter/af_silenceremove.c
index 6f3250bbb1..2f376eb1ca 100644
--- a/libavfilter/af_silenceremove.c
+++ b/libavfilter/af_silenceremove.c
@@ -23,7 +23,6 @@ 
 
 #include <float.h> /* DBL_MAX */
 
-#include "libavutil/audio_fifo.h"
 #include "libavutil/avassert.h"
 #include "libavutil/opt.h"
 #include "libavutil/timestamp.h"
@@ -94,8 +93,6 @@  typedef struct SilenceRemoveContext {
     int64_t window_duration;
     double sum;
 
-    int threshold;
-    int one_period;
     int restart;
     int64_t next_pts;
 
@@ -104,22 +101,20 @@  typedef struct SilenceRemoveContext {
     double (*compute)(struct SilenceRemoveContext *s, AVFrame *frame, int ch, int offset);
     void (*copy)(struct SilenceRemoveContext *s, AVFrame *out, AVFrame *in,
                  int ch, int out_offset, int in_offset);
-
-    AVAudioFifo *fifo;
 } SilenceRemoveContext;
 
 #define OFFSET(x) offsetof(SilenceRemoveContext, x)
 #define AF AV_OPT_FLAG_FILTERING_PARAM|AV_OPT_FLAG_AUDIO_PARAM
 
 static const AVOption silenceremove_options[] = {
-    { "start_periods",   "set periods of silence parts to skip from start",    OFFSET(start_periods),       AV_OPT_TYPE_INT,      {.i64=0},     0,      9000, AF },
+    { "start_periods",   NULL,                                                 OFFSET(start_periods),       AV_OPT_TYPE_INT,      {.i64=0},     0,      9000, AF },
     { "start_duration",  "set start duration of non-silence part",             OFFSET(start_duration_opt),  AV_OPT_TYPE_DURATION, {.i64=0},     0, INT32_MAX, AF },
     { "start_threshold", "set threshold for start silence detection",          OFFSET(start_threshold),     AV_OPT_TYPE_DOUBLE,   {.dbl=0},     0,   DBL_MAX, AF },
     { "start_silence",   "set start duration of silence part to keep",         OFFSET(start_silence_opt),   AV_OPT_TYPE_DURATION, {.i64=0},     0, INT32_MAX, AF },
     { "start_mode",      "set which channel will trigger trimming from start", OFFSET(start_mode),          AV_OPT_TYPE_INT,      {.i64=T_ANY}, T_ANY, T_ALL, AF, "mode" },
     {   "any",           0,                                                    0,                           AV_OPT_TYPE_CONST,    {.i64=T_ANY}, 0,         0, AF, "mode" },
     {   "all",           0,                                                    0,                           AV_OPT_TYPE_CONST,    {.i64=T_ALL}, 0,         0, AF, "mode" },
-    { "stop_periods",    "set periods of silence parts to skip from end",      OFFSET(stop_periods),        AV_OPT_TYPE_INT,      {.i64=0}, -9000,      9000, AF },
+    { "stop_periods",    NULL,                                                 OFFSET(stop_periods),        AV_OPT_TYPE_INT,      {.i64=0}, -9000,      9000, AF },
     { "stop_duration",   "set stop duration of non-silence part",              OFFSET(stop_duration_opt),   AV_OPT_TYPE_DURATION, {.i64=0},     0, INT32_MAX, AF },
     { "stop_threshold",  "set threshold for stop silence detection",           OFFSET(stop_threshold),      AV_OPT_TYPE_DOUBLE,   {.dbl=0},     0,   DBL_MAX, AF },
     { "stop_silence",    "set stop duration of silence part to keep",          OFFSET(stop_silence_opt),    AV_OPT_TYPE_DURATION, {.i64=0},     0, INT32_MAX, AF },
@@ -435,7 +430,6 @@  static int config_input(AVFilterLink *inlink)
     AVFilterContext *ctx = inlink->dst;
     SilenceRemoveContext *s = ctx->priv;
 
-    s->threshold = -1;
     s->next_pts = AV_NOPTS_VALUE;
     s->window_duration = av_rescale(s->window_duration_opt, inlink->sample_rate,
                                    AV_TIME_BASE);
@@ -545,16 +539,12 @@  static int config_input(AVFilterLink *inlink)
         return AVERROR_BUG;
     }
 
-    s->fifo = av_audio_fifo_alloc(inlink->format, inlink->channels, 1024);
-    if (!s->fifo)
-        return AVERROR(ENOMEM);
-
     return 0;
 }
 
 static void flush(SilenceRemoveContext *s,
                   AVFrame *out, AVFilterLink *outlink,
-                  int *nb_samples_written, int flush_silence)
+                  int *nb_samples_written, int *ret, int flush_silence)
 {
     AVFrame *silence;
 
@@ -566,18 +556,22 @@  static void flush(SilenceRemoveContext *s,
                                     (AVRational){1, outlink->sample_rate},
                                     outlink->time_base);
 
-        av_audio_fifo_write(s->fifo, (void **)out->extended_data, out->nb_samples);
+        *ret = ff_filter_frame(outlink, out);
+        if (*ret < 0)
+            return;
         *nb_samples_written = 0;
+    } else {
+        av_frame_free(&out);
     }
 
-    av_frame_free(&out);
-
     if (s->stop_silence_end <= 0 || !flush_silence)
         return;
 
     silence = ff_get_audio_buffer(outlink, s->stop_silence_end);
-    if (!silence)
+    if (!silence) {
+        *ret = AVERROR(ENOMEM);
         return;
+    }
 
     if (s->stop_silence_offset < s->stop_silence_end) {
         av_samples_copy(silence->extended_data, s->stop_silence_hold->extended_data, 0,
@@ -601,8 +595,7 @@  static void flush(SilenceRemoveContext *s,
                                 (AVRational){1, outlink->sample_rate},
                                 outlink->time_base);
 
-    av_audio_fifo_write(s->fifo, (void **)silence->extended_data, silence->nb_samples);
-    av_frame_free(&silence);
+    *ret = ff_filter_frame(outlink, silence);
 }
 
 static int filter_frame(AVFilterLink *inlink, AVFrame *in)
@@ -610,8 +603,8 @@  static int filter_frame(AVFilterLink *inlink, AVFrame *in)
     AVFilterContext *ctx = inlink->dst;
     AVFilterLink *outlink = ctx->outputs[0];
     SilenceRemoveContext *s = ctx->priv;
-    int nbs, nb_samples_read, nb_samples_written;
     int i, j, threshold, ret = 0;
+    int nbs, nb_samples_read, nb_samples_written;
     AVFrame *out;
 
     nb_samples_read = nb_samples_written = 0;
@@ -639,10 +632,6 @@  silence_trim:
                 }
             }
 
-            if (s->threshold >= 0)
-                s->one_period = s->threshold != threshold;
-            s->threshold = threshold;
-
             if (threshold) {
                 for (j = 0; j < outlink->channels; j++) {
                     s->update(s, in, j, nb_samples_read);
@@ -656,8 +645,7 @@  silence_trim:
                 nb_samples_read++;
 
                 if (s->start_holdoff_end >= s->start_duration) {
-                    s->start_found_periods += s->one_period;
-                    if (s->start_found_periods >= s->start_periods) {
+                    if (++s->start_found_periods >= s->start_periods) {
                         s->mode = SILENCE_TRIM_FLUSH;
                         goto silence_trim_flush;
                     }
@@ -731,8 +719,7 @@  silence_trim_flush:
 
         s->start_holdoff_offset += nbs;
 
-        av_audio_fifo_write(s->fifo, (void **)out->extended_data, out->nb_samples);
-        av_frame_free(&out);
+        ret = ff_filter_frame(outlink, out);
 
         if (s->start_holdoff_offset == s->start_holdoff_end) {
             s->start_holdoff_offset = 0;
@@ -770,13 +757,9 @@  silence_copy:
                     }
                 }
 
-                if (s->threshold >= 0)
-                    s->one_period = s->threshold != threshold;
-                s->threshold = threshold;
-
                 if (threshold && s->stop_holdoff_end && !s->stop_silence) {
                     s->mode = SILENCE_COPY_FLUSH;
-                    flush(s, out, outlink, &nb_samples_written, 0);
+                    flush(s, out, outlink, &nb_samples_written, &ret, 0);
                     goto silence_copy_flush;
                 } else if (threshold) {
                     for (j = 0; j < outlink->channels; j++) {
@@ -813,14 +796,13 @@  silence_copy:
                     s->stop_holdoff_end++;
 
                     if (s->stop_holdoff_end >= s->stop_duration) {
-                        s->stop_found_periods += s->one_period;
-                        if (s->stop_found_periods >= s->stop_periods) {
+                        if (++s->stop_found_periods >= s->stop_periods) {
                             s->stop_holdoff_offset = 0;
                             s->stop_holdoff_end = 0;
 
                             if (!s->restart) {
                                 s->mode = SILENCE_STOP;
-                                flush(s, out, outlink, &nb_samples_written, 1);
+                                flush(s, out, outlink, &nb_samples_written, &ret, 1);
                                 goto silence_stop;
                             } else {
                                 s->stop_found_periods = 0;
@@ -831,17 +813,17 @@  silence_copy:
                                 s->start_silence_end = 0;
                                 clear_window(s);
                                 s->mode = SILENCE_TRIM;
-                                flush(s, out, outlink, &nb_samples_written, 1);
+                                flush(s, out, outlink, &nb_samples_written, &ret, 1);
                                 goto silence_trim;
                             }
                         }
                         s->mode = SILENCE_COPY_FLUSH;
-                        flush(s, out, outlink, &nb_samples_written, 0);
+                        flush(s, out, outlink, &nb_samples_written, &ret, 0);
                         goto silence_copy_flush;
                     }
                 }
             }
-            flush(s, out, outlink, &nb_samples_written, 0);
+            flush(s, out, outlink, &nb_samples_written, &ret, 0);
         } else {
             av_samples_copy(out->extended_data, in->extended_data,
                             nb_samples_written,
@@ -853,8 +835,7 @@  silence_copy:
                                         (AVRational){1, outlink->sample_rate},
                                         outlink->time_base);
 
-            av_audio_fifo_write(s->fifo, (void **)out->extended_data, out->nb_samples);
-            av_frame_free(&out);
+            ret = ff_filter_frame(outlink, out);
         }
         break;
 
@@ -881,8 +862,7 @@  silence_copy_flush:
                                     (AVRational){1, outlink->sample_rate},
                                     outlink->time_base);
 
-        av_audio_fifo_write(s->fifo, (void **)out->extended_data, out->nb_samples);
-        av_frame_free(&out);
+        ret = ff_filter_frame(outlink, out);
 
         if (s->stop_holdoff_offset == s->stop_holdoff_end) {
             s->stop_holdoff_offset = 0;
@@ -902,16 +882,6 @@  silence_stop:
 
     av_frame_free(&in);
 
-    if (av_audio_fifo_size(s->fifo) > 0) {
-        out = ff_get_audio_buffer(outlink, av_audio_fifo_size(s->fifo));
-        if (!out)
-            return AVERROR(ENOMEM);
-
-        av_audio_fifo_read(s->fifo, (void **)out->extended_data, out->nb_samples);
-        out->pts = s->next_pts;
-        ret = ff_filter_frame(outlink, out);
-    }
-
     return ret;
 }
 
@@ -975,9 +945,6 @@  static av_cold void uninit(AVFilterContext *ctx)
     av_frame_free(&s->stop_holdoff);
     av_frame_free(&s->stop_silence_hold);
     av_frame_free(&s->window);
-
-    av_audio_fifo_free(s->fifo);
-    s->fifo = NULL;
 }
 
 static const AVFilterPad silenceremove_inputs[] = {