From 5a958921a738f2cd928f8339872b74a3c299ff0e Mon Sep 17 00:00:00 2001 From: wm4 Date: Sat, 23 Mar 2013 13:02:59 +0100 Subject: [PATCH] af: remove automatically inserted filters on full reinit Make sure automatically inserted filters are removed on full reinit (they are re-added later if they are really needed). Automatically inserted filters were never explicitly removed, instead, it was expected that redundant conversion filters detach themselves. This didn't work if there were several chained format conversion filters, e.g. s16le->floatle->s16le, which could result from repeated filter insertion and removal. (format filters detach only if input format and output format are the same.) Further, the dummy filter (which exists only because af.c can't handle an empty filter chain for some reason) could introduce bad conversions due to how the format negotiation works. Change the code so that the dummy filter never takes part on format negotiation. (It would be better to fix format negotiation, but that would be much more complicated and would involving fixing all filters.) Simplify af_reinit() and remove the start audio filter parameter. This means format negotiation and filter initialization is run more often, but should be harmless. --- audio/filter/af.c | 69 +++++++++++++++++++++++++++-------------------- audio/filter/af.h | 6 +++-- core/command.c | 2 +- 3 files changed, 45 insertions(+), 32 deletions(-) diff --git a/audio/filter/af.c b/audio/filter/af.c index 9b00bf0fa5..b7bfbd3145 100644 --- a/audio/filter/af.c +++ b/audio/filter/af.c @@ -20,6 +20,7 @@ #include #include #include +#include #include "af.h" @@ -249,6 +250,17 @@ void af_remove(struct af_stream *s, struct af_instance *af) free(af); } +static void remove_auto_inserted_filters(struct af_stream *s, bool dummy_only) +{ +repeat: + for (struct af_instance *af = s->first; af; af = af->next) { + if ((af->auto_inserted && !dummy_only) || af->info == &af_info_dummy) { + af_remove(s, af); + goto repeat; + } + } +} + static void print_fmt(struct mp_audio *d) { if (d) { @@ -285,18 +297,15 @@ static void af_print_filter_chain(struct af_stream *s) // state (for example, format filters that were tentatively inserted stay // inserted). // In that case, you should always rebuild the filter chain, or abort. -int af_reinit(struct af_stream *s, struct af_instance *af) +// Also, note that for complete reinit, fixup_output_format() must be called +// after this function. +int af_reinit(struct af_stream *s) { - do { - int rv = 0; // Return value + remove_auto_inserted_filters(s, true); - // Check if there are any filters left in the list - if (NULL == af) { - if (!(af = af_append(s, s->first, "dummy"))) - return AF_UNKNOWN; - else - return AF_ERROR; - } + struct af_instance *af = s->first; + while (af) { + int rv = 0; // Return value // Check if this is the first filter struct mp_audio in = af->prev ? *(af->prev->data) : s->input; @@ -334,6 +343,7 @@ int af_reinit(struct af_stream *s, struct af_instance *af) // Create format filter if (NULL == (new = af_prepend(s, af, "format"))) return AF_ERROR; + new->auto_inserted = true; // Set output bits per sample in.format |= af_bits2fmt(in.bps * 8); if (AF_OK != @@ -379,7 +389,15 @@ int af_reinit(struct af_stream *s, struct af_instance *af) af->info->name, rv); return AF_ERROR; } - } while (af); + } + + // At least one filter must exist in the chain. + if (!s->last) { + af = af_append(s, NULL, "dummy"); + if (!af) + return AF_ERROR; + af->control(af, AF_CONTROL_REINIT, &s->input); + } af_print_filter_chain(s); @@ -411,28 +429,29 @@ static int fixup_output_format(struct af_stream *s) if (!af || (AF_OK != af->control(af, AF_CONTROL_CHANNELS, &(s->output.nch)))) return AF_ERROR; - if (AF_OK != af_reinit(s, af)) + if (AF_OK != af_reinit(s)) return AF_ERROR; } // Check output format fix if not OK if (s->output.format != AF_FORMAT_UNKNOWN && s->last->data->format != s->output.format) { - if (strcmp(s->last->info->name, "format")) + if (strcmp(s->last->info->name, "format")) { af = af_append(s, s->last, "format"); - else + af->auto_inserted = true; + } else af = s->last; // Init the new filter s->output.format |= af_bits2fmt(s->output.bps * 8); if (!af || (AF_OK != af->control(af, AF_CONTROL_FORMAT_FMT, &(s->output.format)))) return AF_ERROR; - if (AF_OK != af_reinit(s, af)) + if (AF_OK != af_reinit(s)) return AF_ERROR; } // Re init again just in case - if (AF_OK != af_reinit(s, s->first)) + if (AF_OK != af_reinit(s)) return AF_ERROR; if (s->output.format == AF_FORMAT_UNKNOWN) @@ -509,21 +528,12 @@ int af_init(struct af_stream *s) } } - // If we do not have any filters otherwise - // add dummy to make automatic format conversion work - if (!s->first && !af_append(s, s->first, "dummy")) - return -1; + remove_auto_inserted_filters(s, false); // Init filters - if (AF_OK != af_reinit(s, s->first)) + if (AF_OK != af_reinit(s)) return -1; - // make sure the chain is not empty and valid (e.g. because of AF_DETACH) - if (!s->first) { - if (!af_append(s, s->first, "dummy") || AF_OK != af_reinit(s, s->first)) - return -1; - } - // Check output format if ((AF_INIT_TYPE_MASK & s->cfg.force) != AF_INIT_FORCE) { struct af_instance *af = NULL; // New filter @@ -548,11 +558,12 @@ int af_init(struct af_stream *s) // Init the new filter if (!af) return -1; + af->auto_inserted = true; if (af->control(af, AF_CONTROL_RESAMPLE_RATE | AF_CONTROL_SET, &(s->output.rate)) != AF_OK) return -1; } - if (AF_OK != af_reinit(s, af)) + if (AF_OK != af_reinit(s)) return -1; } if (AF_OK != fixup_output_format(s)) { @@ -587,7 +598,7 @@ struct af_instance *af_add(struct af_stream *s, char *name) return NULL; // Reinitalize the filter list - if (AF_OK != af_reinit(s, s->first) || + if (AF_OK != af_reinit(s) || AF_OK != fixup_output_format(s)) { while (s->first) af_remove(s, s->first); diff --git a/audio/filter/af.h b/audio/filter/af.h index 71892aa144..e4a329abea 100644 --- a/audio/filter/af.h +++ b/audio/filter/af.h @@ -20,6 +20,7 @@ #define MPLAYER_AF_H #include +#include #include "config.h" @@ -75,6 +76,7 @@ struct af_instance { * corresponding output */ double mul; /* length multiplier: how much does this instance change the length of the buffer. */ + bool auto_inserted; // inserted by af.c, such as conversion filters }; // Initialization flags @@ -161,10 +163,10 @@ void af_uninit(struct af_stream *s); /** * \brief Reinit the filter list from the given filter on downwards - * \param Filter instance to begin the reinit from + * See af.c. * \return AF_OK on success or AF_ERROR on failure */ -int af_reinit(struct af_stream *s, struct af_instance *af); +int af_reinit(struct af_stream *s); /** * \brief This function adds the filter "name" to the stream s. diff --git a/core/command.c b/core/command.c index 8f6dfeb4ca..641dfb80e0 100644 --- a/core/command.c +++ b/core/command.c @@ -2274,7 +2274,7 @@ void run_command(MPContext *mpctx, mp_cmd_t *cmd) break; } af->control(af, AF_CONTROL_COMMAND_LINE, cmd->args[1].v.s); - af_reinit(sh_audio->afilter, af); + af_reinit(sh_audio->afilter); } break; case MP_CMD_SHOW_CHAPTERS: