Max Kellermann wrote:
> On 2009/01/20 21:29, Linel Patrice <[email protected]> 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.
It is the merge commit, i thought i had to rewrite it.
>
> There are more patches, most of which I don't understand.
> e.g. 4fe20df is described as "envoi pr david" ???
>
This is an intermediate commit to do a push on another server.
> The patch "Begin implementation for pulse mixer" (d8efc9e0) looks
> pointless, because it only adds some commented code.
>
Yes, but it is an old one , you want me to remove it ?
> 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
What does not work, is when i launch mpd without --no-daemon it fails to
get the volume on the mixer. So I don't understand why.
Do you want me to rewrite all my commit before doing the rebase?
------------------------------------------------------------------------------
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
[email protected]
https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
