On 2009/01/20 21:29, Linel Patrice <patnathan...@gmail.com> wrote:
> I recoded the mixer , in a way it does not change the existing
> output pulse.
>
> git://git.musicpd.org/pat/mpd.git
>
> You should only need to pick the last commit.
I cannot pick the last commit, because it is a merge commit. Don't
merge, rebase on master instead ("git pull --rebase" or "stg rebase").
If you need any help with git usage, you're welcome to ask.
There are two patches with identical description, but with a different
subject (9218b2e3a and b354929). I don't understand the description
at all.
There are more patches, most of which I don't understand.
e.g. 4fe20df is described as "envoi pr david" ???
The patch "Begin implementation for pulse mixer" (d8efc9e0) looks
pointless, because it only adds some commented code.
Your code doesn't compile with --enable-werror (always enable this
option during development!):
cc1: warnings being treated as errors
mixer/pulse_mixer.c:22: error: unused parameter 'context'
mixer/pulse_mixer.c:44: error: unused parameter 'context'
mixer/pulse_mixer.c: In function 'subscribe_cb':
mixer/pulse_mixer.c:61: error: declaration of 'index' shadows a global
declaration
/usr/include/string.h:309: error: shadowed declaration is here
mixer/pulse_mixer.c: At top level:
mixer/pulse_mixer.c:61: error: unused parameter 'c'
mixer/pulse_mixer.c: In function 'context_state_cb':
mixer/pulse_mixer.c:92: error: enumeration value 'PA_CONTEXT_UNCONNECTED' not
handled in switch
mixer/pulse_mixer.c:92: error: enumeration value 'PA_CONTEXT_CONNECTING' not
handled in switch
mixer/pulse_mixer.c:92: error: enumeration value 'PA_CONTEXT_AUTHORIZING' not
handled in switch
mixer/pulse_mixer.c:92: error: enumeration value 'PA_CONTEXT_SETTING_NAME' not
handled in switch
mixer/pulse_mixer.c: In function 'pulse_mixer_configure':
mixer/pulse_mixer.c:160: error: unused variable 'ret'
mixer/pulse_mixer.c:159: error: unused variable 'o'
mixer/pulse_mixer.c: In function 'pulse_mixer_open':
mixer/pulse_mixer.c:194: error: unused variable 'o'
mixer/pulse_mixer.c:193: error: unused variable 'pm'
mixer/pulse_mixer.c: In function 'pulse_mixer_close':
mixer/pulse_mixer.c:208: error: unused variable 'pm'
make[2]: *** [mpd-pulse_mixer.o] Error 1
More formal stuff: try to keep lines less than 80 characters long.
Exceptions are allowed when every other solution is even less
readable. That's not the case in pulse_mixer.c, please break those
overlong lines.
Try to keep MPD's code style. Why a closure for each "case"
statement? pulse_mixer.c has lots of whitespace errors (spaces at the
end of the line).
Your code doesn't have proper error handling:
> if(!(pm->mainloop = pa_threaded_mainloop_new())) g_debug("Failed
> mainloop\n");
>
> if(!(pm->context =
> pa_context_new(pa_threaded_mainloop_get_api(pm->mainloop),"Mixer mpd")))
> g_debug("Failed context\n");
>
> pa_context_set_state_callback(pm->context,
> context_state_cb, pm);
So you check the result, but continue to work with NULL pointers.
This will crash!
> I need some feedback on that please.
> It still not work in daemon.
What do you mean, it does not work? Do not send a pull request if you
know it doesn't work - we can talk about your code, but I'll pull only
when every single patch you submit works and is self-contained, and
doesn't introduce a regression. It's important that every commit
works (at the best of our knowledge), because a broken commit would
make bisecting hard or impossible.
Please describe what exactly does not work.
Max
------------------------------------------------------------------------------
This SF.net email is sponsored by:
SourcForge Community
SourceForge wants to tell your story.
http://p.sf.net/sfu/sf-spreadtheword
_______________________________________________
Musicpd-dev-team mailing list
Musicpd-dev-team@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
