From e8f08cda53437220f7df3b43e412dfce0805d11d Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 5 Mar 2018 19:23:36 +0100 Subject: [PATCH 1/8] AUTHORS: add various recent contributors --- AUTHORS | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/AUTHORS b/AUTHORS index 14aa053fa..7625fcdda 100644 --- a/AUTHORS +++ b/AUTHORS @@ -31,3 +31,8 @@ The following people have contributed code to MPD: Jean-Francois Dockes Yue Wang Matthew Leon Grinshpun + Dimitris Papastamos + Florian Schlichting + François Revol + Jacob Vosmaer + Thomas Guillem From c745e14f4704ebfb0294e8b22a7ae503fc05c2a4 Mon Sep 17 00:00:00 2001 From: Michal Smucr Date: Fri, 9 Mar 2018 00:17:55 +0100 Subject: [PATCH 2/8] Bump minimum required version of Boost to 1.54. lockfree library used by ALSA output plugin is part of Boost from version 1.53, so this can be theoretically the lowest required version, however there are issues which are resolved from 1.54 onwards. --- configure.ac | 2 +- doc/user.xml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index 7e4041a57..06daf6a0b 100644 --- a/configure.ac +++ b/configure.ac @@ -454,7 +454,7 @@ dnl --------------------------------------------------------------------------- dnl Mandatory Libraries dnl --------------------------------------------------------------------------- -AX_BOOST_BASE([1.46],, [AC_MSG_ERROR([Boost not found])]) +AX_BOOST_BASE([1.54],, [AC_MSG_ERROR([Boost not found])]) AC_ARG_ENABLE(icu, AS_HELP_STRING([--enable-icu], diff --git a/doc/user.xml b/doc/user.xml index 1707a8cbc..fd8cd2022 100644 --- a/doc/user.xml +++ b/doc/user.xml @@ -111,7 +111,7 @@ cd mpd-version - Boost 1.46 + Boost 1.54 From c2c2c296589f9b6e679e807cdcafe454da1b066a Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 14 Mar 2018 13:15:03 +0100 Subject: [PATCH 3/8] input/thread: set InputStream::ready after Open() failure Without setting the "ready" flag, the caller will wait in WaitReady() forever, locking up MPD. Closes #252 --- NEWS | 2 ++ src/input/ThreadInputStream.cxx | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index 670cb3256..1a56cee5b 100644 --- a/NEWS +++ b/NEWS @@ -1,6 +1,8 @@ ver 0.20.19 (not yet released) * protocol - validate absolute seek time, reject negative values +* input + - mms: fix lockup bug * macOS: fix crash bug ver 0.20.18 (2018/02/24) diff --git a/src/input/ThreadInputStream.cxx b/src/input/ThreadInputStream.cxx index 88251a70a..a5e5e17d5 100644 --- a/src/input/ThreadInputStream.cxx +++ b/src/input/ThreadInputStream.cxx @@ -68,7 +68,7 @@ ThreadInputStream::ThreadFunc() Open(); } catch (...) { postponed_exception = std::current_exception(); - cond.broadcast(); + SetReady(); return; } From 672bdd3a56d8076aafe7fc3dd14df7652cc71732 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 15 Mar 2018 11:22:17 +0100 Subject: [PATCH 4/8] doc/user.xml: clarify where mpd.conf is read from on Android Closes #247 --- doc/user.xml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/doc/user.xml b/doc/user.xml index fd8cd2022..961590c4c 100644 --- a/doc/user.xml +++ b/doc/user.xml @@ -79,7 +79,10 @@ If you need to tweak the configuration, you can create a file - called mpd.conf on the data partition. + called mpd.conf on the data partition + (the directory which is returned by Android's getExternalStorageDirectory() + API function). From e8099f01b5a2d49dd238287461ff754947490064 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 15 Mar 2018 11:24:50 +0100 Subject: [PATCH 5/8] python/build/libs: upgrade CURL to 7.59.0 --- python/build/libs.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/python/build/libs.py b/python/build/libs.py index 18b4cbbaf..1c99a37cb 100644 --- a/python/build/libs.py +++ b/python/build/libs.py @@ -334,8 +334,8 @@ ffmpeg = FfmpegProject( ) curl = AutotoolsProject( - 'http://curl.haxx.se/download/curl-7.58.0.tar.xz', - '6a813875243609eb75f37fa72044e4ad618b55ec15a4eafdac2df6a7e800e3e3', + 'http://curl.haxx.se/download/curl-7.59.0.tar.xz', + 'e44eaabdf916407585bf5c7939ff1161e6242b6b015d3f2f5b758b2a330461fc', 'lib/libcurl.a', [ '--disable-shared', '--enable-static', From 73013a3c044fef94a6cdc9a422f85b8c86f27c1a Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 15 Mar 2018 19:23:31 +0100 Subject: [PATCH 6/8] input/thread: move code to Stop() Fixes crash due to "pure virtual method called" in the "mms" input plugin. Closes #253 --- NEWS | 2 +- src/input/ThreadInputStream.cxx | 7 ++++++- src/input/ThreadInputStream.hxx | 20 +++++++++++++++++++- src/input/plugins/MmsInputPlugin.cxx | 4 ++++ 4 files changed, 30 insertions(+), 3 deletions(-) diff --git a/NEWS b/NEWS index 1a56cee5b..f422b2239 100644 --- a/NEWS +++ b/NEWS @@ -2,7 +2,7 @@ ver 0.20.19 (not yet released) * protocol - validate absolute seek time, reject negative values * input - - mms: fix lockup bug + - mms: fix lockup bug and a crash bug * macOS: fix crash bug ver 0.20.18 (2018/02/24) diff --git a/src/input/ThreadInputStream.cxx b/src/input/ThreadInputStream.cxx index a5e5e17d5..49a0b0a66 100644 --- a/src/input/ThreadInputStream.cxx +++ b/src/input/ThreadInputStream.cxx @@ -26,8 +26,12 @@ #include #include -ThreadInputStream::~ThreadInputStream() +void +ThreadInputStream::Stop() noexcept { + if (!thread.IsDefined()) + return; + { const std::lock_guard lock(mutex); close = true; @@ -42,6 +46,7 @@ ThreadInputStream::~ThreadInputStream() buffer->Clear(); HugeFree(buffer->Write().data, buffer_size); delete buffer; + buffer = nullptr; } } diff --git a/src/input/ThreadInputStream.hxx b/src/input/ThreadInputStream.hxx index 046e0821c..cbf378926 100644 --- a/src/input/ThreadInputStream.hxx +++ b/src/input/ThreadInputStream.hxx @@ -27,6 +27,7 @@ #include +#include #include template class CircularBuffer; @@ -39,6 +40,11 @@ template class CircularBuffer; * manages the thread and the buffer. * * This works only for "streams": unknown length, no seeking, no tags. + * + * The implementation must call Stop() before its destruction + * completes. This cannot be done in ~ThreadInputStream() because at + * this point, the class has been morphed back to #ThreadInputStream + * and the still-running thread will crash due to pure method call. */ class ThreadInputStream : public InputStream { const char *const plugin; @@ -76,7 +82,13 @@ public: thread(BIND_THIS_METHOD(ThreadFunc)), buffer_size(_buffer_size) {} - virtual ~ThreadInputStream(); +#ifndef NDEBUG + ~ThreadInputStream() override { + /* Stop() must have been called already */ + assert(!thread.IsDefined()); + assert(buffer == nullptr); + } +#endif /** * Initialize the object and start the thread. @@ -90,6 +102,12 @@ public: size_t Read(void *ptr, size_t size) override final; protected: + /** + * Stop the thread and free the buffer. This must be called + * before destruction of this object completes. + */ + void Stop() noexcept; + void SetMimeType(const char *_mime) { assert(thread.IsInside()); diff --git a/src/input/plugins/MmsInputPlugin.cxx b/src/input/plugins/MmsInputPlugin.cxx index c06748adf..5f420af05 100644 --- a/src/input/plugins/MmsInputPlugin.cxx +++ b/src/input/plugins/MmsInputPlugin.cxx @@ -39,6 +39,10 @@ public: MMS_BUFFER_SIZE) { } + ~MmsInputStream() noexcept override { + Stop(); + } + protected: virtual void Open() override; virtual size_t ThreadRead(void *ptr, size_t size) override; From 37b07a5e7ca254fac74aa1f53c9b91f194e37d84 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 15 Mar 2018 20:00:14 +0100 Subject: [PATCH 7/8] pcm/PcmDop: use size_t --- src/pcm/PcmDop.cxx | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/pcm/PcmDop.cxx b/src/pcm/PcmDop.cxx index 8fad889c2..e54276140 100644 --- a/src/pcm/PcmDop.cxx +++ b/src/pcm/PcmDop.cxx @@ -46,19 +46,19 @@ pcm_dsd_to_dop(PcmBuffer &buffer, unsigned channels, assert(audio_valid_channel_count(channels)); assert(_src.size % channels == 0); - const unsigned num_src_samples = _src.size; - const unsigned num_src_frames = num_src_samples / channels; + const size_t num_src_samples = _src.size; + const size_t num_src_frames = num_src_samples / channels; /* this rounds down and discards the last odd frame; not elegant, but good enough for now */ - const unsigned num_frames = num_src_frames / 2; - const unsigned num_samples = num_frames * channels; + const size_t num_frames = num_src_frames / 2; + const size_t num_samples = num_frames * channels; uint32_t *const dest0 = (uint32_t *)buffer.GetT(num_samples), *dest = dest0; auto src = _src.data; - for (unsigned i = num_frames / 2; i > 0; --i) { + for (size_t i = num_frames / 2; i > 0; --i) { for (unsigned c = channels; c > 0; --c) { /* each 24 bit sample has 16 DSD sample bits plus the magic 0x05 marker */ From a2340c313f49d45abf3ade4645264e45c54918c7 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 15 Mar 2018 20:02:00 +0100 Subject: [PATCH 8/8] pcm/PcmDop: round down to the nearest multiple of 4 DSD bytes There was a discrepancy between what was written to the buffer and the size returned by pcm_dsd_to_dop(): the "for" loop uses num_frames/2, rounding down, while the return value is num_samples which is num_frames*channels, without rounding. This could cause undefined data at the end of the destination buffer if the source buffer size was not aligned to multiples of 8 bytes (4 DSD bytes per channel). The latter however can occur in the 0.21 branch after commit a06bf388d96 Closes #233 --- src/pcm/PcmDop.cxx | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/pcm/PcmDop.cxx b/src/pcm/PcmDop.cxx index e54276140..d7512d482 100644 --- a/src/pcm/PcmDop.cxx +++ b/src/pcm/PcmDop.cxx @@ -49,16 +49,17 @@ pcm_dsd_to_dop(PcmBuffer &buffer, unsigned channels, const size_t num_src_samples = _src.size; const size_t num_src_frames = num_src_samples / channels; - /* this rounds down and discards the last odd frame; not + /* this rounds down and discards up to 3 odd frames; not elegant, but good enough for now */ - const size_t num_frames = num_src_frames / 2; + const size_t num_dop_quads = num_src_frames / 4; + const size_t num_frames = num_dop_quads * 2; const size_t num_samples = num_frames * channels; uint32_t *const dest0 = (uint32_t *)buffer.GetT(num_samples), *dest = dest0; auto src = _src.data; - for (size_t i = num_frames / 2; i > 0; --i) { + for (size_t i = num_dop_quads; i > 0; --i) { for (unsigned c = channels; c > 0; --c) { /* each 24 bit sample has 16 DSD sample bits plus the magic 0x05 marker */