From d2679f59c5a6c8df7a2140d40ab65a17b8e5c023 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 11 Nov 2013 16:15:38 +0100 Subject: [PATCH] PcmConvert: add methods Open(), Close() Replaces Reset() and eliminates the AudioFormat parameters from the Convert() method. --- src/DecoderAPI.cxx | 10 ++-- src/DecoderInternal.cxx | 6 ++- src/OutputThread.cxx | 16 ++++-- src/filter/AutoConvertFilterPlugin.cxx | 6 ++- src/filter/ConvertFilterPlugin.cxx | 65 +++++++++++++++++------- src/filter/ConvertFilterPlugin.hxx | 6 ++- src/pcm/PcmConvert.cxx | 68 +++++++++++++++++--------- src/pcm/PcmConvert.hxx | 36 ++++++-------- test/run_convert.cxx | 12 ++++- 9 files changed, 151 insertions(+), 74 deletions(-) diff --git a/src/DecoderAPI.cxx b/src/DecoderAPI.cxx index 7b850265b..1494e3f80 100644 --- a/src/DecoderAPI.cxx +++ b/src/DecoderAPI.cxx @@ -71,6 +71,12 @@ decoder_initialized(Decoder &decoder, &af_string)); decoder.convert = new PcmConvert(); + + Error error; + if (!decoder.convert->Open(dc.in_audio_format, + dc.out_audio_format, + error)) + decoder.error = std::move(error); } dc.Lock(); @@ -401,9 +407,7 @@ decoder_data(Decoder &decoder, assert(dc.in_audio_format != dc.out_audio_format); Error error; - data = decoder.convert->Convert(dc.in_audio_format, - data, length, - dc.out_audio_format, + data = decoder.convert->Convert(data, length, &length, error); if (data == nullptr) { diff --git a/src/DecoderInternal.cxx b/src/DecoderInternal.cxx index 26f91b2d9..38e54a36c 100644 --- a/src/DecoderInternal.cxx +++ b/src/DecoderInternal.cxx @@ -33,7 +33,11 @@ Decoder::~Decoder() /* caller must flush the chunk */ assert(chunk == nullptr); - delete convert; + if (convert != nullptr) { + convert->Close(); + delete convert; + } + delete song_tag; delete stream_tag; delete decoder_tag; diff --git a/src/OutputThread.cxx b/src/OutputThread.cxx index 30d3ba30f..dc81643f3 100644 --- a/src/OutputThread.cxx +++ b/src/OutputThread.cxx @@ -184,7 +184,15 @@ ao_open(struct audio_output *ao) return; } - convert_filter_set(ao->convert_filter, ao->out_audio_format); + if (!convert_filter_set(ao->convert_filter, ao->out_audio_format, + error)) { + FormatError(error, "Failed to convert for \"%s\" [%s]", + ao->name, ao->plugin->name); + + ao_filter_close(ao); + ao->fail_timer = g_timer_new(); + return; + } ao->open = true; @@ -233,7 +241,9 @@ ao_reopen_filter(struct audio_output *ao) ao_filter_close(ao); const AudioFormat filter_audio_format = ao_filter_open(ao, ao->in_audio_format, error); - if (!filter_audio_format.IsDefined()) { + if (!filter_audio_format.IsDefined() || + !convert_filter_set(ao->convert_filter, ao->out_audio_format, + error)) { FormatError(error, "Failed to open filter for \"%s\" [%s]", ao->name, ao->plugin->name); @@ -254,8 +264,6 @@ ao_reopen_filter(struct audio_output *ao) return; } - - convert_filter_set(ao->convert_filter, ao->out_audio_format); } static void diff --git a/src/filter/AutoConvertFilterPlugin.cxx b/src/filter/AutoConvertFilterPlugin.cxx index 918a16e53..44adc8a66 100644 --- a/src/filter/AutoConvertFilterPlugin.cxx +++ b/src/filter/AutoConvertFilterPlugin.cxx @@ -88,7 +88,11 @@ AutoConvertFilter::Open(AudioFormat &in_audio_format, Error &error) assert(audio_format2 == in_audio_format); - convert_filter_set(convert, child_audio_format); + if (!convert_filter_set(convert, child_audio_format, error)) { + delete convert; + filter->Close(); + return AudioFormat::Undefined(); + } } else /* no */ convert = nullptr; diff --git a/src/filter/ConvertFilterPlugin.cxx b/src/filter/ConvertFilterPlugin.cxx index 040f8426f..416b6e81a 100644 --- a/src/filter/ConvertFilterPlugin.cxx +++ b/src/filter/ConvertFilterPlugin.cxx @@ -39,21 +39,18 @@ class ConvertFilter final : public Filter { /** * The output audio format; the consumer of this plugin - * expects PCM data in this format. This defaults to - * #in_audio_format, and can be set with convert_filter_set(). + * expects PCM data in this format. + * + * If this is AudioFormat::Undefined(), then the #PcmConvert + * attribute is not open. This can mean that Set() has failed + * or that no conversion is necessary. */ AudioFormat out_audio_format; Manual state; public: - void Set(const AudioFormat &_out_audio_format) { - assert(in_audio_format.IsValid()); - assert(out_audio_format.IsValid()); - assert(_out_audio_format.IsValid()); - - out_audio_format = _out_audio_format; - } + bool Set(const AudioFormat &_out_audio_format, Error &error); virtual AudioFormat Open(AudioFormat &af, Error &error) override; virtual void Close() override; @@ -69,12 +66,40 @@ convert_filter_init(gcc_unused const config_param ¶m, return new ConvertFilter(); } +bool +ConvertFilter::Set(const AudioFormat &_out_audio_format, Error &error) +{ + assert(in_audio_format.IsValid()); + assert(_out_audio_format.IsValid()); + + if (_out_audio_format == out_audio_format) + /* no change */ + return true; + + if (out_audio_format.IsValid()) { + out_audio_format.Clear(); + state->Close(); + } + + if (_out_audio_format == in_audio_format) + /* optimized special case: no-op */ + return true; + + if (!state->Open(in_audio_format, _out_audio_format, error)) + return false; + + out_audio_format = _out_audio_format; + return true; +} + AudioFormat ConvertFilter::Open(AudioFormat &audio_format, gcc_unused Error &error) { assert(audio_format.IsValid()); - in_audio_format = out_audio_format = audio_format; + in_audio_format = audio_format; + out_audio_format.Clear(); + state.Construct(); return in_audio_format; @@ -83,6 +108,11 @@ ConvertFilter::Open(AudioFormat &audio_format, gcc_unused Error &error) void ConvertFilter::Close() { + assert(in_audio_format.IsValid()); + + if (out_audio_format.IsValid()) + state->Close(); + state.Destruct(); poison_undefined(&in_audio_format, sizeof(in_audio_format)); @@ -93,15 +123,15 @@ const void * ConvertFilter::FilterPCM(const void *src, size_t src_size, size_t *dest_size_r, Error &error) { - if (in_audio_format == out_audio_format) { + assert(in_audio_format.IsValid()); + + if (!out_audio_format.IsValid()) { /* optimized special case: no-op */ *dest_size_r = src_size; return src; } - return state->Convert(in_audio_format, - src, src_size, - out_audio_format, dest_size_r, + return state->Convert(src, src_size, dest_size_r, error); } @@ -110,10 +140,11 @@ const struct filter_plugin convert_filter_plugin = { convert_filter_init, }; -void -convert_filter_set(Filter *_filter, const AudioFormat out_audio_format) +bool +convert_filter_set(Filter *_filter, AudioFormat out_audio_format, + Error &error) { ConvertFilter *filter = (ConvertFilter *)_filter; - filter->Set(out_audio_format); + return filter->Set(out_audio_format, error); } diff --git a/src/filter/ConvertFilterPlugin.hxx b/src/filter/ConvertFilterPlugin.hxx index c814aaf49..f414e59bf 100644 --- a/src/filter/ConvertFilterPlugin.hxx +++ b/src/filter/ConvertFilterPlugin.hxx @@ -21,6 +21,7 @@ #define MPD_CONVERT_FILTER_PLUGIN_HXX class Filter; +class Error; struct AudioFormat; /** @@ -29,7 +30,8 @@ struct AudioFormat; * format switch is a violation of the filter API, this filter must be * the last in a chain. */ -void -convert_filter_set(Filter *filter, AudioFormat out_audio_format); +bool +convert_filter_set(Filter *filter, AudioFormat out_audio_format, + Error &error); #endif diff --git a/src/pcm/PcmConvert.cxx b/src/pcm/PcmConvert.cxx index 8eafe527c..cca2f701e 100644 --- a/src/pcm/PcmConvert.cxx +++ b/src/pcm/PcmConvert.cxx @@ -32,23 +32,48 @@ const Domain pcm_convert_domain("pcm_convert"); PcmConvert::PcmConvert() { +#ifndef NDEBUG + src_format.Clear(); + dest_format.Clear(); +#endif } PcmConvert::~PcmConvert() { + assert(!src_format.IsValid()); + assert(!dest_format.IsValid()); +} + +bool +PcmConvert::Open(AudioFormat _src_format, AudioFormat _dest_format, + gcc_unused Error &error) +{ + assert(!src_format.IsValid()); + assert(!dest_format.IsValid()); + assert(_src_format.IsValid()); + assert(_dest_format.IsValid()); + + src_format = _src_format; + dest_format = _dest_format; + + return true; } void -PcmConvert::Reset() +PcmConvert::Close() { dsd.Reset(); resampler.Reset(); + +#ifndef NDEBUG + src_format.Clear(); + dest_format.Clear(); +#endif } inline const int16_t * -PcmConvert::Convert16(const AudioFormat src_format, - const void *src_buffer, size_t src_size, - const AudioFormat dest_format, size_t *dest_size_r, +PcmConvert::Convert16(const void *src_buffer, size_t src_size, + size_t *dest_size_r, Error &error) { const int16_t *buf; @@ -96,9 +121,8 @@ PcmConvert::Convert16(const AudioFormat src_format, } inline const int32_t * -PcmConvert::Convert24(const AudioFormat src_format, - const void *src_buffer, size_t src_size, - const AudioFormat dest_format, size_t *dest_size_r, +PcmConvert::Convert24(const void *src_buffer, size_t src_size, + size_t *dest_size_r, Error &error) { const int32_t *buf; @@ -145,9 +169,8 @@ PcmConvert::Convert24(const AudioFormat src_format, } inline const int32_t * -PcmConvert::Convert32(const AudioFormat src_format, - const void *src_buffer, size_t src_size, - const AudioFormat dest_format, size_t *dest_size_r, +PcmConvert::Convert32(const void *src_buffer, size_t src_size, + size_t *dest_size_r, Error &error) { const int32_t *buf; @@ -194,9 +217,8 @@ PcmConvert::Convert32(const AudioFormat src_format, } inline const float * -PcmConvert::ConvertFloat(const AudioFormat src_format, - const void *src_buffer, size_t src_size, - const AudioFormat dest_format, size_t *dest_size_r, +PcmConvert::ConvertFloat(const void *src_buffer, size_t src_size, + size_t *dest_size_r, Error &error) { const float *buffer = (const float *)src_buffer; @@ -251,9 +273,7 @@ PcmConvert::ConvertFloat(const AudioFormat src_format, } const void * -PcmConvert::Convert(AudioFormat src_format, - const void *src, size_t src_size, - const AudioFormat dest_format, +PcmConvert::Convert(const void *src, size_t src_size, size_t *dest_size_r, Error &error) { @@ -279,23 +299,23 @@ PcmConvert::Convert(AudioFormat src_format, switch (dest_format.format) { case SampleFormat::S16: - return Convert16(src_format, src, src_size, - dest_format, dest_size_r, + return Convert16(src, src_size, + dest_size_r, error); case SampleFormat::S24_P32: - return Convert24(src_format, src, src_size, - dest_format, dest_size_r, + return Convert24(src, src_size, + dest_size_r, error); case SampleFormat::S32: - return Convert32(src_format, src, src_size, - dest_format, dest_size_r, + return Convert32(src, src_size, + dest_size_r, error); case SampleFormat::FLOAT: - return ConvertFloat(src_format, src, src_size, - dest_format, dest_size_r, + return ConvertFloat(src, src_size, + dest_size_r, error); default: diff --git a/src/pcm/PcmConvert.hxx b/src/pcm/PcmConvert.hxx index 40f785179..4bace1d36 100644 --- a/src/pcm/PcmConvert.hxx +++ b/src/pcm/PcmConvert.hxx @@ -24,10 +24,10 @@ #include "PcmDsd.hxx" #include "PcmResample.hxx" #include "PcmBuffer.hxx" +#include "AudioFormat.hxx" #include -struct AudioFormat; class Error; /** @@ -48,17 +48,23 @@ class PcmConvert { /** the buffer for converting the channel count */ PcmBuffer channels_buffer; + AudioFormat src_format, dest_format; + public: PcmConvert(); ~PcmConvert(); + /** + * Prepare the object. Call Close() when done. + */ + bool Open(AudioFormat _src_format, AudioFormat _dest_format, + Error &error); /** - * Reset the pcm_convert_state object. Use this at the - * boundary between two distinct songs and each time the - * format changes. + * Close the object after it was prepared with Open(). After + * that, it may be reused by calling Open() again. */ - void Reset(); + void Close(); /** * Converts PCM data between two audio formats. @@ -72,34 +78,24 @@ public: * ignore errors * @return the destination buffer, or NULL on error */ - const void *Convert(AudioFormat src_format, - const void *src, size_t src_size, - AudioFormat dest_format, + const void *Convert(const void *src, size_t src_size, size_t *dest_size_r, Error &error); private: - const int16_t *Convert16(AudioFormat src_format, - const void *src_buffer, size_t src_size, - AudioFormat dest_format, + const int16_t *Convert16(const void *src_buffer, size_t src_size, size_t *dest_size_r, Error &error); - const int32_t *Convert24(AudioFormat src_format, - const void *src_buffer, size_t src_size, - AudioFormat dest_format, + const int32_t *Convert24(const void *src_buffer, size_t src_size, size_t *dest_size_r, Error &error); - const int32_t *Convert32(AudioFormat src_format, - const void *src_buffer, size_t src_size, - AudioFormat dest_format, + const int32_t *Convert32(const void *src_buffer, size_t src_size, size_t *dest_size_r, Error &error); - const float *ConvertFloat(AudioFormat src_format, - const void *src_buffer, size_t src_size, - AudioFormat dest_format, + const float *ConvertFloat(const void *src_buffer, size_t src_size, size_t *dest_size_r, Error &error); }; diff --git a/test/run_convert.cxx b/test/run_convert.cxx index 0e873a3b3..d9a1b7f08 100644 --- a/test/run_convert.cxx +++ b/test/run_convert.cxx @@ -90,6 +90,11 @@ int main(int argc, char **argv) const size_t in_frame_size = in_audio_format.GetFrameSize(); PcmConvert state; + if (!state.Open(in_audio_format, out_audio_format_mask, error)) { + g_printerr("Failed to open PcmConvert: %s\n", + error.GetMessage()); + return EXIT_FAILURE; + } FifoBuffer buffer; @@ -115,9 +120,10 @@ int main(int argc, char **argv) buffer.Consume(src.size); size_t length; - output = state.Convert(in_audio_format, src.data, src.size, - out_audio_format, &length, error); + output = state.Convert(src.data, src.size, + &length, error); if (output == NULL) { + state.Close(); g_printerr("Failed to convert: %s\n", error.GetMessage()); return 2; } @@ -125,5 +131,7 @@ int main(int argc, char **argv) gcc_unused ssize_t ignored = write(1, output, length); } + state.Close(); + return EXIT_SUCCESS; }