Max Kellermann wrote: > 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. 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 Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team