.. 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.
The Linux feature allows writing new files to an invisible file, and
then replace the old file. This preserves the old file if we get
interrupted by some event.
Fixes a problem with the "curl" input plugin: IsEOF() always returns
true because the "open" flag was cleared by
CurlInputStream::RequestDone() when end-of-stream was reached. This
flag stays false even when seeking to another position has succeeded.
This patch resets the "open" flag to true after seeking successfully.
NetBSD's pthread_setname_np() prototype is incompatible with the rest
of the world, and it requires to pass the string argument as a
non-const pointer. Instead of working around this misdesign, I hereby
disable the feature on NetBSD.
Here's a change to dynamically allocate the DSD ID3 tag buffer.
Pretty much anything with cover art is going to exceed the existing,
static 4k limit... Here's a change to dynamically allocate the buffer
and sanity check it at some upper limit. I rather arbitrarily pulled
256k out of thin air just to keep a corrupt file from causing it to
trying to allocate a buffer larger than available memory.
When mounting had not yet finished, SocketMonitor::IsDefined() was
always false, due to the workaround at the beginning of the function
that calls SocketMonitor::Steal(). This commit drops the IsDefined()
check because it was never necessary and breaks reconnect.
nfs_destroy_context() will invoke all pending callbacks with
err==-EINTR. In CancellableCallback::Callback(), this will invoke
NfsConnection::DeferClose(), which however is only designed to be
called from nfs_service(). In non-debug mode, this will leak memory
because nfs_close_async() is never called.
Workaround: before nfs_destroy_context(), invoke nfs_close_async() on
all pending file handles.
The method NfsConnection::CancellableCallback::Callback() will always
invoke NfsConnection::Close() on the file handle, even if the void
pointer is not a nfsfh. This can happen if the Open() was not
successful, e.g. when the file does not exist.
MPD used both "album artist" and "albumartist" tags and mapped them to one tag.
This could lead to issues, if a file had both tags, causing MPD to send
a list of albumartists instead of a single one.
Since "album artist" is not a standard tag anyway and even its originators
started to use the proper alternative, its time to say goodbye!
Skipping those songs silently will confuse the client, because
commands specifying the song index within a playlist
(e.g. playlistdelete) will be out of sync.
This copies spl_print()'s behavior to playlist_file_print().
Version 2.5 fixed an API oddity, however it broke API compatibility,
at least with C++. Disable the workaround when a libavformat version
is detected that is recent enough.
The "::" to explicitly refer to the global namespace appeared like a
good idea in C++, but it breaks with C libraries that implement
standard functions using macros (e.g. musl).
This was used by proprietary software. MPD adopted it a few years
ago, which turns out to be a mistake, because it now creates problems
for some MPD users (http://bugs.musicpd.org/view.php?id=4168).
libmp4v2 is licensed under MPL 1.1, which is incompatible with GPLv2.
Unfortunately, this means that we must remove the plugin.
More information can be found in the Debian bug report:
http://bugs.debian.org/767504
Commit d42c0f1dc5 added an OS X-specific
method of calling mpd_main_after_fork(), which uses Grand Central
Dispatch. Since this uses a block literal, it breaks compilation on
compilers which don't support the block extension, e.g. non-Apple
compilers. This affects users on older OS X releases with GCD (which
depend on older Clang releases, or Apple GCCs, which don't support the
C++11 features MPD needs); or which don't support GCD at all (10.5 and
lower).
This patch changes the #ifdef so that the non-GCD code is used
as it was on OS X before this patch if blocks aren't available, via
checking __BLOCKS__ macro.
The old formula calculates the output buffer size with "regular"
rounding (to the nearest integer), however sometimes, that is
insufficient and the last sample cannot be resampled. This causes
audible distortions. By changing the formula to consider the worst
case (always round up), this problem is eliminated.
While seeking, metadata must not be updated. ResponseBoundary() was
added in MPD 0.19.1, but I forgot to add the IsSeeking() check there.
This caused the "seekable" flag to reset.
On "list albumartist", songs that have no AlbumArtist tag will use the
Artist tag. However, if AlbumArtist is disabled via
"metadata_to_use", the TagBuilder::AddItem() call is ignored, and
PrintUniqueTag() attempts to print a nullptr string.
This commit fixes the problem by attempting the fallback only if
AlbumArtist is not disabled.
When uri_apply_base() was moved from db/upnp/Util.cpp to
util/UriUtil.cpp, the parameter order was changed, however without
swapping the parameters in the ContentDirectoryService constructor.
Many years ago, FAAD had a serious ABI bug: the NeAACDecInit()
prototype in its header declared the "samplerate" parameter to be
"unsigned long *", but internally, the function assumed it was
"uint32_t *" instead. On 32 bit machines, that was no difference, but
on 64 bit, this left one portion of the return value uninitialized;
and worse, on big-endian, the wrong word was filled. This bug had to
be worked around in MPD (commit 9c4e97a6).
A few months later, the bug was fixed in the FAAD CVS in commit 1.117
on file libfaad/decoder.c; the commit message was:
"Use public headers internally to prevent duplicate declarations"
The commit message was too brief at best; the problem was not
duplicate declarations, but a prototype mismatch. No mention of the
bug fix in the ChangeLog.
The MPD project never learned about this bug fix, and so MPD would
always pass a "uin32_t *" dressed up as a "unsigned long *". Nearly 6
years later, it's about time to fix this second ABI problem. Let's
kill the workaround!
Many years ago, FAAD had a serious ABI bug: the NeAACDecInit()
prototype in its header declared the "samplerate" parameter to be
"unsigned long *", but internally, the function assumed it was
"uint32_t *" instead. On 32 bit machines, that was no difference, but
on 64 bit, this left one portion of the return value uninitialized;
and worse, on big-endian, the wrong word was filled. This bug had to
be worked around in MPD (commit 9c4e97a6).
A few months later, the bug was fixed in the FAAD CVS in commit 1.117
on file libfaad/decoder.c; the commit message was:
"Use public headers internally to prevent duplicate declarations"
The commit message was too brief at best; the problem was not
duplicate declarations, but a prototype mismatch. No mention of the
bug fix in the ChangeLog.
The MPD project never learned about this bug fix, and so MPD would
always pass a "uin32_t *" dressed up as a "unsigned long *". Nearly 6
years later, it's about time to fix this second ABI problem. Let's
kill the workaround!
Pulseaudio expects clients to specify their channel-map if the
default (ALSA) map does not route the audio to the expected speakers.
Many Google results suggest dealing with this by re-routing the audio
channels with the appropriate ALSA plugin, but this will then simply
break any clients which expect the default ALSA mapping.
Virtually all media files and codecs, certainly flac, dca, a52, and of
course anything based on Microsoft's WAVEFORMAT_EXTENSIBLE specification,
assume the layout in the table here:
http://en.wikipedia.org/wiki/Surround_sound#Standard_speaker_channels
Fortunately, pulseaudio directly addresses this with a built-in channel
map for WAVE-EX which can be set automatically in the stream sample-spec.
MPD handles all strings in UTF-8 internally. Those decoders which
read Latin-1 tags are supposed to implement the conversion, instead of
passing Latin-1 to TagBuilder::AddItem(). FixTagString() is simply
the wrong place to do that, and hard-coding Latin-1 is kind of
arbitrary.
The Release Track Id uniquely identifies a recording on a release - that
is, even if a recording appears twice on a release (meaning that the
combination of recording and release id are not enough to figure out
which one it is), the release track id will allow differentiating the two.
The tag names are taken from
https://musicbrainz.org/doc/MusicBrainz_Picard/Tags/Mapping
On NetBSD, PTHREAD_MUTEX_INITIALIZER and PTHREAD_COND_INITIALIZER are
not compatible with C++11 "constexpr" (see Mantis ticket 0004110). As
a workaround, don't ues "constexpr", and use the functions
pthread_mutex_init(), pthread_mutex_destroy(), pthread_cond_init() and
pthread_cond_destroy() instead. This adds some runtime overhead, but
is portable to POSIX implementations that have awkward initializer
macros.
Casting std::numeric_limits<unsigned>::max() to "long" leads to an
overflow if sizeof(unsigned)==sizeof(long), and the result will be -1.
This happens on some 32 bit architectures, for example ARM and WIN32.
Workaround: use std::numeric_limits<int>::max(), which is the largest
signed integer. Since sizeof(long)>=sizeof(int), this will never
overflow.
Fixes Mantis ticket 0004080.
The IsActive() method returned true even if the timer was not active,
after it completed once. This broke the state file timer, and the
state file was not saved periodically.
Read one block at a time. This discards the last partial block, which
cannot be interleaved anyway. Previously, uninitialised memory was
used to interleave the last block, which generated some noise.
When the data chunk size is not a multiple of the frame size, the last
partial frame lead to an endless loop. We fix this by checking
chunk_sze>=frame instead of chunk_sze>0. This way, the partial frame
is simply skipped.
Previously, MPD tried to slurp the whole song file, count the number
of frames and calculate the song duration from that. That however is
extremely expensive for remote files, and will delay playback for a
long time. Workaround: check only the first 128 frames and try to
extrapolate from here. Fixes Mantis ticket 0004035.
