Re: [Musicpd-dev-team] [PATCH] Add support for DSF files to DSDIFF decoder - v3
On Mon, Apr 23, 2012 at 07:16:49PM +0200, Jurgen Kramer wrote: Please show me how I can check myself. It is getting a bit tiresome to hear about wrong tabs/indents. Jurgen If you use Vim, this little snippet will highlight all trailing white space: au Syntax * \ syn match ExtraSpace \s\+$ containedin=ALL \ | hi def link ExtraSpace Error syn match ExtraSpace \s\+$ hi def link ExtraSpace Error Otherwise, a 'git grep \s$' should get you the respective lines. As for other white space errors, I don't know. Thanks, Jonathan Neuschäfer -- Live Security Virtual Conference Exclusive live event will cover all the ways today's security and threat landscape has changed and how IT managers can respond. Discussions will include endpoint security, mobile security and the latest in malware threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/ ___ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
Re: [Musicpd-dev-team] [PATCH] Add support for DSF files to DSDIFF decoder - v3
At 2012-04-25 00:03:00 +0200, Jonathan Neuschäfer wrote: On Mon, Apr 23, 2012 at 07:16:49PM +0200, Jurgen Kramer wrote: Please show me how I can check myself. It is getting a bit tiresome to hear about wrong tabs/indents. Jurgen If you use Vim, this little snippet will highlight all trailing white space: au Syntax * \ syn match ExtraSpace \s\+$ containedin=ALL \ | hi def link ExtraSpace Error syn match ExtraSpace \s\+$ hi def link ExtraSpace Error Alternatively, switch on list mode, since this is the sort of thing it's meant for: :se list Also see the docs for the listchars option if you want to customize what is displayed. :he 'listchars' I have this in my vimrc: set list if encoding =~ utf-8 set listchars=tab:›\ ,trail:·,extends:…,precedes:…,nbsp:‸ else set listchars=tab:\ ,trail:.,extends:,precedes:,nbsp:. endif This also makes it very obvious when there are mixtures of spaces and tabs and things like that. --bart -- Live Security Virtual Conference Exclusive live event will cover all the ways today's security and threat landscape has changed and how IT managers can respond. Discussions will include endpoint security, mobile security and the latest in malware threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/ ___ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
Re: [Musicpd-dev-team] [PATCH] Add support for DSF files to DSDIFF decoder - v3
On Mon, 2012-04-23 at 19:09 +0200, Max Kellermann wrote: On 2012/04/21 13:29, Jurgen Kramer gtmkra...@xs4all.nl wrote: - Adhered to max 80 colom width This is not doxygen syntax, and is barely readable. Please use doxygen syntax for documenting struct attributes. - Fixed indents (hopefully correctly) What did you fix? Indentation dsdiff_read_metadata() is still all wrong. I took a stab a changing the loop part. Please check, it probably needs some more work. I don't see what you changed. There are 17 whitespace errors in your patch according to git. Please show me how I can check myself. It is getting a bit tiresome to hear about wrong tabs/indents. Jurgen -- For Developers, A Lot Can Happen In A Second. Boundary is the first to Know...and Tell You. Monitor Your Applications in Ultra-Fine Resolution. Try it FREE! http://p.sf.net/sfu/Boundary-d2dvs2 ___ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
Re: [Musicpd-dev-team] [PATCH] Add support for DSF files to DSDIFF decoder - v3
On 2012/04/21 13:29, Jurgen Kramer gtmkra...@xs4all.nl wrote: - Adhered to max 80 colom width This is not doxygen syntax, and is barely readable. Please use doxygen syntax for documenting struct attributes. - Fixed indents (hopefully correctly) What did you fix? Indentation dsdiff_read_metadata() is still all wrong. I took a stab a changing the loop part. Please check, it probably needs some more work. I don't see what you changed. There are 17 whitespace errors in your patch according to git. -- For Developers, A Lot Can Happen In A Second. Boundary is the first to Know...and Tell You. Monitor Your Applications in Ultra-Fine Resolution. Try it FREE! http://p.sf.net/sfu/Boundary-d2dvs2 ___ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
Re: [Musicpd-dev-team] [PATCH] Add support for DSF files to DSDIFF decoder - v3
On 2012/04/21 19:21, Jonathan Neuschäfer j.neuschae...@gmx.net wrote:
On Sat, Apr 21, 2012 at 01:29:37PM +0200, Jurgen Kramer wrote:
+struct dsdiff_header_dsf {
+ struct dsdiff_id id;/* DSD */
+ uint32_t size_low, size_high; /* DSD chunk size, including id = 28 */
+ uint32_t fsize_low, fsize_high; /* Total file size */
+ uint32_t pmeta_low, pmeta_high; /* Pointer to id3v2 metadata, should be
+ at the end of the file */
+};
What's the reason for not using uint64_t?
Because it might be misaligned, due to sizeof(dsdiff_id)==4, and some
CPUs may require 64 bit reads and 64 bit alignment, or MPD gets killed
with SIGBUS.
But since this struct is not explicitly packed (something which is
only possible with compiler extensions), the compiler would choose to
add a hole of 4 bytes after the dsdiff_id.
+ /* Check bits per sample format, determine if bitreverse is needed */
+ metadata-bitreverse = dsf_fmt_chunk.bitssample == 1 ? true : false;
@Max: Would it make sense to directly assign the value, which the '=='
operator returned?
Correct. Less code and easier to read.
Max
--
For Developers, A Lot Can Happen In A Second.
Boundary is the first to Know...and Tell You.
Monitor Your Applications in Ultra-Fine Resolution. Try it FREE!
http://p.sf.net/sfu/Boundary-d2dvs2
___
Musicpd-dev-team mailing list
Musicpd-dev-team@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
[Musicpd-dev-team] [PATCH] Add support for DSF files to DSDIFF decoder - v3
Attached is version 3 of the patch which adds support for
DSF files to the DSDIFF decoder. It can be used in
conjunction with DSD2PCM and DSD-over-USB.
This version should address the concerns with v2.
Changes from v2 patch:
- Adhered to max 80 colom width
- Fixed indents (hopefully correctly)
- Removed usage of splitting 32-bit values in two 16-bit values
- bitreverse is now a bool
- Renamed make_dff_format function, added more comments explaining it
I took a stab a changing the loop part. Please check, it probably needs
some more work.
while (true) {
[...]
+ if (!metadata.fileisdff) {
The current code allows for more then one DSD chunk in a DFF file. From
reading the DSDIFF standard I cannot determine if this is even
supported. I think the loop could be removed without affecting
the ability to play DSDIFF files.
Jurgen
diff --git a/doc/user.xml b/doc/user.xml
index cd36528..46d9c11 100644
--- a/doc/user.xml
+++ b/doc/user.xml
@@ -782,7 +782,7 @@ systemctl start mpd.socket/programlisting
titlevarnamedsdiff/varname/title
para
- Decodes DFF files containing DSDIFF data (e.g. SACD rips).
+ Decodes DFF and DSF files containing DSDIFF data (e.g. SACD rips).
/para
informaltable
diff --git a/src/decoder/dsdiff_decoder_plugin.c b/src/decoder/dsdiff_decoder_plugin.c
index ae42002..4fb9b23 100644
--- a/src/decoder/dsdiff_decoder_plugin.c
+++ b/src/decoder/dsdiff_decoder_plugin.c
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2003-2011 The Music Player Daemon Project
+ * 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
@@ -19,9 +19,12 @@
/* \file
*
- * This plugin decodes DSDIFF data (SACD) embedded in DFF files. It
- * was modeled after the specification found here:
+ * This plugin decodes DSDIFF data (SACD) embedded in DFF and DSF files.
+ * The DFF code was modeled after the specification found here:
* http://www.sonicstudio.com/pdf/dsd/DSDIFF_1.5_Spec.pdf
+ *
+ * The DSF code was created using the specification found here:
+ * http://dsd-guide.com/sonys-dsf-file-format-spec
*/
#include config.h
@@ -53,10 +56,46 @@ struct dsdiff_chunk_header {
struct dsdiff_metadata {
unsigned sample_rate, channels;
+ bool fileisdff;
+ bool bitreverse;
+ uint64_t chunk_size;
};
static bool lsbitfirst;
+struct dsdiff_header_dsf {
+ struct dsdiff_id id; /* DSD */
+ uint32_t size_low, size_high; /* DSD chunk size, including id = 28 */
+ uint32_t fsize_low, fsize_high; /* Total file size */
+ uint32_t pmeta_low, pmeta_high; /* Pointer to id3v2 metadata, should be
+ at the end of the file */
+};
+
+struct dsdiff_dsf_fmt_chunk { /* -- DSF file fmt chunk -- */
+
+ struct dsdiff_id id; /* fmt */
+ uint32_t size_low, size_high; /* fmt chunk size, including id,
+ normally 52 */
+
+ uint32_t version; /* Version of this format = 1 */
+ uint32_t formatid; /* 0: DSD raw */
+ uint32_t channeltype; /* Channel Type, 1 = mono, 2 = stereo,
+ 3 = 3 channels, etc */
+ uint32_t channelnum; /* Channel number, 1 = mono, 2 = stereo,
+ ... 6 = 6 channels */
+ uint32_t sample_freq; /* Sample frequency: 2822400, 5644800 */
+ uint32_t bitssample; /* Bits per sample 1 or 8 */
+ uint32_t scnt_low, scnt_high; /* Sample count per channel in bytes */
+ uint32_t block_size; /* Block size per channel = 4096 */
+ uint32_t reserved; /* Reserved, should be all zero */
+};
+
+struct dsdiff_dsf_data_chunk {
+ struct dsdiff_id id;
+ uint32_t size_low, size_high; /* 'data' chunk size, includes header
+ (id+size) */
+};
+
static bool
dsdiff_init(const struct config_param *param)
{
@@ -224,7 +263,7 @@ dsdiff_read_prop_snd(struct decoder *decoder, struct input_stream *is,
return false;
if (!dsdiff_id_equals(type, DSD ))
-/* only uincompressed DSD audio data
+/* only uncompressed DSD audio data
is implemented */
return false;
} else {
@@ -274,28 +313,111 @@ dsdiff_read_metadata(struct decoder *decoder, struct input_stream *is,
struct dsdiff_header header;
if (!dsdiff_read(decoder, is, header, sizeof(header)) ||
!dsdiff_id_equals(header.id, FRM8) ||
- !dsdiff_id_equals(header.format, DSD ))
+ !dsdiff_id_equals(header.format, DSD )) {
+
+ /* It's not a DFF file, check if it is a DSF file */
+ if (!dsdiff_skip_to(decoder, is, 0)) /* Reset to beginning
+ of the stream */
+ return false;
+
+ uint64_t chunk_size;
+ struct dsdiff_header_dsf dsfheader;
+ if (!dsdiff_read(decoder, is, dsfheader, sizeof(dsfheader)) ||
+ !dsdiff_id_equals(header.id, DSD ))
return false;
- while (true) {
- if (!dsdiff_read_chunk_header(decoder, is, chunk_header))
+ chunk_size =
+ (((uint64_t)GUINT32_FROM_LE(dsfheader.size_high)) 32) |
+ ((uint64_t)GUINT32_FROM_LE(dsfheader.size_low));
+
+ if (sizeof(dsfheader) != chunk_size)
+ return false;
+
+ /* Read the 'fmt '
Re: [Musicpd-dev-team] [PATCH] Add support for DSF files to DSDIFF decoder - v3
On Sat, Apr 21, 2012 at 01:29:37PM +0200, Jurgen Kramer wrote:
+struct dsdiff_header_dsf {
+ struct dsdiff_id id;/* DSD */
+ uint32_t size_low, size_high; /* DSD chunk size, including id = 28 */
+ uint32_t fsize_low, fsize_high; /* Total file size */
+ uint32_t pmeta_low, pmeta_high; /* Pointer to id3v2 metadata, should be
+at the end of the file */
+};
What's the reason for not using uint64_t?
+ /* Check bits per sample format, determine if bitreverse is needed */
+ metadata-bitreverse = dsf_fmt_chunk.bitssample == 1 ? true : false;
@Max: Would it make sense to directly assign the value, which the '=='
operator returned?
+ metadata-fileisdff = false;
+ return true;
+
+ } else {
The indentation is a bit confusing here.
/**
+ * DSF data is build up of alternating 4096 blocks of DSD samples for left
and
+ * right data. Convert the buffer holding 1 block of 4096 DSD left samples
and
+ * 1 block of 4096 DSD right samples to 8k of samples in normal PCM
left/right
+ * order.
+ */
+static void
+dsf_to_pcm_order(uint8_t *p, uint8_t *q, size_t nrbytes)
You can see by reading the code that the content of 'p' is copied into
'q', but more descriptive names (like 'from' and 'to') would make it
more readable.
Thanks,
Jonathan Neuschäfer
--
For Developers, A Lot Can Happen In A Second.
Boundary is the first to Know...and Tell You.
Monitor Your Applications in Ultra-Fine Resolution. Try it FREE!
http://p.sf.net/sfu/Boundary-d2dvs2
___
Musicpd-dev-team mailing list
Musicpd-dev-team@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
