Max, this is the first attempt to get the code in for comments. I think
it's understandable that some things may not be what you expect or require.
I my fear has been that the longer the patch goes on without this kind of
input the harder it would be to submit and have it excepted. Let us look
into your comments....
Best
Jesus R
On Fri, Feb 3, 2012 at 9:49 AM, Max Kellermann <m...@duempel.org> wrote:
> 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
