This commit is similar to 788e3b31e1,
and removes more "pure" attributes which were placed on functions that
could throw exceptions, which is illegal according to clang's
understanding of the attribute (but not according to GCC's). GitHub
issue #58 was most likely about StorageDirectoryReader::GetInfo() and
Storage::GetInfo(), which still had "pure" attributes.
Closes#58
Fixes build failure on OS X, closes#44. With the other plugins,
that's not critical, because those use the AudioOutputWrapper, which
hides this problem.
The "pure" and "const" attributes are not so well-defined, and a
recent clang version implements an optimization which pushes the
definition's boundary beyond what I believed it was. clang now
assumes that functions declared "pure" cannot throw exceptions, even
if they lack the "noexcept" specification.
When compiled with this new clang version, MPD will crash randomly if
an exception happens to get thrown by such as "pure" function
(https://github.com/MusicPlayerDaemon/MPD/issues/41).
This commit removes all such misplaced "pure" and "const" attributes,
closing #41.
An ino_t is usually a 64 bit integer, and some file systems (such as
Linux's kernel NFS client) really uses the upper 32 bit. This can
lead to false positives in the directory loop detection in
FindAncestorLoop(). Increasing these two attributes (in
StorageFileInfo and Directory) to 64 bit adds little overhead, but
makes the check a lot safer.
The TAG_MODIFIED handler (i.e. playlist::TagModified()) works only if
the modified song is the current song - something that is not updated
until SYNC_WITH_PLAYER is finished. This fixes tag updates right
after a new song is started.
https://bugs.musicpd.org/view.php?id=4656 describes a crash due to
division by zero because frame.samples==0. This should never happen,
but apparently can happen after seeking. The best we can do is to
just ignore this frame.
Fixes another buffer overflow: if the stream has a very long title or
URL, resulting in a metadata string of more than 2 kB, icy_string[0]
is a negative value, which gets casted to size_t - ouch!
https://bugs.musicpd.org/view.php?id=4652
Use SND_PCM_NONBLOCK, and perform all snd_pcm_writei() calls in the
IOThread. Use a lockless queue to copy data from the OutputThread to
the IOThread.
This rather major change aims to improve MPD's internal latency. All
waits are now under MPD's control, instead of blocking inside
libasound2.
As a side effect, an output's filter is now decoupled from the actual
device I/O, which solves a major latency problem with the conversion
filter on slow CPUs and small period buffers. See:
https://bugs.musicpd.org/view.php?id=3900
When rpc_reconnect_requeue() gets called from inside nfs_service(),
the NfsInputStream can stall completely because the old socket has
been unregistered from epoll automatically, but the new one has never
been registered. Therefore, nfs_service() will never be called again.
This kludge attempts to detect this condition by checking
nfs_which_events()==POLLOUT.
https://bugs.musicpd.org/view.php?id=4081
If the base class is not accessible, the "catching" the base class
won't work. This caused the fatal error:
terminate called after throwing an instance of 'LibmpdclientError'
Each close/open cycle resets the Filter's state, because a new Filter
instance is being created. That results in the serials
(replay_gain_serial and other_replay_gain_serial) being out of sync
with the internal ReplayGainFilter state.
So instead of initializing those serials once, we need to initialize
them each time we create new ReplayGainFilter instances, i.e. in
OpenFilter().
https://bugs.musicpd.org/view.php?id=4632
Previously, there was no special code to convert stereo to
multi-channel. The generic solution for this was to convert to mono,
and then copy the result to all channels. That's a pretty bad
solution, but at least something which always renders audio. MPD does
something, instead of failing.
Now that MPD has proper support for multi-channel (by defining the
channel order), we can do better than that. It is a (somewhat) common
case to play back stereo music on a DAC which can only do
multi-channel. The best approach here is to copy the stereo channels
to front-left and front-right, and apply the "silence" pattern to all
other channels.
If the input AudioFormat changes but the out_audio_format doesn't
change (e.g. because there is a fixed "format" setting in this
"audio_output" section), the ConvertFilter needs to be reconfigured.
This didn't happen, resulting in awful static noise after changing
songs.
This method is used by DecoderControl::IsCurrentSong(), which is used
by the player thread to check whether the current decoder instance can
be reused to seek. When switching to another song in the same CUE
sheet, previously DetachedSong::IsSame() returned true, and thus the
old decoder instance was used for the new song, not considering the
new end_time. This led to the old decoder quickly quitting.
This way, we have four periods instead of the default of two. With
only two periods, we don't get woken up often enough, and we
frequently encounter buffer overruns. With four periods, we have more
time to breathe, and the buffer overruns magically disappear.
The byte order of DSD_U32 was wrong from the start. The oldest bits
must be in the MSB, not in the LSB, according to
snd_pcm_format_descriptions in alsa-lib.
DSD_U32 packs four bytes instead of one large "sample", thus the
sample rate is one quarter of the input sample rate. This fixes a
rather critical DSD_U32 playback problem.
Changed AlsaMixerPlugin to use the get and set normalized functions from volume_mapping of alsa-utils/alsamixer
Changed volume_mapping set volume to be for all channels and not per channel
added volume_mapping files to Makefile.am
Without this, the pipe would run empty very often, which may result in
an xrun if the roundtrip to the PlayerThread and back takes too long.
By waking up the PlayerThread before the pipe runs empty, we make MPD
much more latency tolerant, which is a major optimization.
The user unit omits the "ProtectKernelModules" setting which fails
with modular kernels:
Failed at step CAPABILITIES spawning /usr/bin/mpd: Operation not permitted
It is unfortunate that systemd (version 232) is unable to reduce its
own capabilities, because this requires us to split system and user
units.
https://bugs.musicpd.org/view.php?id=4608
This commit changes a minor queue priority design to something which
makes a little bit more sense.
Previously, a song that had already been played would only be
re-enqueued if its priority had just been raised above the current
song's. This means that if it was already above, it was not
re-enqueued. That is a surprising behavior, because users expect a
song to be played when its priority is raised.
Now the song is always re-enqueued if its priority is raised (and
above the current song's - no matter if it has already been above
before).
https://bugs.musicpd.org/view.php?id=4592
The ScopeExit library uses C++11 initializers, which gcc 4.6 does not
support. Let's kill support for this ancient incomplete C++11
compiler, nobody should be using it anymore.
The "seeking" flag is not set for the initial seek, and so
decoder_read() could be canceled when another SEEK was emitted during
initial seek.
This fixes several seek problems, for example the one reported for the
FLAC decoder plugin:
https://bugs.musicpd.org/view.php?id=4552
.. instead of doing it after seeking. After seeking, the command had
no effect, because CheckDecoderStartup() waits for all outputs to
finish. This caused a very long delay while seeking and switching
songs (https://bugs.musicpd.org/view.php?id=4534).
Source: mpd
Version: 0.19.14-2
Severity: important
Justification: fails to build form source (but built in the past)
Tags: patch
User: debian-alpha@lists.debian.org
Usertags: alpha
mpd FTBFS on Alpha with a failure in the test suite [1]:
FAIL: test/test_byte_reverse
============================
.F...
!!!FAILURES!!!
Test Results:
Run: 4 Failures: 1 Errors: 0
1) test: ByteReverseTest::TestByteReverse2 (F) line: 58 test/test_byte_reverse.cxx
assertion failed
- Expression: strcmp(result, (const char *)dest) == 0
This occurs because the test suite (in test/test_byte_reversal.cxx)
allocates static char arrays and passes the char arrays to functions
whose respective arguments were declared to be uint16_t *, etc., in
the main code.
This is in the realm of undefined behaviour on architectures with
strict memory alignment requirements. Although the test only fails
on Alpha (because Alpha has a particular CPU load instruction that
gcc likes to use to add bugs ..., ahem, optimise the code on the
assumption of alignment) it is potentially a latent bug for other
architectures with strict alignment requirements.
Since the code is compiled with the c++11 standard I attach a patch
that modifies the test suite to align the non-compliant strings with
the alignas() attribute. The test suite now passes on Alpha with
that patch.
Cheers
Michael
[1] https://buildd.debian.org/status/fetch.php?pkg=mpd&arch=alpha&ver=0.19.14-2&stamp=1461542099
> In file included from src/decoder/DecoderBuffer.cxx:21:0:
> src/decoder/DecoderBuffer.hxx:41:20: error: 'uint8_t' was not declared in this scope
> DynamicFifoBuffer<uint8_t> buffer;
> ^
> src/decoder/DecoderBuffer.hxx:41:27: error: template argument 1 is invalid
> DynamicFifoBuffer<uint8_t> buffer;
> ^
> src/decoder/DecoderBuffer.hxx: In member function 'void DecoderBuffer::Clear()':
> src/decoder/DecoderBuffer.hxx:61:10: error: request for member 'Clear' in '((DecoderBuffer*)this)->DecoderBuffer::buffer', which is of non-class type 'int'
> buffer.Clear();
> ^
> src/decoder/DecoderBuffer.hxx: In member function 'size_t DecoderBuffer::GetAvailable() const':
> src/decoder/DecoderBuffer.hxx:78:17: error: request for member 'GetAvailable' in '((const DecoderBuffer*)this)->DecoderBuffer::buffer', which is of non-class type 'const int'
> return buffer.GetAvailable();
> ^
> src/decoder/DecoderBuffer.hxx: In member function 'ConstBuffer<void> DecoderBuffer::Read() const':
> src/decoder/DecoderBuffer.hxx:87:19: error: request for member 'Read' in '((const DecoderBuffer*)this)->DecoderBuffer::buffer', which is of non-class type 'const int'
> auto r = buffer.Read();
> ^
> src/decoder/DecoderBuffer.hxx:88:27: error: could not convert '{<expression error>, <expression error>}' from '<brace-enclosed initializer list>' to 'ConstBuffer<void>'
> return { r.data, r.size };
> ^
> src/decoder/DecoderBuffer.hxx: In member function 'void DecoderBuffer::Consume(size_t)':
> src/decoder/DecoderBuffer.hxx:105:10: error: request for member 'Consume' in '((DecoderBuffer*)this)->DecoderBuffer::buffer', which is of non-class type 'int'
> buffer.Consume(nbytes);
> ^
This seems to be caused by a lacking include, fixed by the below patch.
I'm unsure what made this appear now, though, compiler and toolchain
libraries seem to be the same upstream versions that built 0.19.14-1
just fine in late March.
When a reference counter is at its limit, don't allocate a new
TagPoolSlot - that would result in many TagPoolSlot instances with
ref==1. This in turn would make the linked list very very large,
which means quadratic runtime for many operations.
Apparently all other C libraries are not compatible with "constexpr".
Those which are not will get a performance penalty, but at least they
work at all.
MPD does not really take advantage of memory-mapped I/O by generating
data right into the ALSA buffer; using plain snd_pcm_mmap_writei() has
no advantage compared to snd_pcm_writei(). Let's kill this
non-feature.
The initgroups() manpage says we need to check for _BSD_SOURCE. The
thing is that glibc deprecated this macro, and doesn't define it
anymore, effectively breaking all MPD supplementary groups.
The real fix is to check for initgroups() availability at configure
time, instead of relying on the deprecated _BSD_SOURCE macro.
Apply padding only to the fseek(), not to the chunk size. This fixes
bogus "failed to read riff chunk" messages when the last chunk has an
odd size.
See http://bugs.musicpd.org/view.php?id=4486
systemd does not understand LimitRTTIME=-1. For no limit we have to use
the string 'infinity' (see systemd.exec(5)).
Signed-off-by: Christian Hesse <mail@eworm.de>
This reverts commit d7d9dbd2c2 by
reimplementing it with the current MPD API.
3 years ago, I was wrong about the "embcue" plugin being able to
replace this one, because "embcue" reads a tag named "CUESHEET", while
this plugin reads the "CUESHEET" FLAC metablock. There's an important
difference between those two!
Allocate the buffer dynamically using av_malloc(), and free
AVIOContext.buffer in the destructor, as mandated by the libavformat
documentation.
Fixes http://bugs.musicpd.org/view.php?id=4446
Wildcard matches are directly applied to all filenames in
subdirectories without any attempt at matching relative paths.
This change is based on the following feature request:
http://bugs.musicpd.org/view.php?id=3729
Use the first INDEX in each TRACK section, instead of the last, for the
start time. This preserves the original CD layout (including gaps
between tracks), and avoids skipping sections of songs in more exotic
cuesheets (eg musical suite tracks).
Fixes 0004355 and 0003359
If the song tag comes from a stream, and MPD playback restarts, MPD
would believe the tag should override the newly received tag. This
makes the previous tag appear stuck. This change passes the song tag
only if it's authoritative - i.e. if it's a song file.
Right after booting, the monotonic clock starts with a very small
value, and AudioOutput::LockUpdate() may believe that the fail_timer
has not recovered yet.
The Connect method can be called between Schedule and lock. In that case, when
locked, the state is already set to CONNECTING of READY and the condition won't
be signaled anymore.
Not initialising granulepos leads to it having arbitrary values in the
encoded stream including possibly negative values which are not valid
and confuse opusdec. Explicitly initialise opus_encoder::granulepos
to avoid that problem.
Requiring this prefix makes the client's intention very clear, but it
was too hard to understand why this prefix was needed. Initially, my
intention was to differentiate from broken clients which prefix relate
URIs with a slash; once MPD allowed that. In the past few years
however, MPD has disallowed that, and there was no significant
breakage (except for the "add /" special case which some clients
apparently still do). So I figure it's about time to define that an
URI that begins with a slash points to an arbitrary file on the file
system.
The file handle is never reset to INVALID_HANDLE_VALUE, and thus the
destructor will assume the operation shall be cancelled and will
delete the temporary file.
This was a major breakage for saving the database file and the state
file.
Build a table of pre-existing tag types before adding new items. The
old way would check HasType() each time, which would return true after
the first instance of that tag type had been added, preventing
duplicate tag types to be merged.
This broke duplicate tag types loaded from the state file, because
this code path uses TagBuilder::Complement().
This is Darwin specific: the previous implementation was causing an integer
overflow when base.numer is very large. On PPC Darwin, the timebase info is 1000000000/33330116 and this is too large for integer arithmetic.
This is Darwin specific: the previous implementation was causing an integer
overflow when base.numer is very large. On PPC Darwin, the timebase info is 1000000000/33330116 and this is too large for integer arithmetic.
Max Kellermann