From 3f221e2edb3bebe3806c4a7f5490ba92ed99c101 Mon Sep 17 00:00:00 2001 From: Gustavo Zacarias Date: Tue, 8 Jul 2014 10:46:15 -0300 Subject: [PATCH 01/10] decoder/AudiofileDecoderPlugin: fix build failure due to missing stdio.h include Signed-off-by: Gustavo Zacarias --- src/decoder/AudiofileDecoderPlugin.cxx | 1 + 1 file changed, 1 insertion(+) diff --git a/src/decoder/AudiofileDecoderPlugin.cxx b/src/decoder/AudiofileDecoderPlugin.cxx index ab3557e52..92223f214 100644 --- a/src/decoder/AudiofileDecoderPlugin.cxx +++ b/src/decoder/AudiofileDecoderPlugin.cxx @@ -31,6 +31,7 @@ #include #include +#include /* pick 1020 since its devisible for 8,16,24, and 32-bit audio */ #define CHUNK_SIZE 1020 From a70443af319d5aa562c44a685ef753d117643fc0 Mon Sep 17 00:00:00 2001 From: Gustavo Zacarias Date: Tue, 8 Jul 2014 10:46:16 -0300 Subject: [PATCH 02/10] decoder/OpusDecoderPlugin: fix build failure due to missing stdio.h include Signed-off-by: Gustavo Zacarias --- src/decoder/OpusDecoderPlugin.cxx | 1 + 1 file changed, 1 insertion(+) diff --git a/src/decoder/OpusDecoderPlugin.cxx b/src/decoder/OpusDecoderPlugin.cxx index f3d7b342e..01ea3687a 100644 --- a/src/decoder/OpusDecoderPlugin.cxx +++ b/src/decoder/OpusDecoderPlugin.cxx @@ -40,6 +40,7 @@ #include #include +#include static constexpr opus_int32 opus_sample_rate = 48000; From d8e8eabf606ee170e2f2b46b3db4bd90a2573e7b Mon Sep 17 00:00:00 2001 From: Gustavo Zacarias Date: Tue, 8 Jul 2014 10:46:17 -0300 Subject: [PATCH 03/10] output/HttpdClient: fix build failure due to missing stdio.h include Signed-off-by: Gustavo Zacarias --- src/output/HttpdClient.cxx | 1 + 1 file changed, 1 insertion(+) diff --git a/src/output/HttpdClient.cxx b/src/output/HttpdClient.cxx index 8e13fda38..dc337053d 100644 --- a/src/output/HttpdClient.cxx +++ b/src/output/HttpdClient.cxx @@ -30,6 +30,7 @@ #include #include +#include HttpdClient::~HttpdClient() { From d4bd947bf550a208007b777eb1425f7fd75ce9b0 Mon Sep 17 00:00:00 2001 From: Gustavo Zacarias Date: Tue, 8 Jul 2014 10:46:18 -0300 Subject: [PATCH 04/10] playlist/PlsPlaylistPlugin: fix build failure due to missing stdio.h include Signed-off-by: Gustavo Zacarias --- src/playlist/PlsPlaylistPlugin.cxx | 1 + 1 file changed, 1 insertion(+) diff --git a/src/playlist/PlsPlaylistPlugin.cxx b/src/playlist/PlsPlaylistPlugin.cxx index 99be3ad35..7b5c8824c 100644 --- a/src/playlist/PlsPlaylistPlugin.cxx +++ b/src/playlist/PlsPlaylistPlugin.cxx @@ -31,6 +31,7 @@ #include #include +#include static constexpr Domain pls_domain("pls"); From bc6472bb9ea12fb2226570eb2dc49058b4d8bd6c Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 9 Jul 2014 18:46:58 +0200 Subject: [PATCH 05/10] decoder/audiofile: use decoder_read() .. instead of InputStream::LockRead(). The former is cancellable. --- NEWS | 2 ++ src/decoder/AudiofileDecoderPlugin.cxx | 39 ++++++++++++++++---------- 2 files changed, 26 insertions(+), 15 deletions(-) diff --git a/NEWS b/NEWS index add093393..19e3b2219 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,8 @@ ver 0.18.12 (not yet released) * database - proxy: fix build failure with libmpdclient 2.2 - proxy: fix add/search and other commands with libmpdclient < 2.9 +* decoder + - audiofile: improve responsiveness ver 0.18.11 (2014/05/12) * decoder diff --git a/src/decoder/AudiofileDecoderPlugin.cxx b/src/decoder/AudiofileDecoderPlugin.cxx index 92223f214..7c76deb7d 100644 --- a/src/decoder/AudiofileDecoderPlugin.cxx +++ b/src/decoder/AudiofileDecoderPlugin.cxx @@ -38,6 +38,15 @@ static constexpr Domain audiofile_domain("audiofile"); +struct AudioFileInputStream { + Decoder *const decoder; + InputStream &is; + + size_t Read(void *buffer, size_t size) { + return decoder_read(decoder, is, buffer, size); + } +}; + static int audiofile_get_duration(const char *file) { int total_time; @@ -55,29 +64,26 @@ static int audiofile_get_duration(const char *file) static ssize_t audiofile_file_read(AFvirtualfile *vfile, void *data, size_t length) { - InputStream &is = *(InputStream *)vfile->closure; + AudioFileInputStream &afis = *(AudioFileInputStream *)vfile->closure; - Error error; - size_t nbytes = is.LockRead(data, length, error); - if (nbytes == 0 && error.IsDefined()) { - LogError(error); - return -1; - } - - return nbytes; + return afis.Read(data, length); } static AFfileoffset audiofile_file_length(AFvirtualfile *vfile) { - InputStream &is = *(InputStream *)vfile->closure; + AudioFileInputStream &afis = *(AudioFileInputStream *)vfile->closure; + InputStream &is = afis.is; + return is.GetSize(); } static AFfileoffset audiofile_file_tell(AFvirtualfile *vfile) { - InputStream &is = *(InputStream *)vfile->closure; + AudioFileInputStream &afis = *(AudioFileInputStream *)vfile->closure; + InputStream &is = afis.is; + return is.GetOffset(); } @@ -92,7 +98,9 @@ audiofile_file_destroy(AFvirtualfile *vfile) static AFfileoffset audiofile_file_seek(AFvirtualfile *vfile, AFfileoffset offset, int is_relative) { - InputStream &is = *(InputStream *)vfile->closure; + AudioFileInputStream &afis = *(AudioFileInputStream *)vfile->closure; + InputStream &is = afis.is; + int whence = (is_relative ? SEEK_CUR : SEEK_SET); Error error; @@ -104,10 +112,10 @@ audiofile_file_seek(AFvirtualfile *vfile, AFfileoffset offset, int is_relative) } static AFvirtualfile * -setup_virtual_fops(InputStream &stream) +setup_virtual_fops(AudioFileInputStream &afis) { AFvirtualfile *vf = new AFvirtualfile(); - vf->closure = &stream; + vf->closure = &afis; vf->write = nullptr; vf->read = audiofile_file_read; vf->length = audiofile_file_length; @@ -174,7 +182,8 @@ audiofile_stream_decode(Decoder &decoder, InputStream &is) return; } - vf = setup_virtual_fops(is); + AudioFileInputStream afis{&decoder, is}; + vf = setup_virtual_fops(afis); af_fp = afOpenVirtualFile(vf, "r", nullptr); if (af_fp == AF_NULL_FILEHANDLE) { From dba41e2e4afd9cf342a6274cd8103dc45b5b883e Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sat, 14 Dec 2013 12:23:31 +0100 Subject: [PATCH 06/10] test: merge duplicate code to FakeDecoderAPI.cxx --- Makefile.am | 2 + test/FakeDecoderAPI.cxx | 110 ++++++++++++++++++++++++++++++++++++++++ test/dump_playlist.cxx | 83 ------------------------------ test/read_tags.cxx | 75 +-------------------------- 4 files changed, 113 insertions(+), 157 deletions(-) create mode 100644 test/FakeDecoderAPI.cxx diff --git a/Makefile.am b/Makefile.am index c7cf631bb..3eb8fac0a 100644 --- a/Makefile.am +++ b/Makefile.am @@ -1218,6 +1218,7 @@ test_dump_playlist_LDADD = \ libpcm.a \ $(GLIB_LIBS) test_dump_playlist_SOURCES = test/dump_playlist.cxx \ + test/FakeDecoderAPI.cxx \ $(DECODER_SRC) \ src/Log.cxx \ src/IOThread.cxx \ @@ -1271,6 +1272,7 @@ test_read_tags_LDADD = \ libutil.a \ $(GLIB_LIBS) test_read_tags_SOURCES = test/read_tags.cxx \ + test/FakeDecoderAPI.cxx \ src/Log.cxx \ src/IOThread.cxx \ src/ReplayGainInfo.cxx \ diff --git a/test/FakeDecoderAPI.cxx b/test/FakeDecoderAPI.cxx new file mode 100644 index 000000000..e2040b40d --- /dev/null +++ b/test/FakeDecoderAPI.cxx @@ -0,0 +1,110 @@ +/* + * Copyright (C) 2003-2012 The Music Player Daemon Project + * http://www.musicpd.org + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#include "config.h" +#include "DecoderAPI.hxx" +#include "InputStream.hxx" +#include "util/Error.hxx" +#include "Compiler.h" + +#include + +#include + +void +decoder_initialized(gcc_unused Decoder &decoder, + gcc_unused const AudioFormat audio_format, + gcc_unused bool seekable, + gcc_unused float total_time) +{ +} + +DecoderCommand +decoder_get_command(gcc_unused Decoder &decoder) +{ + return DecoderCommand::NONE; +} + +void +decoder_command_finished(gcc_unused Decoder &decoder) +{ +} + +double +decoder_seek_where(gcc_unused Decoder &decoder) +{ + return 1.0; +} + +void +decoder_seek_error(gcc_unused Decoder &decoder) +{ +} + +size_t +decoder_read(gcc_unused Decoder *decoder, + InputStream &is, + void *buffer, size_t length) +{ + return is.LockRead(buffer, length, IgnoreError()); +} + +void +decoder_timestamp(gcc_unused Decoder &decoder, + gcc_unused double t) +{ +} + +DecoderCommand +decoder_data(gcc_unused Decoder &decoder, + gcc_unused InputStream *is, + const void *data, size_t datalen, + gcc_unused uint16_t kbit_rate) +{ + gcc_unused ssize_t nbytes = write(1, data, datalen); + return DecoderCommand::NONE; +} + +DecoderCommand +decoder_tag(gcc_unused Decoder &decoder, + gcc_unused InputStream *is, + gcc_unused Tag &&tag) +{ + return DecoderCommand::NONE; +} + +void +decoder_replay_gain(gcc_unused Decoder &decoder, + const ReplayGainInfo *rgi) +{ + const ReplayGainTuple *tuple = &rgi->tuples[REPLAY_GAIN_ALBUM]; + if (tuple->IsDefined()) + g_printerr("replay_gain[album]: gain=%f peak=%f\n", + tuple->gain, tuple->peak); + + tuple = &rgi->tuples[REPLAY_GAIN_TRACK]; + if (tuple->IsDefined()) + g_printerr("replay_gain[track]: gain=%f peak=%f\n", + tuple->gain, tuple->peak); +} + +void +decoder_mixramp(gcc_unused Decoder &decoder, gcc_unused MixRampInfo &&mix_ramp) +{ +} diff --git a/test/dump_playlist.cxx b/test/dump_playlist.cxx index d11562930..4ac02985a 100644 --- a/test/dump_playlist.cxx +++ b/test/dump_playlist.cxx @@ -24,7 +24,6 @@ #include "Directory.hxx" #include "InputStream.hxx" #include "ConfigGlobal.hxx" -#include "DecoderAPI.hxx" #include "DecoderList.hxx" #include "InputInit.hxx" #include "IOThread.hxx" @@ -53,88 +52,6 @@ my_log_func(const gchar *log_domain, gcc_unused GLogLevelFlags log_level, g_printerr("%s\n", message); } -void -decoder_initialized(gcc_unused Decoder &decoder, - gcc_unused const AudioFormat audio_format, - gcc_unused bool seekable, - gcc_unused float total_time) -{ -} - -DecoderCommand -decoder_get_command(gcc_unused Decoder &decoder) -{ - return DecoderCommand::NONE; -} - -void -decoder_command_finished(gcc_unused Decoder &decoder) -{ -} - -double -decoder_seek_where(gcc_unused Decoder &decoder) -{ - return 1.0; -} - -void -decoder_seek_error(gcc_unused Decoder &decoder) -{ -} - -size_t -decoder_read(gcc_unused Decoder *decoder, - InputStream &is, - void *buffer, size_t length) -{ - return is.LockRead(buffer, length, IgnoreError()); -} - -void -decoder_timestamp(gcc_unused Decoder &decoder, - gcc_unused double t) -{ -} - -DecoderCommand -decoder_data(gcc_unused Decoder &decoder, - gcc_unused InputStream *is, - const void *data, size_t datalen, - gcc_unused uint16_t kbit_rate) -{ - gcc_unused ssize_t nbytes = write(1, data, datalen); - return DecoderCommand::NONE; -} - -DecoderCommand -decoder_tag(gcc_unused Decoder &decoder, - gcc_unused InputStream *is, - gcc_unused Tag &&tag) -{ - return DecoderCommand::NONE; -} - -void -decoder_replay_gain(gcc_unused Decoder &decoder, - const ReplayGainInfo *rgi) -{ - const ReplayGainTuple *tuple = &rgi->tuples[REPLAY_GAIN_ALBUM]; - if (tuple->IsDefined()) - g_printerr("replay_gain[album]: gain=%f peak=%f\n", - tuple->gain, tuple->peak); - - tuple = &rgi->tuples[REPLAY_GAIN_TRACK]; - if (tuple->IsDefined()) - g_printerr("replay_gain[track]: gain=%f peak=%f\n", - tuple->gain, tuple->peak); -} - -void -decoder_mixramp(gcc_unused Decoder &decoder, gcc_unused MixRampInfo &&mix_ramp) -{ -} - int main(int argc, char **argv) { const char *uri; diff --git a/test/read_tags.cxx b/test/read_tags.cxx index 90f1424d9..52b2561b8 100644 --- a/test/read_tags.cxx +++ b/test/read_tags.cxx @@ -20,7 +20,7 @@ #include "config.h" #include "IOThread.hxx" #include "DecoderList.hxx" -#include "DecoderAPI.hxx" +#include "DecoderPlugin.hxx" #include "InputInit.hxx" #include "InputStream.hxx" #include "AudioFormat.hxx" @@ -42,79 +42,6 @@ #include #endif -void -decoder_initialized(gcc_unused Decoder &decoder, - gcc_unused const AudioFormat audio_format, - gcc_unused bool seekable, - gcc_unused float total_time) -{ -} - -DecoderCommand -decoder_get_command(gcc_unused Decoder &decoder) -{ - return DecoderCommand::NONE; -} - -void -decoder_command_finished(gcc_unused Decoder &decoder) -{ -} - -double -decoder_seek_where(gcc_unused Decoder &decoder) -{ - return 1.0; -} - -void -decoder_seek_error(gcc_unused Decoder &decoder) -{ -} - -size_t -decoder_read(gcc_unused Decoder *decoder, - InputStream &is, - void *buffer, size_t length) -{ - return is.LockRead(buffer, length, IgnoreError()); -} - -void -decoder_timestamp(gcc_unused Decoder &decoder, - gcc_unused double t) -{ -} - -DecoderCommand -decoder_data(gcc_unused Decoder &decoder, - gcc_unused InputStream *is, - const void *data, size_t datalen, - gcc_unused uint16_t kbit_rate) -{ - gcc_unused ssize_t nbytes = write(1, data, datalen); - return DecoderCommand::NONE; -} - -DecoderCommand -decoder_tag(gcc_unused Decoder &decoder, - gcc_unused InputStream *is, - gcc_unused Tag &&tag) -{ - return DecoderCommand::NONE; -} - -void -decoder_replay_gain(gcc_unused Decoder &decoder, - gcc_unused const ReplayGainInfo *replay_gain_info) -{ -} - -void -decoder_mixramp(gcc_unused Decoder &decoder, gcc_unused MixRampInfo &&mix_ramp) -{ -} - static bool empty = true; static void From bf7417981f2527348fb261f40f4de15f9b350db5 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sat, 14 Dec 2013 12:21:23 +0100 Subject: [PATCH 07/10] DecoderAPI: add function decoder_skip() Move code from the "mad" plugin. --- src/DecoderAPI.cxx | 16 ++++++++++++++++ src/DecoderAPI.hxx | 8 ++++++++ src/decoder/MadDecoderPlugin.cxx | 15 +-------------- test/FakeDecoderAPI.cxx | 16 ++++++++++++++++ test/run_decoder.cxx | 16 ++++++++++++++++ 5 files changed, 57 insertions(+), 14 deletions(-) diff --git a/src/DecoderAPI.cxx b/src/DecoderAPI.cxx index 4fea02bef..52b30cfb5 100644 --- a/src/DecoderAPI.cxx +++ b/src/DecoderAPI.cxx @@ -292,6 +292,22 @@ decoder_read(Decoder *decoder, return nbytes; } +bool +decoder_skip(Decoder *decoder, InputStream &is, size_t size) +{ + while (size > 0) { + char buffer[1024]; + size_t nbytes = decoder_read(decoder, is, buffer, + std::min(sizeof(buffer), size)); + if (nbytes == 0) + return false; + + size -= nbytes; + } + + return true; +} + void decoder_timestamp(Decoder &decoder, double t) { diff --git a/src/DecoderAPI.hxx b/src/DecoderAPI.hxx index 2ee42483c..50cd3e74a 100644 --- a/src/DecoderAPI.hxx +++ b/src/DecoderAPI.hxx @@ -112,6 +112,14 @@ decoder_read(Decoder &decoder, InputStream &is, return decoder_read(&decoder, is, buffer, length); } +/** + * Skip data on the #InputStream. + * + * @return true on success, false on error or command + */ +bool +decoder_skip(Decoder *decoder, InputStream &is, size_t size); + /** * Sets the time stamp for the next data chunk [seconds]. The MPD * core automatically counts it up, and a decoder plugin only needs to diff --git a/src/decoder/MadDecoderPlugin.cxx b/src/decoder/MadDecoderPlugin.cxx index 9dd86c55f..7afc8ef8f 100644 --- a/src/decoder/MadDecoderPlugin.cxx +++ b/src/decoder/MadDecoderPlugin.cxx @@ -413,20 +413,7 @@ MadDecoder::ParseId3(size_t tagsize, Tag **mpd_tag) mad_stream_skip(&stream, tagsize); } else { mad_stream_skip(&stream, count); - - while (count < tagsize) { - size_t len = tagsize - count; - char ignored[1024]; - if (len > sizeof(ignored)) - len = sizeof(ignored); - - len = decoder_read(decoder, input_stream, - ignored, len); - if (len == 0) - break; - else - count += len; - } + decoder_skip(decoder, input_stream, tagsize - count); } #endif } diff --git a/test/FakeDecoderAPI.cxx b/test/FakeDecoderAPI.cxx index e2040b40d..469e37feb 100644 --- a/test/FakeDecoderAPI.cxx +++ b/test/FakeDecoderAPI.cxx @@ -65,6 +65,22 @@ decoder_read(gcc_unused Decoder *decoder, return is.LockRead(buffer, length, IgnoreError()); } +bool +decoder_skip(Decoder *decoder, InputStream &is, size_t size) +{ + while (size > 0) { + char buffer[1024]; + size_t nbytes = decoder_read(decoder, is, buffer, + std::min(sizeof(buffer), size)); + if (nbytes == 0) + return false; + + size -= nbytes; + } + + return true; +} + void decoder_timestamp(gcc_unused Decoder &decoder, gcc_unused double t) diff --git a/test/run_decoder.cxx b/test/run_decoder.cxx index 2f7330a1d..cab68c64e 100644 --- a/test/run_decoder.cxx +++ b/test/run_decoder.cxx @@ -101,6 +101,22 @@ decoder_read(gcc_unused Decoder *decoder, return is.LockRead(buffer, length, IgnoreError()); } +bool +decoder_skip(Decoder *decoder, InputStream &is, size_t size) +{ + while (size > 0) { + char buffer[1024]; + size_t nbytes = decoder_read(decoder, is, buffer, + std::min(sizeof(buffer), size)); + if (nbytes == 0) + return false; + + size -= nbytes; + } + + return true; +} + void decoder_timestamp(gcc_unused Decoder &decoder, gcc_unused double t) From 0759421d119d8d3d829978ede81683ca54d65d69 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sat, 14 Dec 2013 12:43:06 +0100 Subject: [PATCH 08/10] DecoderAPI: add function decoder_read_full() Move code from the "mad" plugin. --- src/DecoderAPI.cxx | 18 ++++++++++++++++++ src/DecoderAPI.hxx | 11 +++++++++++ src/decoder/MadDecoderPlugin.cxx | 14 ++------------ test/FakeDecoderAPI.cxx | 18 ++++++++++++++++++ test/run_decoder.cxx | 18 ++++++++++++++++++ 5 files changed, 67 insertions(+), 12 deletions(-) diff --git a/src/DecoderAPI.cxx b/src/DecoderAPI.cxx index 52b30cfb5..e4122d60e 100644 --- a/src/DecoderAPI.cxx +++ b/src/DecoderAPI.cxx @@ -292,6 +292,24 @@ decoder_read(Decoder *decoder, return nbytes; } +bool +decoder_read_full(Decoder *decoder, InputStream &is, + void *_buffer, size_t size) +{ + uint8_t *buffer = (uint8_t *)_buffer; + + while (size > 0) { + size_t nbytes = decoder_read(decoder, is, buffer, size); + if (nbytes == 0) + return false; + + buffer += nbytes; + size -= nbytes; + } + + return true; +} + bool decoder_skip(Decoder *decoder, InputStream &is, size_t size) { diff --git a/src/DecoderAPI.hxx b/src/DecoderAPI.hxx index 50cd3e74a..a9da76305 100644 --- a/src/DecoderAPI.hxx +++ b/src/DecoderAPI.hxx @@ -112,6 +112,17 @@ decoder_read(Decoder &decoder, InputStream &is, return decoder_read(&decoder, is, buffer, length); } +/** + * Blocking read from the input stream. Attempts to fill the buffer + * completely; there is no partial result. + * + * @return true on success, false on error or command or not enough + * data + */ +bool +decoder_read_full(Decoder *decoder, InputStream &is, + void *buffer, size_t size); + /** * Skip data on the #InputStream. * diff --git a/src/decoder/MadDecoderPlugin.cxx b/src/decoder/MadDecoderPlugin.cxx index 7afc8ef8f..6f619b34b 100644 --- a/src/decoder/MadDecoderPlugin.cxx +++ b/src/decoder/MadDecoderPlugin.cxx @@ -353,18 +353,8 @@ MadDecoder::ParseId3(size_t tagsize, Tag **mpd_tag) memcpy(allocated, stream.this_frame, count); mad_stream_skip(&(stream), count); - while (count < tagsize) { - size_t len; - - len = decoder_read(decoder, input_stream, - allocated + count, tagsize - count); - if (len == 0) - break; - else - count += len; - } - - if (count != tagsize) { + if (!decoder_read_full(decoder, input_stream, + allocated + count, tagsize - count)) { LogDebug(mad_domain, "error parsing ID3 tag"); g_free(allocated); return; diff --git a/test/FakeDecoderAPI.cxx b/test/FakeDecoderAPI.cxx index 469e37feb..ca09ca982 100644 --- a/test/FakeDecoderAPI.cxx +++ b/test/FakeDecoderAPI.cxx @@ -65,6 +65,24 @@ decoder_read(gcc_unused Decoder *decoder, return is.LockRead(buffer, length, IgnoreError()); } +bool +decoder_read_full(Decoder *decoder, InputStream &is, + void *_buffer, size_t size) +{ + uint8_t *buffer = (uint8_t *)_buffer; + + while (size > 0) { + size_t nbytes = decoder_read(decoder, is, buffer, size); + if (nbytes == 0) + return false; + + buffer += nbytes; + size -= nbytes; + } + + return true; +} + bool decoder_skip(Decoder *decoder, InputStream &is, size_t size) { diff --git a/test/run_decoder.cxx b/test/run_decoder.cxx index cab68c64e..7db8dde22 100644 --- a/test/run_decoder.cxx +++ b/test/run_decoder.cxx @@ -101,6 +101,24 @@ decoder_read(gcc_unused Decoder *decoder, return is.LockRead(buffer, length, IgnoreError()); } +bool +decoder_read_full(Decoder *decoder, InputStream &is, + void *_buffer, size_t size) +{ + uint8_t *buffer = (uint8_t *)_buffer; + + while (size > 0) { + size_t nbytes = decoder_read(decoder, is, buffer, size); + if (nbytes == 0) + return false; + + buffer += nbytes; + size -= nbytes; + } + + return true; +} + bool decoder_skip(Decoder *decoder, InputStream &is, size_t size) { From 20538516b92082067ce3477d612fb404ba8671ad Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 9 Jul 2014 19:05:20 +0200 Subject: [PATCH 09/10] decoder/audiofile: use decoder_read_full() Works around WAV stream playback bug, because libaudiofile does not like partial reads (Mantis 0004028). --- NEWS | 1 + src/decoder/AudiofileDecoderPlugin.cxx | 7 ++++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index 19e3b2219..d6fd59519 100644 --- a/NEWS +++ b/NEWS @@ -4,6 +4,7 @@ ver 0.18.12 (not yet released) - proxy: fix add/search and other commands with libmpdclient < 2.9 * decoder - audiofile: improve responsiveness + - audiofile: fix WAV stream playback ver 0.18.11 (2014/05/12) * decoder diff --git a/src/decoder/AudiofileDecoderPlugin.cxx b/src/decoder/AudiofileDecoderPlugin.cxx index 7c76deb7d..3fb23bc20 100644 --- a/src/decoder/AudiofileDecoderPlugin.cxx +++ b/src/decoder/AudiofileDecoderPlugin.cxx @@ -43,7 +43,12 @@ struct AudioFileInputStream { InputStream &is; size_t Read(void *buffer, size_t size) { - return decoder_read(decoder, is, buffer, size); + /* libaudiofile does not like partial reads at all, + and wil abort playback; therefore always force full + reads */ + return decoder_read_full(decoder, is, buffer, size) + ? size + : 0; } }; From 09384df32cc6bef40bc3630de1c928d2eb424909 Mon Sep 17 00:00:00 2001 From: Joff Date: Wed, 9 Jul 2014 19:18:36 +0200 Subject: [PATCH 10/10] decoder/dsd: use decoder_read_full() where appropriate Addresses Mantis ticket 0004015. [mk: use decoder_read_full() only when needed, and a few formal changes] --- NEWS | 1 + src/decoder/DsdLib.cxx | 10 +--------- src/decoder/DsdLib.hxx | 4 ---- src/decoder/DsdiffDecoderPlugin.cxx | 25 ++++++++++++------------- src/decoder/DsfDecoderPlugin.cxx | 11 ++++++----- 5 files changed, 20 insertions(+), 31 deletions(-) diff --git a/NEWS b/NEWS index d6fd59519..b67eb9314 100644 --- a/NEWS +++ b/NEWS @@ -5,6 +5,7 @@ ver 0.18.12 (not yet released) * decoder - audiofile: improve responsiveness - audiofile: fix WAV stream playback + - dsdiff, dsf: fix stream playback ver 0.18.11 (2014/05/12) * decoder diff --git a/src/decoder/DsdLib.cxx b/src/decoder/DsdLib.cxx index 67cc7e945..eafedda8f 100644 --- a/src/decoder/DsdLib.cxx +++ b/src/decoder/DsdLib.cxx @@ -49,14 +49,6 @@ DsdId::Equals(const char *s) const return memcmp(value, s, sizeof(value)) == 0; } -bool -dsdlib_read(Decoder *decoder, InputStream &is, - void *data, size_t length) -{ - size_t nbytes = decoder_read(decoder, is, data, length); - return nbytes == length; -} - /** * Skip the #input_stream to the specified offset. */ @@ -149,7 +141,7 @@ dsdlib_tag_id3(InputStream &is, id3_byte_t *dsdid3data; dsdid3data = dsdid3; - if (!dsdlib_read(nullptr, is, dsdid3data, count)) + if (!decoder_read_full(nullptr, is, dsdid3data, count)) return; id3_tag = id3_tag_parse(dsdid3data, count); diff --git a/src/decoder/DsdLib.hxx b/src/decoder/DsdLib.hxx index 53160cf1e..5c6127149 100644 --- a/src/decoder/DsdLib.hxx +++ b/src/decoder/DsdLib.hxx @@ -58,10 +58,6 @@ public: } }; -bool -dsdlib_read(Decoder *decoder, InputStream &is, - void *data, size_t length); - bool dsdlib_skip_to(Decoder *decoder, InputStream &is, int64_t offset); diff --git a/src/decoder/DsdiffDecoderPlugin.cxx b/src/decoder/DsdiffDecoderPlugin.cxx index a3c0149b9..f8506851a 100644 --- a/src/decoder/DsdiffDecoderPlugin.cxx +++ b/src/decoder/DsdiffDecoderPlugin.cxx @@ -93,14 +93,14 @@ static bool dsdiff_read_id(Decoder *decoder, InputStream &is, DsdId *id) { - return dsdlib_read(decoder, is, id, sizeof(*id)); + return decoder_read_full(decoder, is, id, sizeof(*id)); } static bool dsdiff_read_chunk_header(Decoder *decoder, InputStream &is, DsdiffChunkHeader *header) { - return dsdlib_read(decoder, is, header, sizeof(*header)); + return decoder_read_full(decoder, is, header, sizeof(*header)); } static bool @@ -112,8 +112,7 @@ dsdiff_read_payload(Decoder *decoder, InputStream &is, if (size != (uint64_t)length) return false; - size_t nbytes = decoder_read(decoder, is, data, length); - return nbytes == length; + return decoder_read_full(decoder, is, data, length); } /** @@ -145,8 +144,8 @@ dsdiff_read_prop_snd(Decoder *decoder, InputStream &is, } else if (header.id.Equals("CHNL")) { uint16_t channels; if (header.GetSize() < sizeof(channels) || - !dsdlib_read(decoder, is, - &channels, sizeof(channels)) || + !decoder_read_full(decoder, is, + &channels, sizeof(channels)) || !dsdlib_skip_to(decoder, is, chunk_end_offset)) return false; @@ -154,8 +153,8 @@ dsdiff_read_prop_snd(Decoder *decoder, InputStream &is, } else if (header.id.Equals("CMPR")) { DsdId type; if (header.GetSize() < sizeof(type) || - !dsdlib_read(decoder, is, - &type, sizeof(type)) || + !decoder_read_full(decoder, is, + &type, sizeof(type)) || !dsdlib_skip_to(decoder, is, chunk_end_offset)) return false; @@ -208,7 +207,7 @@ dsdiff_handle_native_tag(InputStream &is, struct dsdiff_native_tag metatag; - if (!dsdlib_read(nullptr, is, &metatag, sizeof(metatag))) + if (!decoder_read_full(nullptr, is, &metatag, sizeof(metatag))) return; uint32_t length = FromBE32(metatag.size); @@ -221,7 +220,7 @@ dsdiff_handle_native_tag(InputStream &is, char *label; label = string; - if (!dsdlib_read(nullptr, is, label, (size_t)length)) + if (!decoder_read_full(nullptr, is, label, (size_t)length)) return; string[length] = '\0'; @@ -328,7 +327,7 @@ dsdiff_read_metadata(Decoder *decoder, InputStream &is, DsdiffChunkHeader *chunk_header) { DsdiffHeader header; - if (!dsdlib_read(decoder, is, &header, sizeof(header)) || + if (!decoder_read_full(decoder, is, &header, sizeof(header)) || !header.id.Equals("FRM8") || !header.format.Equals("DSD ")) return false; @@ -391,10 +390,10 @@ dsdiff_decode_chunk(Decoder &decoder, InputStream &is, now_size = now_frames * frame_size; } - size_t nbytes = decoder_read(decoder, is, buffer, now_size); - if (nbytes != now_size) + if (!decoder_read_full(&decoder, is, buffer, now_size)) return false; + const size_t nbytes = now_size; chunk_size -= nbytes; if (lsbitfirst) diff --git a/src/decoder/DsfDecoderPlugin.cxx b/src/decoder/DsfDecoderPlugin.cxx index 5ef94e647..ad5483c32 100644 --- a/src/decoder/DsfDecoderPlugin.cxx +++ b/src/decoder/DsfDecoderPlugin.cxx @@ -103,7 +103,7 @@ dsf_read_metadata(Decoder *decoder, InputStream &is, DsfMetaData *metadata) { DsfHeader dsf_header; - if (!dsdlib_read(decoder, is, &dsf_header, sizeof(dsf_header)) || + if (!decoder_read_full(decoder, is, &dsf_header, sizeof(dsf_header)) || !dsf_header.id.Equals("DSD ")) return false; @@ -117,7 +117,8 @@ dsf_read_metadata(Decoder *decoder, InputStream &is, /* read the 'fmt ' chunk of the DSF file */ DsfFmtChunk dsf_fmt_chunk; - if (!dsdlib_read(decoder, is, &dsf_fmt_chunk, sizeof(dsf_fmt_chunk)) || + if (!decoder_read_full(decoder, is, + &dsf_fmt_chunk, sizeof(dsf_fmt_chunk)) || !dsf_fmt_chunk.id.Equals("fmt ")) return false; @@ -143,7 +144,7 @@ dsf_read_metadata(Decoder *decoder, InputStream &is, /* read the 'data' chunk of the DSF file */ DsfDataChunk data_chunk; - if (!dsdlib_read(decoder, is, &data_chunk, sizeof(data_chunk)) || + if (!decoder_read_full(decoder, is, &data_chunk, sizeof(data_chunk)) || !data_chunk.id.Equals("data")) return false; @@ -247,10 +248,10 @@ dsf_decode_chunk(Decoder &decoder, InputStream &is, now_size = now_frames * frame_size; } - size_t nbytes = decoder_read(&decoder, is, buffer, now_size); - if (nbytes != now_size) + if (!decoder_read_full(&decoder, is, buffer, now_size)) return false; + const size_t nbytes = now_size; chunk_size -= nbytes; if (bitreverse)