On 2012/02/03 15:12, Jurgen Kramer <gtmkra...@xs4all.nl> wrote: > Hi, > > I hereby sent a series of patches to add my new DSD decoder I talked > about last December. The decoder supports outputting > DSD data over PCM according to the proposed > 'DSD-over-USB' standard by dCS. It reads raw DSD data > from DFF (Philips) and DSF (Sony) formatted DSD files > and sends the DSD data packed over PCM to a capable > audio device like a DAC. > > Further features: > - Packs DSD data into 24 or 32-bit PCM format > - Seek support > - Supports ID3 tags and tags native to DFF format > > Tested on 32-bit and 64-bit x86 and 32-bit ARM systems using a > USB-to-spdif converter or firewire (snd-dice firewire sound driver). > > Patches are against current git.
You made a few odd design choices which I don't agree with. Your code duplicates code from the existing decoder plugin. Not just that, it also adds new features to the "forked" version, which are not present in the other plugin. This is not acceptable for me. The plugin claims to write 24 or 32 bit samples, but in fact it does not. This will break many things, for example cross-fading, software volume, normalization, resampling, and breaks completely when you have multiple outputs one of which doesn't support this funny encoding that pretends to be PCM. A "clean" implementation would pass something like raw DSD data to the MPD core, and add code for conversion to the MPD's PCM library. That way, each output could declare if they want "real" PCM, or if they allow this encoded DSD data, or something else. Lines are too long (140 columns and more), and there are many stray whitespaces at line endings. Documentation in doc/user.xml is missing. Your way of splitting patches is useless. Your last patch is the one that adds the new source files to the Makefile.am, which means none of the patches prior to that are useful - they don't add anything, except for source files that are not being used! For example, your first patch adds a source file that #includes "dsdnative/dsdnative.h", but that file does not exist yet - it will be added in the following patch. The macro CONF_DSDIFF_NATIVE isn't used. What is it supposed to be? The end result fails to compile: src/decoder/dsdiff_nativeplayback_plugin.c:182:2: error: 'input' undeclared (first use in this function) Max ------------------------------------------------------------------------------ Try before you buy = See our experts in action! The most comprehensive online learning library for Microsoft developers is just $99.99! Visual Studio, SharePoint, SQL - plus HTML5, CSS3, MVC3, Metro Style Apps, more. Free future releases when you subscribe now! http://p.sf.net/sfu/learndevnow-dev2 _______________________________________________ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
