From 691b6a236ebc769f3d1aaa21ddc3a4c7ad9211be Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 1 Jul 2020 17:20:19 +0200 Subject: [PATCH 01/20] output/osx: improve sample rate selection The formula in osx_output_score_sample_rate() to detect multiples of the source sample rate was broken: when given a 44.1 kHz input file, it preferred 16 kHz over 48 kHz, because its `frac_portion(16)=0.75` is smaller than `frac_portion(48)=0.91`. That formula, introduced by commit 40a1ebee295c569, looks completely wrong. It doesn't do what the code comment pretends it does. Instead of using that `frac_portion` to calculate a score, this patch adds to the score only if `frac_portion` is nearly `0` or `1`. This means that the factor is nearly integer. Closes https://github.com/MusicPlayerDaemon/MPD/issues/904 --- NEWS | 2 ++ src/output/plugins/OSXOutputPlugin.cxx | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index 99c52a92b..308e70712 100644 --- a/NEWS +++ b/NEWS @@ -4,6 +4,8 @@ ver 0.21.25 (not yet released) * input - file: detect premature end of file - smbclient: don't send credentials to MPD clients +* output + - osx: improve sample rate selection * Windows/Android: - fix Boost detection after breaking change in Meson 0.54 diff --git a/src/output/plugins/OSXOutputPlugin.cxx b/src/output/plugins/OSXOutputPlugin.cxx index 7f84e6e83..da7bb4f19 100644 --- a/src/output/plugins/OSXOutputPlugin.cxx +++ b/src/output/plugins/OSXOutputPlugin.cxx @@ -342,7 +342,8 @@ osx_output_score_sample_rate(Float64 destination_rate, unsigned source_rate) double int_portion; double frac_portion = modf(source_rate / destination_rate, &int_portion); // prefer sample rates that are multiples of the source sample rate - score += (1 - frac_portion) * 1000; + if (frac_portion < 0.01 || frac_portion >= 0.99) + score += 1000; // prefer exact matches over other multiples score += (int_portion == 1.0) ? 500 : 0; if (source_rate == destination_rate) From c4efc37ad88d8ee99195544f2dfb9954090df181 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 8 Mar 2019 10:21:10 +0100 Subject: [PATCH 02/20] system/ByteOrder: move to util/ --- src/decoder/plugins/DsdLib.hxx | 2 +- src/decoder/plugins/DsdiffDecoderPlugin.cxx | 2 +- src/decoder/plugins/DsfDecoderPlugin.cxx | 2 +- src/decoder/plugins/HybridDsdDecoderPlugin.cxx | 2 +- src/decoder/plugins/PcmDecoderPlugin.cxx | 2 +- src/decoder/plugins/SidplayDecoderPlugin.cxx | 2 +- src/encoder/plugins/OpusEncoderPlugin.cxx | 2 +- src/encoder/plugins/WaveEncoderPlugin.cxx | 2 +- src/input/plugins/CdioParanoiaInputPlugin.cxx | 2 +- src/lib/alsa/HwSetup.cxx | 2 +- src/net/IPv4Address.hxx | 2 +- src/net/IPv6Address.hxx | 2 +- src/output/plugins/OSXOutputPlugin.cxx | 2 +- src/output/plugins/OssOutputPlugin.cxx | 2 +- src/output/plugins/sles/SlesOutputPlugin.cxx | 2 +- src/pcm/PcmPack.cxx | 2 +- src/tag/Aiff.cxx | 2 +- src/tag/ApeLoader.cxx | 2 +- src/tag/Riff.cxx | 2 +- src/{system => util}/ByteOrder.hxx | 2 +- src/util/ByteReverse.cxx | 2 +- test/test_pcm_export.cxx | 2 +- test/test_pcm_pack.cxx | 2 +- 23 files changed, 23 insertions(+), 23 deletions(-) rename src/{system => util}/ByteOrder.hxx (99%) diff --git a/src/decoder/plugins/DsdLib.hxx b/src/decoder/plugins/DsdLib.hxx index 6709f2821..e4758e25a 100644 --- a/src/decoder/plugins/DsdLib.hxx +++ b/src/decoder/plugins/DsdLib.hxx @@ -20,7 +20,7 @@ #ifndef MPD_DECODER_DSDLIB_HXX #define MPD_DECODER_DSDLIB_HXX -#include "system/ByteOrder.hxx" +#include "util/ByteOrder.hxx" #include "input/Offset.hxx" #include "util/Compiler.h" diff --git a/src/decoder/plugins/DsdiffDecoderPlugin.cxx b/src/decoder/plugins/DsdiffDecoderPlugin.cxx index b1779632a..caca7f7ba 100644 --- a/src/decoder/plugins/DsdiffDecoderPlugin.cxx +++ b/src/decoder/plugins/DsdiffDecoderPlugin.cxx @@ -32,7 +32,7 @@ #include "input/InputStream.hxx" #include "CheckAudioFormat.hxx" #include "util/bit_reverse.h" -#include "system/ByteOrder.hxx" +#include "util/ByteOrder.hxx" #include "tag/Handler.hxx" #include "DsdLib.hxx" #include "Log.hxx" diff --git a/src/decoder/plugins/DsfDecoderPlugin.cxx b/src/decoder/plugins/DsfDecoderPlugin.cxx index 332f0525c..d95fda9da 100644 --- a/src/decoder/plugins/DsfDecoderPlugin.cxx +++ b/src/decoder/plugins/DsfDecoderPlugin.cxx @@ -33,7 +33,7 @@ #include "input/InputStream.hxx" #include "CheckAudioFormat.hxx" #include "util/bit_reverse.h" -#include "system/ByteOrder.hxx" +#include "util/ByteOrder.hxx" #include "DsdLib.hxx" #include "tag/Handler.hxx" #include "Log.hxx" diff --git a/src/decoder/plugins/HybridDsdDecoderPlugin.cxx b/src/decoder/plugins/HybridDsdDecoderPlugin.cxx index 993bfb283..46f109eb2 100644 --- a/src/decoder/plugins/HybridDsdDecoderPlugin.cxx +++ b/src/decoder/plugins/HybridDsdDecoderPlugin.cxx @@ -20,7 +20,7 @@ #include "HybridDsdDecoderPlugin.hxx" #include "../DecoderAPI.hxx" #include "input/InputStream.hxx" -#include "system/ByteOrder.hxx" +#include "util/ByteOrder.hxx" #include "util/Domain.hxx" #include "util/WritableBuffer.hxx" #include "util/StaticFifoBuffer.hxx" diff --git a/src/decoder/plugins/PcmDecoderPlugin.cxx b/src/decoder/plugins/PcmDecoderPlugin.cxx index e114c8a32..2448e59cc 100644 --- a/src/decoder/plugins/PcmDecoderPlugin.cxx +++ b/src/decoder/plugins/PcmDecoderPlugin.cxx @@ -22,7 +22,7 @@ #include "CheckAudioFormat.hxx" #include "pcm/PcmPack.hxx" #include "input/InputStream.hxx" -#include "system/ByteOrder.hxx" +#include "util/ByteOrder.hxx" #include "util/Domain.hxx" #include "util/ByteReverse.hxx" #include "util/StaticFifoBuffer.hxx" diff --git a/src/decoder/plugins/SidplayDecoderPlugin.cxx b/src/decoder/plugins/SidplayDecoderPlugin.cxx index 581f78981..1d21bab7c 100644 --- a/src/decoder/plugins/SidplayDecoderPlugin.cxx +++ b/src/decoder/plugins/SidplayDecoderPlugin.cxx @@ -35,7 +35,7 @@ #include "util/Domain.hxx" #include "util/AllocatedString.hxx" #include "util/CharUtil.hxx" -#include "system/ByteOrder.hxx" +#include "util/ByteOrder.hxx" #include "Log.hxx" #ifdef HAVE_SIDPLAYFP diff --git a/src/encoder/plugins/OpusEncoderPlugin.cxx b/src/encoder/plugins/OpusEncoderPlugin.cxx index fea308d61..114ea1aa9 100644 --- a/src/encoder/plugins/OpusEncoderPlugin.cxx +++ b/src/encoder/plugins/OpusEncoderPlugin.cxx @@ -22,7 +22,7 @@ #include "AudioFormat.hxx" #include "config/Domain.hxx" #include "util/Alloc.hxx" -#include "system/ByteOrder.hxx" +#include "util/ByteOrder.hxx" #include "util/StringUtil.hxx" #include diff --git a/src/encoder/plugins/WaveEncoderPlugin.cxx b/src/encoder/plugins/WaveEncoderPlugin.cxx index 2b6156569..20abcc474 100644 --- a/src/encoder/plugins/WaveEncoderPlugin.cxx +++ b/src/encoder/plugins/WaveEncoderPlugin.cxx @@ -19,7 +19,7 @@ #include "WaveEncoderPlugin.hxx" #include "../EncoderAPI.hxx" -#include "system/ByteOrder.hxx" +#include "util/ByteOrder.hxx" #include "util/DynamicFifoBuffer.hxx" #include diff --git a/src/input/plugins/CdioParanoiaInputPlugin.cxx b/src/input/plugins/CdioParanoiaInputPlugin.cxx index eb7fb26c8..d5c04df0d 100644 --- a/src/input/plugins/CdioParanoiaInputPlugin.cxx +++ b/src/input/plugins/CdioParanoiaInputPlugin.cxx @@ -30,7 +30,7 @@ #include "util/StringCompare.hxx" #include "util/RuntimeError.hxx" #include "util/Domain.hxx" -#include "system/ByteOrder.hxx" +#include "util/ByteOrder.hxx" #include "fs/AllocatedPath.hxx" #include "Log.hxx" #include "config/Block.hxx" diff --git a/src/lib/alsa/HwSetup.cxx b/src/lib/alsa/HwSetup.cxx index eae4ae32e..abdaf983f 100644 --- a/src/lib/alsa/HwSetup.cxx +++ b/src/lib/alsa/HwSetup.cxx @@ -19,7 +19,7 @@ #include "HwSetup.hxx" #include "Format.hxx" -#include "system/ByteOrder.hxx" +#include "util/ByteOrder.hxx" #include "util/Domain.hxx" #include "util/RuntimeError.hxx" #include "AudioFormat.hxx" diff --git a/src/net/IPv4Address.hxx b/src/net/IPv4Address.hxx index 3af0210c3..e040c7c5b 100644 --- a/src/net/IPv4Address.hxx +++ b/src/net/IPv4Address.hxx @@ -31,7 +31,7 @@ #define IPV4_ADDRESS_HXX #include "SocketAddress.hxx" -#include "system/ByteOrder.hxx" +#include "util/ByteOrder.hxx" #include diff --git a/src/net/IPv6Address.hxx b/src/net/IPv6Address.hxx index eae9b60c1..a3aa695a5 100644 --- a/src/net/IPv6Address.hxx +++ b/src/net/IPv6Address.hxx @@ -31,7 +31,7 @@ #define IPV6_ADDRESS_HXX #include "SocketAddress.hxx" -#include "system/ByteOrder.hxx" +#include "util/ByteOrder.hxx" #include "util/Compiler.h" #include diff --git a/src/output/plugins/OSXOutputPlugin.cxx b/src/output/plugins/OSXOutputPlugin.cxx index da7bb4f19..af4ad56d3 100644 --- a/src/output/plugins/OSXOutputPlugin.cxx +++ b/src/output/plugins/OSXOutputPlugin.cxx @@ -29,7 +29,7 @@ #include "pcm/PcmExport.hxx" #include "thread/Mutex.hxx" #include "thread/Cond.hxx" -#include "system/ByteOrder.hxx" +#include "util/ByteOrder.hxx" #include "util/StringBuffer.hxx" #include "util/StringFormat.hxx" #include "Log.hxx" diff --git a/src/output/plugins/OssOutputPlugin.cxx b/src/output/plugins/OssOutputPlugin.cxx index 77ec21bed..2938fb868 100644 --- a/src/output/plugins/OssOutputPlugin.cxx +++ b/src/output/plugins/OssOutputPlugin.cxx @@ -25,7 +25,7 @@ #include "util/ConstBuffer.hxx" #include "util/Domain.hxx" #include "util/Macros.hxx" -#include "system/ByteOrder.hxx" +#include "util/ByteOrder.hxx" #include "Log.hxx" #include diff --git a/src/output/plugins/sles/SlesOutputPlugin.cxx b/src/output/plugins/sles/SlesOutputPlugin.cxx index a2a265c59..09968559d 100644 --- a/src/output/plugins/sles/SlesOutputPlugin.cxx +++ b/src/output/plugins/sles/SlesOutputPlugin.cxx @@ -27,7 +27,7 @@ #include "thread/Cond.hxx" #include "util/Macros.hxx" #include "util/Domain.hxx" -#include "system/ByteOrder.hxx" +#include "util/ByteOrder.hxx" #include "mixer/MixerList.hxx" #include "Log.hxx" diff --git a/src/pcm/PcmPack.cxx b/src/pcm/PcmPack.cxx index 58d6a1a1d..cb4f4261e 100644 --- a/src/pcm/PcmPack.cxx +++ b/src/pcm/PcmPack.cxx @@ -18,7 +18,7 @@ */ #include "PcmPack.hxx" -#include "system/ByteOrder.hxx" +#include "util/ByteOrder.hxx" static void pack_sample(uint8_t *dest, const int32_t *src0) noexcept diff --git a/src/tag/Aiff.cxx b/src/tag/Aiff.cxx index 57ec343d5..3cc3a7f43 100644 --- a/src/tag/Aiff.cxx +++ b/src/tag/Aiff.cxx @@ -19,7 +19,7 @@ #include "Aiff.hxx" #include "input/InputStream.hxx" -#include "system/ByteOrder.hxx" +#include "util/ByteOrder.hxx" #include #include diff --git a/src/tag/ApeLoader.cxx b/src/tag/ApeLoader.cxx index 88556cd4b..eb2c76b75 100644 --- a/src/tag/ApeLoader.cxx +++ b/src/tag/ApeLoader.cxx @@ -18,7 +18,7 @@ */ #include "ApeLoader.hxx" -#include "system/ByteOrder.hxx" +#include "util/ByteOrder.hxx" #include "input/InputStream.hxx" #include "util/StringView.hxx" diff --git a/src/tag/Riff.cxx b/src/tag/Riff.cxx index 85cbcf743..9671861fe 100644 --- a/src/tag/Riff.cxx +++ b/src/tag/Riff.cxx @@ -19,7 +19,7 @@ #include "Riff.hxx" #include "input/InputStream.hxx" -#include "system/ByteOrder.hxx" +#include "util/ByteOrder.hxx" #include #include diff --git a/src/system/ByteOrder.hxx b/src/util/ByteOrder.hxx similarity index 99% rename from src/system/ByteOrder.hxx rename to src/util/ByteOrder.hxx index 8ed3b6c72..edeeb31c3 100644 --- a/src/system/ByteOrder.hxx +++ b/src/util/ByteOrder.hxx @@ -30,7 +30,7 @@ #ifndef BYTE_ORDER_HXX #define BYTE_ORDER_HXX -#include "util/Compiler.h" +#include "Compiler.h" #include diff --git a/src/util/ByteReverse.cxx b/src/util/ByteReverse.cxx index b0518316a..225f4089c 100644 --- a/src/util/ByteReverse.cxx +++ b/src/util/ByteReverse.cxx @@ -18,7 +18,7 @@ */ #include "ByteReverse.hxx" -#include "system/ByteOrder.hxx" +#include "util/ByteOrder.hxx" #include "Compiler.h" #include diff --git a/test/test_pcm_export.cxx b/test/test_pcm_export.cxx index c8b266617..e53cba931 100644 --- a/test/test_pcm_export.cxx +++ b/test/test_pcm_export.cxx @@ -20,7 +20,7 @@ #include "config.h" #include "pcm/PcmExport.hxx" #include "pcm/Traits.hxx" -#include "system/ByteOrder.hxx" +#include "util/ByteOrder.hxx" #include "util/ConstBuffer.hxx" #include diff --git a/test/test_pcm_pack.cxx b/test/test_pcm_pack.cxx index a3055cbf7..8acbac61c 100644 --- a/test/test_pcm_pack.cxx +++ b/test/test_pcm_pack.cxx @@ -19,7 +19,7 @@ #include "test_pcm_util.hxx" #include "pcm/PcmPack.hxx" -#include "system/ByteOrder.hxx" +#include "util/ByteOrder.hxx" #include From 472881cb954b9817679b2801e5b013f0bf3351eb Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 8 Mar 2019 10:23:14 +0100 Subject: [PATCH 03/20] util/ByteOrder: remove redundant `inline` keywords from `constexpr` functions --- src/util/ByteOrder.hxx | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/src/util/ByteOrder.hxx b/src/util/ByteOrder.hxx index edeeb31c3..719d3a083 100644 --- a/src/util/ByteOrder.hxx +++ b/src/util/ByteOrder.hxx @@ -73,39 +73,39 @@ # endif #endif -static inline constexpr bool +constexpr bool IsLittleEndian() { return IS_LITTLE_ENDIAN; } -static inline constexpr bool +constexpr bool IsBigEndian() { return IS_BIG_ENDIAN; } -static inline constexpr uint16_t +constexpr uint16_t GenericByteSwap16(uint16_t value) { return (value >> 8) | (value << 8); } -static inline constexpr uint32_t +constexpr uint32_t GenericByteSwap32(uint32_t value) { return (value >> 24) | ((value >> 8) & 0x0000ff00) | ((value << 8) & 0x00ff0000) | (value << 24); } -static inline constexpr uint64_t +constexpr uint64_t GenericByteSwap64(uint64_t value) { return uint64_t(GenericByteSwap32(uint32_t(value >> 32))) | (uint64_t(GenericByteSwap32(value)) << 32); } -static inline constexpr uint16_t +constexpr uint16_t ByteSwap16(uint16_t value) { #if CLANG_OR_GCC_VERSION(4,8) @@ -115,7 +115,7 @@ ByteSwap16(uint16_t value) #endif } -static inline constexpr uint32_t +constexpr uint32_t ByteSwap32(uint32_t value) { #if CLANG_OR_GCC_VERSION(4,3) @@ -125,7 +125,7 @@ ByteSwap32(uint32_t value) #endif } -static inline constexpr uint64_t +constexpr uint64_t ByteSwap64(uint64_t value) { #if CLANG_OR_GCC_VERSION(4,3) @@ -138,7 +138,7 @@ ByteSwap64(uint64_t value) /** * Converts a 16bit value from big endian to the system's byte order */ -static inline constexpr uint16_t +constexpr uint16_t FromBE16(uint16_t value) { return IsBigEndian() ? value : ByteSwap16(value); @@ -147,7 +147,7 @@ FromBE16(uint16_t value) /** * Converts a 32bit value from big endian to the system's byte order */ -static inline constexpr uint32_t +constexpr uint32_t FromBE32(uint32_t value) { return IsBigEndian() ? value : ByteSwap32(value); @@ -156,7 +156,7 @@ FromBE32(uint32_t value) /** * Converts a 64bit value from big endian to the system's byte order */ -static inline constexpr uint64_t +constexpr uint64_t FromBE64(uint64_t value) { return IsBigEndian() ? value : ByteSwap64(value); @@ -165,7 +165,7 @@ FromBE64(uint64_t value) /** * Converts a 16bit value from little endian to the system's byte order */ -static inline constexpr uint16_t +constexpr uint16_t FromLE16(uint16_t value) { return IsLittleEndian() ? value : ByteSwap16(value); @@ -174,7 +174,7 @@ FromLE16(uint16_t value) /** * Converts a 32bit value from little endian to the system's byte order */ -static inline constexpr uint32_t +constexpr uint32_t FromLE32(uint32_t value) { return IsLittleEndian() ? value : ByteSwap32(value); @@ -183,7 +183,7 @@ FromLE32(uint32_t value) /** * Converts a 64bit value from little endian to the system's byte order */ -static inline constexpr uint64_t +constexpr uint64_t FromLE64(uint64_t value) { return IsLittleEndian() ? value : ByteSwap64(value); @@ -192,7 +192,7 @@ FromLE64(uint64_t value) /** * Converts a 16bit value from the system's byte order to big endian */ -static inline constexpr uint16_t +constexpr uint16_t ToBE16(uint16_t value) { return IsBigEndian() ? value : ByteSwap16(value); @@ -201,7 +201,7 @@ ToBE16(uint16_t value) /** * Converts a 32bit value from the system's byte order to big endian */ -static inline constexpr uint32_t +constexpr uint32_t ToBE32(uint32_t value) { return IsBigEndian() ? value : ByteSwap32(value); @@ -210,7 +210,7 @@ ToBE32(uint32_t value) /** * Converts a 64bit value from the system's byte order to big endian */ -static inline constexpr uint64_t +constexpr uint64_t ToBE64(uint64_t value) { return IsBigEndian() ? value : ByteSwap64(value); @@ -219,7 +219,7 @@ ToBE64(uint64_t value) /** * Converts a 16bit value from the system's byte order to little endian */ -static inline constexpr uint16_t +constexpr uint16_t ToLE16(uint16_t value) { return IsLittleEndian() ? value : ByteSwap16(value); @@ -228,7 +228,7 @@ ToLE16(uint16_t value) /** * Converts a 32bit value from the system's byte order to little endian */ -static inline constexpr uint32_t +constexpr uint32_t ToLE32(uint32_t value) { return IsLittleEndian() ? value : ByteSwap32(value); @@ -237,7 +237,7 @@ ToLE32(uint32_t value) /** * Converts a 64bit value from the system's byte order to little endian */ -static inline constexpr uint64_t +constexpr uint64_t ToLE64(uint64_t value) { return IsLittleEndian() ? value : ByteSwap64(value); From a99b4abae8a6bbefbb3628cc59f8cc35854a005c Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 1 Jul 2020 17:48:44 +0200 Subject: [PATCH 04/20] decoder/OpusHead: return pre-skip --- src/decoder/plugins/OpusDecoderPlugin.cxx | 8 +++++--- src/decoder/plugins/OpusHead.cxx | 5 ++++- src/decoder/plugins/OpusHead.hxx | 3 ++- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/decoder/plugins/OpusDecoderPlugin.cxx b/src/decoder/plugins/OpusDecoderPlugin.cxx index 3b3c312af..8a262b901 100644 --- a/src/decoder/plugins/OpusDecoderPlugin.cxx +++ b/src/decoder/plugins/OpusDecoderPlugin.cxx @@ -136,8 +136,8 @@ MPDOpusDecoder::OnOggBeginning(const ogg_packet &packet) if (opus_decoder != nullptr || !IsOpusHead(packet)) throw std::runtime_error("BOS packet must be OpusHead"); - unsigned channels; - if (!ScanOpusHeader(packet.packet, packet.bytes, channels) || + unsigned channels, pre_skip; + if (!ScanOpusHeader(packet.packet, packet.bytes, channels, pre_skip) || !audio_valid_channel_count(channels)) throw std::runtime_error("Malformed BOS packet"); @@ -305,10 +305,12 @@ ReadAndParseOpusHead(OggSyncState &sync, OggStreamState &stream, unsigned &channels) { ogg_packet packet; + unsigned pre_skip; return OggReadPacket(sync, stream, packet) && packet.b_o_s && IsOpusHead(packet) && - ScanOpusHeader(packet.packet, packet.bytes, channels) && + ScanOpusHeader(packet.packet, packet.bytes, channels, + pre_skip) && audio_valid_channel_count(channels); } diff --git a/src/decoder/plugins/OpusHead.cxx b/src/decoder/plugins/OpusHead.cxx index 2c7555c79..30d959f04 100644 --- a/src/decoder/plugins/OpusHead.cxx +++ b/src/decoder/plugins/OpusHead.cxx @@ -18,6 +18,7 @@ */ #include "OpusHead.hxx" +#include "util/ByteOrder.hxx" #include @@ -31,12 +32,14 @@ struct OpusHead { }; bool -ScanOpusHeader(const void *data, size_t size, unsigned &channels_r) +ScanOpusHeader(const void *data, size_t size, unsigned &channels_r, + unsigned &pre_skip_r) { const OpusHead *h = (const OpusHead *)data; if (size < 19 || (h->version & 0xf0) != 0) return false; channels_r = h->channels; + pre_skip_r = FromLE16(h->pre_skip); return true; } diff --git a/src/decoder/plugins/OpusHead.hxx b/src/decoder/plugins/OpusHead.hxx index 5bec73208..39b5f832a 100644 --- a/src/decoder/plugins/OpusHead.hxx +++ b/src/decoder/plugins/OpusHead.hxx @@ -23,6 +23,7 @@ #include bool -ScanOpusHeader(const void *data, size_t size, unsigned &channels_r); +ScanOpusHeader(const void *data, size_t size, unsigned &channels_r, + unsigned &pre_skip_r); #endif From 760238fe16e496eeb8ecbc4ca8784a3deb76e52e Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 1 Jul 2020 17:53:23 +0200 Subject: [PATCH 05/20] decoder/opus: apply pre-skip (RFC7845 4.2) Fixes the first part of https://github.com/MusicPlayerDaemon/MPD/issues/867 --- NEWS | 2 ++ src/decoder/plugins/OpusDecoderPlugin.cxx | 35 ++++++++++++++++------- 2 files changed, 27 insertions(+), 10 deletions(-) diff --git a/NEWS b/NEWS index 308e70712..703ce8478 100644 --- a/NEWS +++ b/NEWS @@ -4,6 +4,8 @@ ver 0.21.25 (not yet released) * input - file: detect premature end of file - smbclient: don't send credentials to MPD clients +* decoder + - opus: apply pre-skip * output - osx: improve sample rate selection * Windows/Android: diff --git a/src/decoder/plugins/OpusDecoderPlugin.cxx b/src/decoder/plugins/OpusDecoderPlugin.cxx index 8a262b901..9043d76c0 100644 --- a/src/decoder/plugins/OpusDecoderPlugin.cxx +++ b/src/decoder/plugins/OpusDecoderPlugin.cxx @@ -75,6 +75,8 @@ class MPDOpusDecoder final : public OggDecoder { OpusDecoder *opus_decoder = nullptr; opus_int16 *output_buffer = nullptr; + unsigned pre_skip, skip; + /** * If non-zero, then a previous Opus stream has been found * already with this number of channels. If opus_decoder is @@ -136,11 +138,13 @@ MPDOpusDecoder::OnOggBeginning(const ogg_packet &packet) if (opus_decoder != nullptr || !IsOpusHead(packet)) throw std::runtime_error("BOS packet must be OpusHead"); - unsigned channels, pre_skip; + unsigned channels; if (!ScanOpusHeader(packet.packet, packet.bytes, channels, pre_skip) || !audio_valid_channel_count(channels)) throw std::runtime_error("Malformed BOS packet"); + skip = pre_skip; + assert(opus_decoder == nullptr); assert(IsInitialized() == (output_buffer != nullptr)); @@ -236,15 +240,25 @@ MPDOpusDecoder::HandleAudio(const ogg_packet &packet) opus_strerror(nframes)); if (nframes > 0) { + if (skip >= (unsigned)nframes) { + skip -= nframes; + return; + } + + const opus_int16 *data = output_buffer; + data += skip * previous_channels; + nframes -= skip; + skip = 0; + const size_t nbytes = nframes * frame_size; auto cmd = client.SubmitData(input_stream, - output_buffer, nbytes, + data, nbytes, 0); if (cmd != DecoderCommand::NONE) throw cmd; if (packet.granulepos > 0) - client.SubmitTimestamp(FloatDuration(packet.granulepos) + client.SubmitTimestamp(FloatDuration(packet.granulepos - pre_skip) / opus_sample_rate); } } @@ -260,6 +274,7 @@ MPDOpusDecoder::Seek(uint64_t where_frame) try { SeekGranulePos(where_granulepos); + skip = pre_skip; return true; } catch (...) { return false; @@ -302,10 +317,9 @@ mpd_opus_stream_decode(DecoderClient &client, static bool ReadAndParseOpusHead(OggSyncState &sync, OggStreamState &stream, - unsigned &channels) + unsigned &channels, unsigned &pre_skip) { ogg_packet packet; - unsigned pre_skip; return OggReadPacket(sync, stream, packet) && packet.b_o_s && IsOpusHead(packet) && @@ -329,11 +343,12 @@ ReadAndVisitOpusTags(OggSyncState &sync, OggStreamState &stream, static void VisitOpusDuration(InputStream &is, OggSyncState &sync, OggStreamState &stream, - TagHandler &handler) + ogg_int64_t pre_skip, TagHandler &handler) { ogg_packet packet; - if (OggSeekFindEOS(sync, stream, packet, is)) { + if (OggSeekFindEOS(sync, stream, packet, is) && + packet.granulepos >= pre_skip) { const auto duration = SongTime::FromScale(packet.granulepos, opus_sample_rate); @@ -353,15 +368,15 @@ mpd_opus_scan_stream(InputStream &is, TagHandler &handler) noexcept OggStreamState os(first_page); - unsigned channels; - if (!ReadAndParseOpusHead(oy, os, channels) || + unsigned channels, pre_skip; + if (!ReadAndParseOpusHead(oy, os, channels, pre_skip) || !ReadAndVisitOpusTags(oy, os, handler)) return false; handler.OnAudioFormat(AudioFormat(opus_sample_rate, SampleFormat::S16, channels)); - VisitOpusDuration(is, oy, os, handler); + VisitOpusDuration(is, oy, os, pre_skip, handler); return true; } From 5ca137c73cda80f4c1f43bbcd627b4eda4a169cd Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 1 Jul 2020 20:51:44 +0200 Subject: [PATCH 06/20] decoder/opus: add API docs --- src/decoder/plugins/OpusDecoderPlugin.cxx | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/src/decoder/plugins/OpusDecoderPlugin.cxx b/src/decoder/plugins/OpusDecoderPlugin.cxx index 9043d76c0..88e8098f0 100644 --- a/src/decoder/plugins/OpusDecoderPlugin.cxx +++ b/src/decoder/plugins/OpusDecoderPlugin.cxx @@ -75,7 +75,18 @@ class MPDOpusDecoder final : public OggDecoder { OpusDecoder *opus_decoder = nullptr; opus_int16 *output_buffer = nullptr; - unsigned pre_skip, skip; + /** + * The pre-skip value from the Opus header. Initialized by + * OnOggBeginning(). + */ + unsigned pre_skip; + + /** + * The number of decoded samples which shall be skipped. At + * the beginning of the file, this gets set to #pre_skip (by + * OnOggBeginning()), and may also be set while seeking. + */ + unsigned skip; /** * If non-zero, then a previous Opus stream has been found @@ -240,6 +251,7 @@ MPDOpusDecoder::HandleAudio(const ogg_packet &packet) opus_strerror(nframes)); if (nframes > 0) { + /* apply the "skip" value */ if (skip >= (unsigned)nframes) { skip -= nframes; return; @@ -250,6 +262,7 @@ MPDOpusDecoder::HandleAudio(const ogg_packet &packet) nframes -= skip; skip = 0; + /* submit decoded samples to the DecoderClient */ const size_t nbytes = nframes * frame_size; auto cmd = client.SubmitData(input_stream, data, nbytes, @@ -274,6 +287,12 @@ MPDOpusDecoder::Seek(uint64_t where_frame) try { SeekGranulePos(where_granulepos); + + /* since all frame numbers are offset by the file's + pre-skip value, we need to apply it here as well; + we could just seek to "where_frame+pre_skip" as + well, but I think by decoding those samples and + discard them, we're safer */ skip = pre_skip; return true; } catch (...) { From 46eab05045d48e580bc2787da17e92f8d7d2846a Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 1 Jul 2020 20:56:50 +0200 Subject: [PATCH 07/20] decoder/opus: allocate buffer only in the first chained song Fixes memory leak. That's what we get for --- NEWS | 1 + src/decoder/plugins/OpusDecoderPlugin.cxx | 8 ++++++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/NEWS b/NEWS index 703ce8478..0f86d2431 100644 --- a/NEWS +++ b/NEWS @@ -6,6 +6,7 @@ ver 0.21.25 (not yet released) - smbclient: don't send credentials to MPD clients * decoder - opus: apply pre-skip + - opus: fix memory leak * output - osx: improve sample rate selection * Windows/Android: diff --git a/src/decoder/plugins/OpusDecoderPlugin.cxx b/src/decoder/plugins/OpusDecoderPlugin.cxx index 88e8098f0..fc77b5454 100644 --- a/src/decoder/plugins/OpusDecoderPlugin.cxx +++ b/src/decoder/plugins/OpusDecoderPlugin.cxx @@ -192,8 +192,12 @@ MPDOpusDecoder::OnOggBeginning(const ogg_packet &packet) client.Ready(audio_format, eos_granulepos > 0, duration); frame_size = audio_format.GetFrameSize(); - output_buffer = new opus_int16[opus_output_buffer_frames - * audio_format.channels]; + if (output_buffer == nullptr) + /* note: if we ever support changing the channel count + in chained streams, we need to reallocate this + buffer instead of keeping it */ + output_buffer = new opus_int16[opus_output_buffer_frames + * audio_format.channels]; auto cmd = client.GetCommand(); if (cmd != DecoderCommand::NONE) From 4244e61214db8d421a09be353b2374f21fc33f2d Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 1 Jul 2020 21:19:18 +0200 Subject: [PATCH 08/20] decoder/opus: simplify indentation in HandleAudio() --- src/decoder/plugins/OpusDecoderPlugin.cxx | 54 ++++++++++++----------- 1 file changed, 28 insertions(+), 26 deletions(-) diff --git a/src/decoder/plugins/OpusDecoderPlugin.cxx b/src/decoder/plugins/OpusDecoderPlugin.cxx index fc77b5454..34e8a9d7e 100644 --- a/src/decoder/plugins/OpusDecoderPlugin.cxx +++ b/src/decoder/plugins/OpusDecoderPlugin.cxx @@ -250,34 +250,36 @@ MPDOpusDecoder::HandleAudio(const ogg_packet &packet) packet.bytes, output_buffer, opus_output_buffer_frames, 0); - if (nframes < 0) - throw FormatRuntimeError("libopus error: %s", - opus_strerror(nframes)); - - if (nframes > 0) { - /* apply the "skip" value */ - if (skip >= (unsigned)nframes) { - skip -= nframes; + if (gcc_unlikely(nframes <= 0)) { + if (nframes < 0) + throw FormatRuntimeError("libopus error: %s", + opus_strerror(nframes)); + else return; - } - - const opus_int16 *data = output_buffer; - data += skip * previous_channels; - nframes -= skip; - skip = 0; - - /* submit decoded samples to the DecoderClient */ - const size_t nbytes = nframes * frame_size; - auto cmd = client.SubmitData(input_stream, - data, nbytes, - 0); - if (cmd != DecoderCommand::NONE) - throw cmd; - - if (packet.granulepos > 0) - client.SubmitTimestamp(FloatDuration(packet.granulepos - pre_skip) - / opus_sample_rate); } + + /* apply the "skip" value */ + if (skip >= (unsigned)nframes) { + skip -= nframes; + return; + } + + const opus_int16 *data = output_buffer; + data += skip * previous_channels; + nframes -= skip; + skip = 0; + + /* submit decoded samples to the DecoderClient */ + const size_t nbytes = nframes * frame_size; + auto cmd = client.SubmitData(input_stream, + data, nbytes, + 0); + if (cmd != DecoderCommand::NONE) + throw cmd; + + if (packet.granulepos > 0) + client.SubmitTimestamp(FloatDuration(packet.granulepos - pre_skip) + / opus_sample_rate); } bool From 7befab7e830f830ffbda43959d136c57a392a608 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 1 Jul 2020 21:15:59 +0200 Subject: [PATCH 09/20] decoder/opus: keep track of the granulepos Will be needed for End Trimming (RFC7845 4.4, https://github.com/MusicPlayerDaemon/MPD/issues/867). --- src/decoder/plugins/OpusDecoderPlugin.cxx | 29 +++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/src/decoder/plugins/OpusDecoderPlugin.cxx b/src/decoder/plugins/OpusDecoderPlugin.cxx index 34e8a9d7e..ed4ec3d01 100644 --- a/src/decoder/plugins/OpusDecoderPlugin.cxx +++ b/src/decoder/plugins/OpusDecoderPlugin.cxx @@ -98,6 +98,13 @@ class MPDOpusDecoder final : public OggDecoder { size_t frame_size; + /** + * The granulepos of the next sample to be submitted to + * DecoderClient::SubmitData(). Negative if unkown. + * Initialized by OnOggBeginning(). + */ + ogg_int64_t granulepos; + public: explicit MPDOpusDecoder(DecoderReader &reader) :OggDecoder(reader) {} @@ -114,6 +121,13 @@ public: bool Seek(uint64_t where_frame); private: + void AddGranulepos(ogg_int64_t n) noexcept { + assert(n >= 0); + + if (granulepos >= 0) + granulepos += n; + } + void HandleTags(const ogg_packet &packet); void HandleAudio(const ogg_packet &packet); @@ -154,6 +168,7 @@ MPDOpusDecoder::OnOggBeginning(const ogg_packet &packet) !audio_valid_channel_count(channels)) throw std::runtime_error("Malformed BOS packet"); + granulepos = 0; skip = pre_skip; assert(opus_decoder == nullptr); @@ -261,12 +276,14 @@ MPDOpusDecoder::HandleAudio(const ogg_packet &packet) /* apply the "skip" value */ if (skip >= (unsigned)nframes) { skip -= nframes; + AddGranulepos(nframes); return; } const opus_int16 *data = output_buffer; data += skip * previous_channels; nframes -= skip; + AddGranulepos(skip); skip = 0; /* submit decoded samples to the DecoderClient */ @@ -277,9 +294,12 @@ MPDOpusDecoder::HandleAudio(const ogg_packet &packet) if (cmd != DecoderCommand::NONE) throw cmd; - if (packet.granulepos > 0) - client.SubmitTimestamp(FloatDuration(packet.granulepos - pre_skip) + if (packet.granulepos > 0) { + granulepos = packet.granulepos; + client.SubmitTimestamp(FloatDuration(granulepos - pre_skip) / opus_sample_rate); + } else + AddGranulepos(nframes); } bool @@ -291,6 +311,11 @@ MPDOpusDecoder::Seek(uint64_t where_frame) const ogg_int64_t where_granulepos(where_frame); + /* we don't know the exact granulepos after seeking, so let's + set it to -1 - it will be set after the next packet which + declares its granulepos */ + granulepos = -1; + try { SeekGranulePos(where_granulepos); From faee5bbb78aa3a855fcecd49d111467dfbd40167 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 1 Jul 2020 21:21:38 +0200 Subject: [PATCH 10/20] decoder/opus: implement End Trimming (RFC7845 4.4) Closes https://github.com/MusicPlayerDaemon/MPD/issues/867 --- NEWS | 2 +- src/decoder/plugins/OpusDecoderPlugin.cxx | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index 0f86d2431..cfab38190 100644 --- a/NEWS +++ b/NEWS @@ -5,7 +5,7 @@ ver 0.21.25 (not yet released) - file: detect premature end of file - smbclient: don't send credentials to MPD clients * decoder - - opus: apply pre-skip + - opus: apply pre-skip and end trimming - opus: fix memory leak * output - osx: improve sample rate selection diff --git a/src/decoder/plugins/OpusDecoderPlugin.cxx b/src/decoder/plugins/OpusDecoderPlugin.cxx index ed4ec3d01..fa448a2ff 100644 --- a/src/decoder/plugins/OpusDecoderPlugin.cxx +++ b/src/decoder/plugins/OpusDecoderPlugin.cxx @@ -286,6 +286,23 @@ MPDOpusDecoder::HandleAudio(const ogg_packet &packet) AddGranulepos(skip); skip = 0; + if (packet.e_o_s && packet.granulepos > 0 && granulepos >= 0) { + /* End Trimming (RFC7845 4.4): "The page with the 'end + of stream' flag set MAY have a granule position + that indicates the page contains less audio data + than would normally be returned by decoding up + through the final packet. This is used to end the + stream somewhere other than an even frame + boundary. [...] The remaining samples are + discarded. */ + ogg_int64_t remaining = packet.granulepos - granulepos; + if (remaining <= 0) + return; + + if (remaining < nframes) + nframes = remaining; + } + /* submit decoded samples to the DecoderClient */ const size_t nbytes = nframes * frame_size; auto cmd = client.SubmitData(input_stream, From cb49a03fd7b86d3ce19fc982338a32a9ac60ef79 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 8 May 2019 22:32:50 +0200 Subject: [PATCH 11/20] util/HugeAllocator: add `noexcept` --- src/util/HugeAllocator.hxx | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/util/HugeAllocator.hxx b/src/util/HugeAllocator.hxx index c1eb50ce3..f7891316f 100644 --- a/src/util/HugeAllocator.hxx +++ b/src/util/HugeAllocator.hxx @@ -153,17 +153,17 @@ public: explicit HugeArray(size_type _size) :buffer(Buffer::FromVoidFloor(HugeAllocate(sizeof(value_type) * _size))) {} - constexpr HugeArray(HugeArray &&other) + constexpr HugeArray(HugeArray &&other) noexcept :buffer(std::exchange(other.buffer, nullptr)) {} - ~HugeArray() { + ~HugeArray() noexcept { if (buffer != nullptr) { auto v = buffer.ToVoid(); HugeFree(v.data, v.size); } } - HugeArray &operator=(HugeArray &&other) { + HugeArray &operator=(HugeArray &&other) noexcept { std::swap(buffer, other.buffer); return *this; } @@ -178,64 +178,64 @@ public: HugeDiscard(v.data, v.size); } - constexpr bool operator==(std::nullptr_t) const { + constexpr bool operator==(std::nullptr_t) const noexcept { return buffer == nullptr; } - constexpr bool operator!=(std::nullptr_t) const { + constexpr bool operator!=(std::nullptr_t) const noexcept { return buffer != nullptr; } /** * Returns the number of allocated elements. */ - constexpr size_type size() const { + constexpr size_type size() const noexcept { return buffer.size; } - reference front() { + reference front() noexcept { return buffer.front(); } - const_reference front() const { + const_reference front() const noexcept { return buffer.front(); } - reference back() { + reference back() noexcept { return buffer.back(); } - const_reference back() const { + const_reference back() const noexcept { return buffer.back(); } /** * Returns one element. No bounds checking. */ - reference operator[](size_type i) { + reference operator[](size_type i) noexcept { return buffer[i]; } /** * Returns one constant element. No bounds checking. */ - const_reference operator[](size_type i) const { + const_reference operator[](size_type i) const noexcept { return buffer[i]; } - iterator begin() { + iterator begin() noexcept { return buffer.begin(); } - constexpr const_iterator begin() const { + constexpr const_iterator begin() const noexcept { return buffer.cbegin(); } - iterator end() { + iterator end() noexcept { return buffer.end(); } - constexpr const_iterator end() const { + constexpr const_iterator end() const noexcept { return buffer.cend(); } }; From df38e7565b447b1bcdc02d01818fad6aa1979301 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 8 May 2019 22:33:41 +0200 Subject: [PATCH 12/20] util/HugeAllocator: import std::swap() --- src/util/HugeAllocator.cxx | 2 +- src/util/HugeAllocator.hxx | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/util/HugeAllocator.cxx b/src/util/HugeAllocator.cxx index 99afe23f7..df6f106b1 100644 --- a/src/util/HugeAllocator.cxx +++ b/src/util/HugeAllocator.cxx @@ -1,5 +1,5 @@ /* - * Copyright (C) 2013-2017 Max Kellermann + * Copyright 2013-2019 Max Kellermann * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions diff --git a/src/util/HugeAllocator.hxx b/src/util/HugeAllocator.hxx index f7891316f..6ae68f78e 100644 --- a/src/util/HugeAllocator.hxx +++ b/src/util/HugeAllocator.hxx @@ -1,5 +1,5 @@ /* - * Copyright (C) 2013-2017 Max Kellermann + * Copyright 2013-2019 Max Kellermann * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions @@ -164,7 +164,8 @@ public: } HugeArray &operator=(HugeArray &&other) noexcept { - std::swap(buffer, other.buffer); + using std::swap; + swap(buffer, other.buffer); return *this; } From 191919d1b1b43b7fc3467790a48e7b789a853742 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 3 Jul 2019 22:21:43 +0200 Subject: [PATCH 13/20] output/osx: remove trailing newline from exception messages --- src/output/plugins/OSXOutputPlugin.cxx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/output/plugins/OSXOutputPlugin.cxx b/src/output/plugins/OSXOutputPlugin.cxx index af4ad56d3..5ee21e9ff 100644 --- a/src/output/plugins/OSXOutputPlugin.cxx +++ b/src/output/plugins/OSXOutputPlugin.cxx @@ -418,7 +418,7 @@ osx_output_set_device_format(AudioDeviceID dev_id, &aopa_device_streams, 0, NULL, &property_size); if (err != noErr) - throw FormatRuntimeError("Cannot get number of streams: %d\n", err); + throw FormatRuntimeError("Cannot get number of streams: %d", err); const size_t n_streams = property_size / sizeof(AudioStreamID); static constexpr size_t MAX_STREAMS = 64; @@ -429,7 +429,7 @@ osx_output_set_device_format(AudioDeviceID dev_id, err = AudioObjectGetPropertyData(dev_id, &aopa_device_streams, 0, NULL, &property_size, streams); if (err != noErr) - throw FormatRuntimeError("Cannot get streams: %d\n", err); + throw FormatRuntimeError("Cannot get streams: %d", err); bool format_found = false; int output_stream; @@ -446,7 +446,7 @@ osx_output_set_device_format(AudioDeviceID dev_id, &property_size, &direction); if (err != noErr) - throw FormatRuntimeError("Cannot get streams direction: %d\n", + throw FormatRuntimeError("Cannot get streams direction: %d", err); if (direction != 0) @@ -507,7 +507,7 @@ osx_output_set_device_format(AudioDeviceID dev_id, sizeof(output_format), &output_format); if (err != noErr) - throw FormatRuntimeError("Failed to change the stream format: %d\n", + throw FormatRuntimeError("Failed to change the stream format: %d", err); } From d8a74802d1af164edd30b64e75bbd3fe78671585 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 28 May 2020 14:06:44 +0200 Subject: [PATCH 14/20] apple/StringRef: new library wrapping CFStringRef --- src/apple/StringRef.hxx | 73 ++++++++++++++++++++++++++ src/output/plugins/OSXOutputPlugin.cxx | 20 +++---- 2 files changed, 80 insertions(+), 13 deletions(-) create mode 100644 src/apple/StringRef.hxx diff --git a/src/apple/StringRef.hxx b/src/apple/StringRef.hxx new file mode 100644 index 000000000..72afcd1a9 --- /dev/null +++ b/src/apple/StringRef.hxx @@ -0,0 +1,73 @@ +/* + * Copyright 2020 Max Kellermann + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * + * - Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * + * - Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the + * distribution. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS + * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE + * FOUNDATION OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR + * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, + * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED + * OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +#ifndef APPLE_STRING_REF_HXX +#define APPLE_STRING_REF_HXX + +#include + +#include + +namespace Apple { + +class StringRef { + CFStringRef ref = nullptr; + +public: + explicit StringRef(CFStringRef _ref) noexcept + :ref(_ref) {} + + StringRef(StringRef &&src) noexcept + :ref(std::exchange(src.ref, nullptr)) {} + + ~StringRef() noexcept { + if (ref) + CFRelease(ref); + } + + StringRef &operator=(StringRef &&src) noexcept { + using std::swap; + swap(ref, src.ref); + return *this; + } + + operator bool() const noexcept { + return ref != nullptr; + } + + bool GetCString(char *buffer, std::size_t size, + CFStringEncoding encoding=kCFStringEncodingUTF8) const noexcept + { + return CFStringGetCString(ref, buffer, size, encoding); + } +}; + +} // namespace Apple + +#endif diff --git a/src/output/plugins/OSXOutputPlugin.cxx b/src/output/plugins/OSXOutputPlugin.cxx index 5ee21e9ff..33460ed0d 100644 --- a/src/output/plugins/OSXOutputPlugin.cxx +++ b/src/output/plugins/OSXOutputPlugin.cxx @@ -17,11 +17,11 @@ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. */ +#include "apple/StringRef.hxx" #include "config.h" #include "OSXOutputPlugin.hxx" #include "../OutputAPI.hxx" #include "mixer/MixerList.hxx" -#include "util/ScopeExit.hxx" #include "util/RuntimeError.hxx" #include "util/Domain.hxx" #include "util/Manual.hxx" @@ -111,15 +111,13 @@ static void osx_os_status_to_cstring(OSStatus status, char *str, size_t size) { CFErrorRef cferr = CFErrorCreate(nullptr, kCFErrorDomainOSStatus, status, nullptr); - CFStringRef cfstr = CFErrorCopyDescription(cferr); - if (!CFStringGetCString(cfstr, str, size, kCFStringEncodingUTF8)) { + const Apple::StringRef cfstr(CFErrorCopyDescription(cferr)); + if (!cfstr.GetCString(str, size)) { /* conversion failed, return empty string */ *str = '\0'; } if (cferr) CFRelease(cferr); - if (cfstr) - CFRelease(cfstr); } static bool @@ -629,12 +627,6 @@ osx_output_set_device(OSXOutput *oo) { OSStatus status; UInt32 size, numdevices; - CFStringRef cfname = nullptr; - - AtScopeExit(&cfname) { - if (cfname) - CFRelease(cfname); - }; if (oo->component_subtype != kAudioUnitSubType_HALOutput) return; @@ -678,6 +670,7 @@ osx_output_set_device(OSXOutput *oo) unsigned i; size = sizeof(CFStringRef); for (i = 0; i < numdevices; i++) { + CFStringRef cfname = nullptr; status = AudioObjectGetPropertyData(deviceids[i], &aopa_name, 0, nullptr, &size, &cfname); @@ -690,9 +683,10 @@ osx_output_set_device(OSXOutput *oo) errormsg); } + const Apple::StringRef cfname_(cfname); + char name[256]; - if (!CFStringGetCString(cfname, name, sizeof(name), - kCFStringEncodingUTF8)) + if (!cfname_.GetCString(name, sizeof(name))) throw std::runtime_error("Unable to convert device name from CFStringRef to char*"); if (strcmp(oo->device_name, name) == 0) { From 3d03683e7d38d933bf88a3e2a8d37832cc048472 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 26 Sep 2019 14:44:48 +0200 Subject: [PATCH 15/20] output: use StringIsEqual() --- src/output/Init.cxx | 7 ++++--- src/output/MultipleOutputs.cxx | 3 ++- src/output/Registry.cxx | 5 ++--- src/output/plugins/AoOutputPlugin.cxx | 5 ++--- src/output/plugins/OSXOutputPlugin.cxx | 7 ++++--- src/output/plugins/ShoutOutputPlugin.cxx | 19 +++++++++---------- 6 files changed, 23 insertions(+), 23 deletions(-) diff --git a/src/output/Init.cxx b/src/output/Init.cxx index 116d417c3..64f9cfe73 100644 --- a/src/output/Init.cxx +++ b/src/output/Init.cxx @@ -39,6 +39,7 @@ #include "config/Option.hxx" #include "config/Block.hxx" #include "util/RuntimeError.hxx" +#include "util/StringAPI.hxx" #include "util/StringFormat.hxx" #include "Log.hxx" @@ -214,7 +215,7 @@ FilteredAudioOutput::Setup(EventLoop &event_loop, const char *replay_gain_handler = block.GetBlockValue("replay_gain_handler", "software"); - if (strcmp(replay_gain_handler, "none") != 0) { + if (!StringIsEqual(replay_gain_handler, "none")) { prepared_replay_gain_filter = NewReplayGainFilter(replay_gain_config); assert(prepared_replay_gain_filter != nullptr); @@ -240,14 +241,14 @@ FilteredAudioOutput::Setup(EventLoop &event_loop, /* use the hardware mixer for replay gain? */ - if (strcmp(replay_gain_handler, "mixer") == 0) { + if (StringIsEqual(replay_gain_handler, "mixer")) { if (mixer != nullptr) replay_gain_filter_set_mixer(*prepared_replay_gain_filter, mixer, 100); else FormatError(output_domain, "No such mixer for output '%s'", name); - } else if (strcmp(replay_gain_handler, "software") != 0 && + } else if (!StringIsEqual(replay_gain_handler, "software") && prepared_replay_gain_filter != nullptr) { throw std::runtime_error("Invalid \"replay_gain_handler\" value"); } diff --git a/src/output/MultipleOutputs.cxx b/src/output/MultipleOutputs.cxx index 12c102618..fe3c9ad5f 100644 --- a/src/output/MultipleOutputs.cxx +++ b/src/output/MultipleOutputs.cxx @@ -28,6 +28,7 @@ #include "config/Data.hxx" #include "config/Option.hxx" #include "util/RuntimeError.hxx" +#include "util/StringAPI.hxx" #include @@ -147,7 +148,7 @@ AudioOutputControl * MultipleOutputs::FindByName(const char *name) noexcept { for (auto *i : outputs) - if (strcmp(i->GetName(), name) == 0) + if (StringIsEqual(i->GetName(), name)) return i; return nullptr; diff --git a/src/output/Registry.cxx b/src/output/Registry.cxx index 035a26130..aa372289e 100644 --- a/src/output/Registry.cxx +++ b/src/output/Registry.cxx @@ -38,8 +38,7 @@ #include "plugins/sles/SlesOutputPlugin.hxx" #include "plugins/SolarisOutputPlugin.hxx" #include "plugins/WinmmOutputPlugin.hxx" - -#include +#include "util/StringAPI.hxx" const AudioOutputPlugin *const audio_output_plugins[] = { #ifdef HAVE_SHOUT @@ -101,7 +100,7 @@ const AudioOutputPlugin * AudioOutputPlugin_get(const char *name) { audio_output_plugins_for_each(plugin) - if (strcmp(plugin->name, name) == 0) + if (StringIsEqual(plugin->name, name)) return plugin; return nullptr; diff --git a/src/output/plugins/AoOutputPlugin.cxx b/src/output/plugins/AoOutputPlugin.cxx index 10fdd130d..8f859aba7 100644 --- a/src/output/plugins/AoOutputPlugin.cxx +++ b/src/output/plugins/AoOutputPlugin.cxx @@ -25,12 +25,11 @@ #include "util/SplitString.hxx" #include "util/RuntimeError.hxx" #include "util/Domain.hxx" +#include "util/StringAPI.hxx" #include "Log.hxx" #include -#include - /* An ao_sample_format, with all fields set to zero: */ static ao_sample_format OUR_AO_FORMAT_INITIALIZER; @@ -105,7 +104,7 @@ AoOutput::AoOutput(const ConfigBlock &block) write_size(block.GetPositiveValue("write_size", 1024U)) { const char *value = block.GetBlockValue("driver", "default"); - if (0 == strcmp(value, "default")) + if (StringIsEqual(value, "default")) driver = ao_default_driver_id(); else driver = ao_driver_id(value); diff --git a/src/output/plugins/OSXOutputPlugin.cxx b/src/output/plugins/OSXOutputPlugin.cxx index 33460ed0d..c57e86b73 100644 --- a/src/output/plugins/OSXOutputPlugin.cxx +++ b/src/output/plugins/OSXOutputPlugin.cxx @@ -30,6 +30,7 @@ #include "thread/Mutex.hxx" #include "thread/Cond.hxx" #include "util/ByteOrder.hxx" +#include "util/StringAPI.hxx" #include "util/StringBuffer.hxx" #include "util/StringFormat.hxx" #include "Log.hxx" @@ -138,11 +139,11 @@ OSXOutput::OSXOutput(const ConfigBlock &block) { const char *device = block.GetBlockValue("device"); - if (device == nullptr || 0 == strcmp(device, "default")) { + if (device == nullptr || StringIsEqual(device, "default")) { component_subtype = kAudioUnitSubType_DefaultOutput; device_name = nullptr; } - else if (0 == strcmp(device, "system")) { + else if (StringIsEqual(device, "system")) { component_subtype = kAudioUnitSubType_SystemOutput; device_name = nullptr; } @@ -689,7 +690,7 @@ osx_output_set_device(OSXOutput *oo) if (!cfname_.GetCString(name, sizeof(name))) throw std::runtime_error("Unable to convert device name from CFStringRef to char*"); - if (strcmp(oo->device_name, name) == 0) { + if (StringIsEqual(oo->device_name, name)) { FormatDebug(osx_output_domain, "found matching device: ID=%u, name=%s", (unsigned)deviceids[i], name); diff --git a/src/output/plugins/ShoutOutputPlugin.cxx b/src/output/plugins/ShoutOutputPlugin.cxx index e5aaa63e6..3811fc596 100644 --- a/src/output/plugins/ShoutOutputPlugin.cxx +++ b/src/output/plugins/ShoutOutputPlugin.cxx @@ -35,7 +35,6 @@ #include #include -#include #include static constexpr unsigned DEFAULT_CONN_TIMEOUT = 2; @@ -123,15 +122,15 @@ ShoutOutput::ShoutOutput(const ConfigBlock &block) unsigned protocol; const char *value = block.GetBlockValue("protocol"); if (value != nullptr) { - if (0 == strcmp(value, "shoutcast") && + if (StringIsEqual(value, "shoutcast") && !StringIsEqual(mime_type, "audio/mpeg")) throw FormatRuntimeError("you cannot stream \"%s\" to shoutcast, use mp3", mime_type); - else if (0 == strcmp(value, "shoutcast")) + else if (StringIsEqual(value, "shoutcast")) protocol = SHOUT_PROTOCOL_ICY; - else if (0 == strcmp(value, "icecast1")) + else if (StringIsEqual(value, "icecast1")) protocol = SHOUT_PROTOCOL_XAUDIOCAST; - else if (0 == strcmp(value, "icecast2")) + else if (StringIsEqual(value, "icecast2")) protocol = SHOUT_PROTOCOL_HTTP; else throw FormatRuntimeError("shout protocol \"%s\" is not \"shoutcast\" or " @@ -145,15 +144,15 @@ ShoutOutput::ShoutOutput(const ConfigBlock &block) unsigned tls; value = block.GetBlockValue("tls"); if (value != nullptr) { - if (0 == strcmp(value, "disabled")) + if (StringIsEqual(value, "disabled")) tls = SHOUT_TLS_DISABLED; - else if(0 == strcmp(value, "auto")) + else if (StringIsEqual(value, "auto")) tls = SHOUT_TLS_AUTO; - else if(0 == strcmp(value, "auto_no_plain")) + else if (StringIsEqual(value, "auto_no_plain")) tls = SHOUT_TLS_AUTO_NO_PLAIN; - else if(0 == strcmp(value, "rfc2818")) + else if (StringIsEqual(value, "rfc2818")) tls = SHOUT_TLS_RFC2818; - else if(0 == strcmp(value, "rfc2817")) + else if (StringIsEqual(value, "rfc2817")) tls = SHOUT_TLS_RFC2817; else throw FormatRuntimeError("invalid shout TLS option \"%s\"", value); From 90d85319c2bd9bc957291380e33c4ea0a7c3328e Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 28 May 2020 14:50:07 +0200 Subject: [PATCH 16/20] apple/ErrorRef: new library wrapping CFErrorRef --- src/apple/ErrorRef.hxx | 75 ++++++++++++++++++++++++++ src/output/plugins/OSXOutputPlugin.cxx | 9 ++-- 2 files changed, 79 insertions(+), 5 deletions(-) create mode 100644 src/apple/ErrorRef.hxx diff --git a/src/apple/ErrorRef.hxx b/src/apple/ErrorRef.hxx new file mode 100644 index 000000000..f8ff57bac --- /dev/null +++ b/src/apple/ErrorRef.hxx @@ -0,0 +1,75 @@ +/* + * Copyright 2020 Max Kellermann + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * + * - Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * + * - Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the + * distribution. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS + * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE + * FOUNDATION OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR + * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, + * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED + * OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +#ifndef APPLE_ERROR_REF_HXX +#define APPLE_ERROR_REF_HXX + +#include + +#include + +namespace Apple { + +class ErrorRef { + CFErrorRef ref = nullptr; + +public: + explicit ErrorRef(CFErrorRef _ref) noexcept + :ref(_ref) {} + + ErrorRef(CFAllocatorRef allocator, CFErrorDomain domain, + CFIndex code, CFDictionaryRef userInfo) noexcept + :ref(CFErrorCreate(allocator, domain, code, userInfo)) {} + + ErrorRef(ErrorRef &&src) noexcept + :ref(std::exchange(src.ref, nullptr)) {} + + ~ErrorRef() noexcept { + if (ref) + CFRelease(ref); + } + + ErrorRef &operator=(ErrorRef &&src) noexcept { + using std::swap; + swap(ref, src.ref); + return *this; + } + + operator bool() const noexcept { + return ref != nullptr; + } + + CFStringRef CopyDescription() const noexcept { + return CFErrorCopyDescription(ref); + } +}; + +} // namespace Apple + +#endif diff --git a/src/output/plugins/OSXOutputPlugin.cxx b/src/output/plugins/OSXOutputPlugin.cxx index c57e86b73..af9fd79d4 100644 --- a/src/output/plugins/OSXOutputPlugin.cxx +++ b/src/output/plugins/OSXOutputPlugin.cxx @@ -17,9 +17,10 @@ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. */ -#include "apple/StringRef.hxx" #include "config.h" #include "OSXOutputPlugin.hxx" +#include "apple/ErrorRef.hxx" +#include "apple/StringRef.hxx" #include "../OutputAPI.hxx" #include "mixer/MixerList.hxx" #include "util/RuntimeError.hxx" @@ -111,14 +112,12 @@ static constexpr Domain osx_output_domain("osx_output"); static void osx_os_status_to_cstring(OSStatus status, char *str, size_t size) { - CFErrorRef cferr = CFErrorCreate(nullptr, kCFErrorDomainOSStatus, status, nullptr); - const Apple::StringRef cfstr(CFErrorCopyDescription(cferr)); + Apple::ErrorRef cferr(nullptr, kCFErrorDomainOSStatus, status, nullptr); + const Apple::StringRef cfstr(cferr.CopyDescription()); if (!cfstr.GetCString(str, size)) { /* conversion failed, return empty string */ *str = '\0'; } - if (cferr) - CFRelease(cferr); } static bool From bbceb5eb91702dd83b1552bba5ea8b1bef3ead42 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 28 May 2020 15:02:14 +0200 Subject: [PATCH 17/20] output/osx: silently ignore some errors in osx_output_set_device() --- src/output/plugins/OSXOutputPlugin.cxx | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/output/plugins/OSXOutputPlugin.cxx b/src/output/plugins/OSXOutputPlugin.cxx index af9fd79d4..7136da5ad 100644 --- a/src/output/plugins/OSXOutputPlugin.cxx +++ b/src/output/plugins/OSXOutputPlugin.cxx @@ -674,20 +674,14 @@ osx_output_set_device(OSXOutput *oo) status = AudioObjectGetPropertyData(deviceids[i], &aopa_name, 0, nullptr, &size, &cfname); - if (status != noErr) { - char errormsg[1024]; - osx_os_status_to_cstring(status, errormsg, sizeof(errormsg)); - throw FormatRuntimeError("Unable to determine OS X device name " - "(device %u): %s", - (unsigned int) deviceids[i], - errormsg); - } + if (status != noErr) + continue; const Apple::StringRef cfname_(cfname); char name[256]; if (!cfname_.GetCString(name, sizeof(name))) - throw std::runtime_error("Unable to convert device name from CFStringRef to char*"); + continue; if (StringIsEqual(oo->device_name, name)) { FormatDebug(osx_output_domain, From 346084da1edc6a14e07c147d04915bee381de513 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 28 May 2020 14:54:30 +0200 Subject: [PATCH 18/20] apple/Throw: new helper library replacing osx_os_status_to_cstring() --- src/apple/Throw.cxx | 67 ++++++++++++++++ src/apple/Throw.hxx | 45 +++++++++++ src/output/plugins/OSXOutputPlugin.cxx | 104 +++++++------------------ src/output/plugins/meson.build | 5 +- 4 files changed, 143 insertions(+), 78 deletions(-) create mode 100644 src/apple/Throw.cxx create mode 100644 src/apple/Throw.hxx diff --git a/src/apple/Throw.cxx b/src/apple/Throw.cxx new file mode 100644 index 000000000..9f6fd7a58 --- /dev/null +++ b/src/apple/Throw.cxx @@ -0,0 +1,67 @@ +/* + * Copyright 2020 Max Kellermann + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * + * - Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * + * - Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the + * distribution. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS + * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE + * FOUNDATION OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR + * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, + * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED + * OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +#include "Throw.hxx" +#include "ErrorRef.hxx" +#include "StringRef.hxx" + +#include + +namespace Apple { + +void +ThrowOSStatus(OSStatus status) +{ + const Apple::ErrorRef cferr(nullptr, kCFErrorDomainOSStatus, + status, nullptr); + const Apple::StringRef cfstr(cferr.CopyDescription()); + + char msg[1024]; + if (!cfstr.GetCString(msg, sizeof(msg))) + throw std::runtime_error("Unknown OSStatus"); + + throw std::runtime_error(msg); +} + +void +ThrowOSStatus(OSStatus status, const char *_msg) +{ + const Apple::ErrorRef cferr(nullptr, kCFErrorDomainOSStatus, + status, nullptr); + const Apple::StringRef cfstr(cferr.CopyDescription()); + + char msg[1024]; + strcpy(msg, _msg); + size_t length = strlen(msg); + + cfstr.GetCString(msg + length, sizeof(msg) - length); + throw std::runtime_error(msg); +} + +} // namespace Apple diff --git a/src/apple/Throw.hxx b/src/apple/Throw.hxx new file mode 100644 index 000000000..af4bb6932 --- /dev/null +++ b/src/apple/Throw.hxx @@ -0,0 +1,45 @@ +/* + * Copyright 2020 Max Kellermann + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * + * - Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * + * - Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the + * distribution. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS + * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE + * FOUNDATION OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR + * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, + * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED + * OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +#ifndef APPLE_THROW_HXX +#define APPLE_THROW_HXX + +#include + +namespace Apple { + +void +ThrowOSStatus(OSStatus status); + +void +ThrowOSStatus(OSStatus status, const char *msg); + +} // namespace Apple + +#endif diff --git a/src/output/plugins/OSXOutputPlugin.cxx b/src/output/plugins/OSXOutputPlugin.cxx index 7136da5ad..712948b7b 100644 --- a/src/output/plugins/OSXOutputPlugin.cxx +++ b/src/output/plugins/OSXOutputPlugin.cxx @@ -19,8 +19,8 @@ #include "config.h" #include "OSXOutputPlugin.hxx" -#include "apple/ErrorRef.hxx" #include "apple/StringRef.hxx" +#include "apple/Throw.hxx" #include "../OutputAPI.hxx" #include "mixer/MixerList.hxx" #include "util/RuntimeError.hxx" @@ -109,17 +109,6 @@ private: static constexpr Domain osx_output_domain("osx_output"); -static void -osx_os_status_to_cstring(OSStatus status, char *str, size_t size) -{ - Apple::ErrorRef cferr(nullptr, kCFErrorDomainOSStatus, status, nullptr); - const Apple::StringRef cfstr(cferr.CopyDescription()); - if (!cfstr.GetCString(str, size)) { - /* conversion failed, return empty string */ - *str = '\0'; - } -} - static bool osx_output_test_default_device() { @@ -209,11 +198,8 @@ OSXOutput::GetVolume() &size, &vol); - if (status != noErr) { - char errormsg[1024]; - osx_os_status_to_cstring(status, errormsg, sizeof(errormsg)); - throw FormatRuntimeError("unable to get volume: %s", errormsg); - } + if (status != noErr) + Apple::ThrowOSStatus(status); return static_cast(vol * 100.0); } @@ -235,12 +221,8 @@ OSXOutput::SetVolume(unsigned new_volume) size, &vol); - if (status != noErr) { - char errormsg[1024]; - osx_os_status_to_cstring(status, errormsg, sizeof(errormsg)); - throw FormatRuntimeError( "unable to set new volume %u: %s", - new_volume, errormsg); - } + if (status != noErr) + Apple::ThrowOSStatus(status); } static void @@ -304,12 +286,9 @@ osx_output_set_channel_map(OSXOutput *oo) 0, &desc, &size); - if (status != noErr) { - char errormsg[1024]; - osx_os_status_to_cstring(status, errormsg, sizeof(errormsg)); - throw FormatRuntimeError("%s: unable to get number of output device channels: %s", - oo->device_name, errormsg); - } + if (status != noErr) + Apple::ThrowOSStatus(status, + "unable to get number of output device channels"); UInt32 num_channels = desc.mChannelsPerFrame; std::unique_ptr channel_map(new SInt32[num_channels]); @@ -325,11 +304,8 @@ osx_output_set_channel_map(OSXOutput *oo) 0, channel_map.get(), size); - if (status != noErr) { - char errormsg[1024]; - osx_os_status_to_cstring(status, errormsg, sizeof(errormsg)); - throw FormatRuntimeError("%s: unable to set channel map: %s", oo->device_name, errormsg); - } + if (status != noErr) + Apple::ThrowOSStatus(status, "unable to set channel map"); } @@ -640,12 +616,8 @@ osx_output_set_device(OSXOutput *oo) status = AudioObjectGetPropertyDataSize(kAudioObjectSystemObject, &aopa_hw_devices, 0, nullptr, &size); - if (status != noErr) { - char errormsg[1024]; - osx_os_status_to_cstring(status, errormsg, sizeof(errormsg)); - throw FormatRuntimeError("Unable to determine number of OS X audio devices: %s", - errormsg); - } + if (status != noErr) + Apple::ThrowOSStatus(status); /* what are the available audio device IDs? */ numdevices = size / sizeof(AudioDeviceID); @@ -653,12 +625,8 @@ osx_output_set_device(OSXOutput *oo) status = AudioObjectGetPropertyData(kAudioObjectSystemObject, &aopa_hw_devices, 0, nullptr, &size, deviceids.get()); - if (status != noErr) { - char errormsg[1024]; - osx_os_status_to_cstring(status, errormsg, sizeof(errormsg)); - throw FormatRuntimeError("Unable to determine OS X audio device IDs: %s", - errormsg); - } + if (status != noErr) + Apple::ThrowOSStatus(status); /* which audio device matches oo->device_name? */ static constexpr AudioObjectPropertyAddress aopa_name{ @@ -701,12 +669,9 @@ osx_output_set_device(OSXOutput *oo) 0, &(deviceids[i]), sizeof(AudioDeviceID)); - if (status != noErr) { - char errormsg[1024]; - osx_os_status_to_cstring(status, errormsg, sizeof(errormsg)); - throw FormatRuntimeError("Unable to set OS X audio output device: %s", - errormsg); - } + if (status != noErr) + Apple::ThrowOSStatus(status, + "Unable to set OS X audio output device"); oo->dev_id = deviceids[i]; FormatDebug(osx_output_domain, @@ -757,12 +722,9 @@ OSXOutput::Enable() throw std::runtime_error("Error finding OS X component"); OSStatus status = AudioComponentInstanceNew(comp, &au); - if (status != noErr) { - char errormsg[1024]; - osx_os_status_to_cstring(status, errormsg, sizeof(errormsg)); - throw FormatRuntimeError("Unable to open OS X component: %s", - errormsg); - } + if (status != noErr) + Apple::ThrowOSStatus(status, "Unable to open OS X component"); + #ifdef ENABLE_DSD pcm_export.Construct(); #endif @@ -883,21 +845,13 @@ OSXOutput::Open(AudioFormat &audio_format) } status = AudioUnitInitialize(au); - if (status != noErr) { - char errormsg[1024]; - osx_os_status_to_cstring(status, errormsg, sizeof(errormsg)); - throw FormatRuntimeError("Unable to initialize OS X audio unit: %s", - errormsg); - } + if (status != noErr) + Apple::ThrowOSStatus(status, "Unable to initialize OS X audio unit"); UInt32 buffer_frame_size = 1; status = osx_output_set_buffer_size(au, asbd, &buffer_frame_size); - if (status != noErr) { - char errormsg[1024]; - osx_os_status_to_cstring(status, errormsg, sizeof(errormsg)); - throw FormatRuntimeError("Unable to set frame size: %s", - errormsg); - } + if (status != noErr) + Apple::ThrowOSStatus(status, "Unable to set frame size"); size_t ring_buffer_size = std::max(buffer_frame_size, MPD_OSX_BUFFER_TIME_MS * audio_format.GetFrameSize() * audio_format.sample_rate / 1000); @@ -912,13 +866,9 @@ OSXOutput::Open(AudioFormat &audio_format) ring_buffer = new boost::lockfree::spsc_queue(ring_buffer_size); status = AudioOutputUnitStart(au); - if (status != 0) { - AudioUnitUninitialize(au); - char errormsg[1024]; - osx_os_status_to_cstring(status, errormsg, sizeof(errormsg)); - throw FormatRuntimeError("Unable to start audio output: %s", - errormsg); - } + if (status != 0) + Apple::ThrowOSStatus(status, "Unable to start audio output"); + pause = false; } diff --git a/src/output/plugins/meson.build b/src/output/plugins/meson.build index bdfd47130..dde46da79 100644 --- a/src/output/plugins/meson.build +++ b/src/output/plugins/meson.build @@ -72,7 +72,10 @@ if enable_oss endif if is_darwin - output_plugins_sources += 'OSXOutputPlugin.cxx' + output_plugins_sources += [ + 'OSXOutputPlugin.cxx', + '../../apple/Throw.cxx', + ] audiounit_dep = declare_dependency( link_args: [ '-framework', 'AudioUnit', '-framework', 'CoreAudio', '-framework', 'CoreServices', From cdf706259708ad1a96193ce6ac01d9df9e40a9b0 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 28 May 2020 15:16:16 +0200 Subject: [PATCH 19/20] apple/AudioUnit: wrapper functions for AudioObject properties --- src/apple/AudioUnit.hxx | 97 +++++++++++++++++++++ src/output/plugins/OSXOutputPlugin.cxx | 115 ++++++------------------- 2 files changed, 122 insertions(+), 90 deletions(-) create mode 100644 src/apple/AudioUnit.hxx diff --git a/src/apple/AudioUnit.hxx b/src/apple/AudioUnit.hxx new file mode 100644 index 000000000..99b5b7bf2 --- /dev/null +++ b/src/apple/AudioUnit.hxx @@ -0,0 +1,97 @@ +/* + * Copyright 2020 Max Kellermann + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * + * - Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * + * - Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the + * distribution. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS + * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE + * FOUNDATION OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR + * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, + * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED + * OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +#ifndef APPLE_AUDIO_OBJECT_HXX +#define APPLE_AUDIO_OBJECT_HXX + +#include "Throw.hxx" +#include "util/AllocatedArray.hxx" + +#include + +#include + +std::size_t +AudioObjectGetPropertyDataSize(AudioObjectID inObjectID, + const AudioObjectPropertyAddress &inAddress) +{ + UInt32 size; + OSStatus status = AudioObjectGetPropertyDataSize(inObjectID, + &inAddress, + 0, nullptr, &size); + if (status != noErr) + Apple::ThrowOSStatus(status); + + return size; +} + +template +T +AudioObjectGetPropertyDataT(AudioObjectID inObjectID, + const AudioObjectPropertyAddress &inAddress) +{ + OSStatus status; + UInt32 size = sizeof(T); + T value; + + status = AudioObjectGetPropertyData(inObjectID, &inAddress, + 0, nullptr, + &size, &value); + if (status != noErr) + Apple::ThrowOSStatus(status); + + return value; +} + +template +AllocatedArray +AudioObjectGetPropertyDataArray(AudioObjectID inObjectID, + const AudioObjectPropertyAddress &inAddress) +{ + OSStatus status; + UInt32 size; + + status = AudioObjectGetPropertyDataSize(inObjectID, + &inAddress, + 0, nullptr, &size); + if (status != noErr) + Apple::ThrowOSStatus(status); + + AllocatedArray result(size / sizeof(T)); + + status = AudioObjectGetPropertyData(inObjectID, &inAddress, + 0, nullptr, + &size, result.begin()); + if (status != noErr) + Apple::ThrowOSStatus(status); + + return result; +} + +#endif diff --git a/src/output/plugins/OSXOutputPlugin.cxx b/src/output/plugins/OSXOutputPlugin.cxx index 712948b7b..fd2520f14 100644 --- a/src/output/plugins/OSXOutputPlugin.cxx +++ b/src/output/plugins/OSXOutputPlugin.cxx @@ -19,6 +19,7 @@ #include "config.h" #include "OSXOutputPlugin.hxx" +#include "apple/AudioUnit.hxx" #include "apple/StringRef.hxx" #include "apple/Throw.hxx" #include "../OutputAPI.hxx" @@ -184,22 +185,14 @@ OSXOutput::Create(EventLoop &, const ConfigBlock &block) int OSXOutput::GetVolume() { - Float32 vol; static constexpr AudioObjectPropertyAddress aopa = { kAudioHardwareServiceDeviceProperty_VirtualMasterVolume, kAudioObjectPropertyScopeOutput, kAudioObjectPropertyElementMaster, }; - UInt32 size = sizeof(vol); - OSStatus status = AudioObjectGetPropertyData(dev_id, - &aopa, - 0, - NULL, - &size, - &vol); - if (status != noErr) - Apple::ThrowOSStatus(status); + const auto vol = AudioObjectGetPropertyDataT(dev_id, + aopa); return static_cast(vol * 100.0); } @@ -387,65 +380,27 @@ osx_output_set_device_format(AudioDeviceID dev_id, kAudioObjectPropertyElementMaster }; - UInt32 property_size; - OSStatus err = AudioObjectGetPropertyDataSize(dev_id, - &aopa_device_streams, - 0, NULL, &property_size); - if (err != noErr) - throw FormatRuntimeError("Cannot get number of streams: %d", err); + OSStatus err; - const size_t n_streams = property_size / sizeof(AudioStreamID); - static constexpr size_t MAX_STREAMS = 64; - if (n_streams > MAX_STREAMS) - throw std::runtime_error("Too many streams"); - - AudioStreamID streams[MAX_STREAMS]; - err = AudioObjectGetPropertyData(dev_id, &aopa_device_streams, 0, NULL, - &property_size, streams); - if (err != noErr) - throw FormatRuntimeError("Cannot get streams: %d", err); + const auto streams = + AudioObjectGetPropertyDataArray(dev_id, + aopa_device_streams); bool format_found = false; int output_stream; AudioStreamBasicDescription output_format; - for (size_t i = 0; i < n_streams; i++) { - UInt32 direction; - AudioStreamID stream = streams[i]; - property_size = sizeof(direction); - err = AudioObjectGetPropertyData(stream, - &aopa_stream_direction, - 0, - NULL, - &property_size, - &direction); - if (err != noErr) - throw FormatRuntimeError("Cannot get streams direction: %d", - err); - + for (const auto stream : streams) { + const auto direction = + AudioObjectGetPropertyDataT(stream, + aopa_stream_direction); if (direction != 0) continue; - err = AudioObjectGetPropertyDataSize(stream, - &aopa_stream_phys_formats, - 0, NULL, &property_size); - if (err != noErr) - throw FormatRuntimeError("Unable to get format size s for stream %d. Error = %s", - streams[i], err); - - const size_t format_count = property_size / sizeof(AudioStreamRangedDescription); - static constexpr size_t MAX_FORMATS = 256; - if (format_count > MAX_FORMATS) - throw std::runtime_error("Too many formats"); - - AudioStreamRangedDescription format_list[MAX_FORMATS]; - err = AudioObjectGetPropertyData(stream, - &aopa_stream_phys_formats, - 0, NULL, - &property_size, format_list); - if (err != noErr) - throw FormatRuntimeError("Unable to get available formats for stream %d. Error = %s", - streams[i], err); + const auto format_list = + AudioObjectGetPropertyDataArray(stream, + aopa_stream_phys_formats); + const size_t format_count = format_list.size(); float output_score = 0; for (size_t j = 0; j < format_count; j++) { @@ -543,26 +498,14 @@ osx_output_set_buffer_size(AudioUnit au, AudioStreamBasicDescription desc, static void osx_output_hog_device(AudioDeviceID dev_id, bool hog) { - pid_t hog_pid; static constexpr AudioObjectPropertyAddress aopa = { kAudioDevicePropertyHogMode, kAudioObjectPropertyScopeOutput, kAudioObjectPropertyElementMaster }; - UInt32 size = sizeof(hog_pid); - OSStatus err = AudioObjectGetPropertyData(dev_id, - &aopa, - 0, - NULL, - &size, - &hog_pid); - if (err != noErr) { - FormatDebug(osx_output_domain, - "Cannot get hog information: %d", - err); - return; - } + pid_t hog_pid = AudioObjectGetPropertyDataT(dev_id, + aopa); if (hog) { if (hog_pid != -1) { FormatDebug(osx_output_domain, @@ -578,7 +521,8 @@ osx_output_hog_device(AudioDeviceID dev_id, bool hog) } hog_pid = hog ? getpid() : -1; - size = sizeof(hog_pid); + UInt32 size = sizeof(hog_pid); + OSStatus err; err = AudioObjectSetPropertyData(dev_id, &aopa, 0, @@ -602,31 +546,21 @@ static void osx_output_set_device(OSXOutput *oo) { OSStatus status; - UInt32 size, numdevices; + UInt32 size; if (oo->component_subtype != kAudioUnitSubType_HALOutput) return; - /* how many audio devices are there? */ + /* what are the available audio device IDs? */ static constexpr AudioObjectPropertyAddress aopa_hw_devices{ kAudioHardwarePropertyDevices, kAudioObjectPropertyScopeGlobal, kAudioObjectPropertyElementMaster, }; - status = AudioObjectGetPropertyDataSize(kAudioObjectSystemObject, - &aopa_hw_devices, 0, nullptr, &size); - if (status != noErr) - Apple::ThrowOSStatus(status); - - /* what are the available audio device IDs? */ - numdevices = size / sizeof(AudioDeviceID); - std::unique_ptr deviceids(new AudioDeviceID[numdevices]); - status = AudioObjectGetPropertyData(kAudioObjectSystemObject, - &aopa_hw_devices, 0, nullptr, - &size, deviceids.get()); - if (status != noErr) - Apple::ThrowOSStatus(status); + const auto deviceids = + AudioObjectGetPropertyDataArray(kAudioObjectSystemObject, + aopa_hw_devices); /* which audio device matches oo->device_name? */ static constexpr AudioObjectPropertyAddress aopa_name{ @@ -635,6 +569,7 @@ osx_output_set_device(OSXOutput *oo) kAudioObjectPropertyElementMaster, }; + const unsigned numdevices = deviceids.size(); unsigned i; size = sizeof(CFStringRef); for (i = 0; i < numdevices; i++) { From beeb02025e05995215e176b75d435e08fbde9d11 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 28 May 2020 15:57:37 +0200 Subject: [PATCH 20/20] output/osx: use range-based `for` --- src/output/plugins/OSXOutputPlugin.cxx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/output/plugins/OSXOutputPlugin.cxx b/src/output/plugins/OSXOutputPlugin.cxx index fd2520f14..6eeb4392b 100644 --- a/src/output/plugins/OSXOutputPlugin.cxx +++ b/src/output/plugins/OSXOutputPlugin.cxx @@ -400,11 +400,11 @@ osx_output_set_device_format(AudioDeviceID dev_id, const auto format_list = AudioObjectGetPropertyDataArray(stream, aopa_stream_phys_formats); - const size_t format_count = format_list.size(); float output_score = 0; - for (size_t j = 0; j < format_count; j++) { - AudioStreamBasicDescription format_desc = format_list[j].mFormat; + + for (const auto &format : format_list) { + AudioStreamBasicDescription format_desc = format.mFormat; std::string format_string; // for devices with kAudioStreamAnyRate