shithub: sox

Info  •  Files  •  Log  •  Branches

Don't drop samples at effects chain transitions

When using multiple/restarted effects chains, at each transition to
or restart of an effects chain, a number of samples would be lost,
even if the effect causing the transition was the first in its chain
(as recommended in the man page). The exact number of affected
samples would depend on buffer size, number of channels, flow
behaviour of the effects/handlers involved etc. and was thus
unpredictable.

To correct this problem, this commit modifies sox_flow_effects() to
not free effects' output buffers at the end of processing (this is
left to sox_delete_effect() instead). Since the input combiner
effect is reused in the new effects chain, this allows samples
already processed by the combiner to remain in its buffer and be
flowed down the new effects chain later.

This should, a.o., make sure that after
  sox infile splitfiles trim 0 1:00 : newfile : restart
  sox splitfiles* recombined
"infile" and "recombined" are identical (provided the encoding is
lossless, of course).

(Note that samples buffered by "normal" effects, i.e. those that are
not reused, to the left of the effect terminating the chain will still
be dropped. E.g., with
  sox infile outfile highpass 40 trim 0 10 :
a number of samples already processed by the highpass effect but not
consumed by trim will be lost. This behaviour is already documented in
the man page.)

--- a/ChangeLog

+++ b/ChangeLog

@@ -88,6 +88,8 @@

   o Complete rewrite of the trim effect with extended syntax (backwards

     compatible) and capabilities. [FR 2941349] (Ulrich Klauer)

   o Fix trim optimization unexpectedly seeking backwards. (Ulrich Klauer)

+  o Prevent samples from getting lost at effects chain transitions in

+    multiple effects chain/multiple output modes. (Ulrich Klauer)

 Misc:

--- a/src/effects.c

+++ b/src/effects.c

@@ -58,6 +58,7 @@

 sox_effect_t * sox_create_effect(sox_effect_handler_t const * eh)

 {

   sox_effect_t * effp = lsx_calloc(1, sizeof(*effp));

+  effp->obuf = NULL;

   effp->global_info = sox_get_effects_globals();

   effp->handler = *eh;

@@ -362,8 +363,11 @@

   sox_bool draining = sox_true;

   for (e = 0; e < chain->length; ++e) {

-    chain->effects[e][0].obuf = lsx_malloc(sox_globals.bufsiz * sizeof(chain->effects[e][0].obuf[0]));

-    chain->effects[e][0].obeg = chain->effects[e][0].oend = 0;

+    chain->effects[e][0].obuf = lsx_realloc(chain->effects[e][0].obuf,

+        sox_globals.bufsiz * sizeof(chain->effects[e][0].obuf[0]));

+      /* Possibly there is already a buffer, if this is a used effect;

+         it may still contain samples in that case. */

+      /* Memory will be freed by sox_delete_effect() later. */

     max_flows = max(max_flows, chain->effects[e][0].flows);

   }

@@ -410,9 +414,6 @@

   free(chain->obufc);

   free(chain->ibufc);

-  for (e = 0; e < chain->length; ++e)

-    free(chain->effects[e][0].obuf);

-

   return flow_status;

 }

@@ -476,9 +477,15 @@

   if ((clips = sox_stop_effect(effp)) != 0)

     lsx_warn("%s clipped %" PRIu64 " samples; decrease volume?",

         effp->handler.name, clips);

+  if (effp->obeg != effp->oend)

+    lsx_debug("output buffer still held %" PRIu64 " samples; dropped.",

+        (effp->oend - effp->obeg)/effp->out_signal.channels);

+      /* May or may not indicate a problem; it is normal if the user aborted

+         processing, or if an effect like "trim" stopped early. */

   effp->handler.kill(effp); /* N.B. only one kill; not one per flow */

   for (f = 0; f < effp->flows; ++f)

     free(effp[f].priv);

+  free(effp->obuf);

   free(effp);

 }

--- a/src/sox.c

+++ b/src/sox.c

@@ -1169,13 +1169,8 @@

     if (reuse_output)

       save_output_eff = sox_pop_effect_last(effects_chain);

-    /* TODO: Warn user when an effect is deleted that still

-     * had unprocessed samples.

-     */

     while (effects_chain->length > 1)

-    {

       sox_delete_effect_last(effects_chain);

-    }

   }

   return SOX_SUCCESS;

 } /* advance_eff_chain */