Rockbox

I guess I should add that this was only tested on my Sansa e200.

Hmm, tried to patch it but after going through all the-p0 -p1... none of them worked! I do have other patches but none of them that envole mmpeg and it didn't even get to the HUNK FAILED #1 of 2 or w/e it is. are you sure that it patches?

Yes, I just checked, but using the tortoise svn patch program...
I don't know if that's different from the patch program in cygwin. If so, I can create one in there, it just seemed easier at them time to use tortoise.

the patch is made in the mpegplayer directory, you'll need to place it there to apply it or point patch to apps/plugins/mpegplayer/mpegplayer.c

Ok, so it patches fine now when you put it in that specific directory, but next time one should make it so that you can leave it in the root of the source. But anywyas. I have a H10 5g and the volume works fine except that it is backwards. having volume capabilities is great but for some reason it is backwards. I will be watching this and will gladly test for you.

Thanks for this. I can't believe they didn't bother with it in the first place...

if(button == BUTTON_SCROLL_UP)
vol -= vol_interval;
else
vol += vol_interval;

Shouldnt it be like that:?
vol += vol_interval;
else
vol -= vol_interval;

Sacha and Hepdog: Meh...
On the Sansa E200 series it worked the other way, thus the comment of mine about it being backwards. When I get home on Sunday, I'll try to add compatibility for more devices.

nls: Yes, sorry, my first patch, I'll use the patch program in cygwin next time instead of tortoise svn.

How about this:
[code]/* button definitions */
#if (CONFIG_KEYPAD == IRIVER_H100_PAD) || (CONFIG_KEYPAD == IRIVER_H300_PAD)
#define MPEG_MENU BUTTON_MODE
#define MPEG_STOP BUTTON_OFF
#define MPEG_PAUSE BUTTON_ON
#define MPEG_VOL_DOWN BUTTON_DOWN
#define MPEG_VOL_UP BUTTON_UP

#elif (CONFIG_KEYPAD == IPOD_3G_PAD) || (CONFIG_KEYPAD == IPOD_4G_PAD)
#define MPEG_MENU BUTTON_MENU
#define MPEG_PAUSE (BUTTON_PLAY | BUTTON_REL)
#define MPEG_STOP (BUTTON_PLAY | BUTTON_REPEAT)
//#define MPEG_VOL_DOWN BUTTON_DOWN
//#define MPEG_VOL_UP BUTTON_UP

#elif CONFIG_KEYPAD == IAUDIO_X5_PAD
#define MPEG_MENU (BUTTON_REC | BUTTON_REL)
#define MPEG_STOP BUTTON_POWER
#define MPEG_PAUSE BUTTON_PLAY
#define MPEG_VOL_DOWN BUTTON_DOWN
#define MPEG_VOL_UP BUTTON_UP

#elif CONFIG_KEYPAD == GIGABEAT_PAD
#define MPEG_MENU BUTTON_MENU
#define MPEG_STOP BUTTON_A
#define MPEG_PAUSE BUTTON_SELECT
#define MPEG_VOL_DOWN BUTTON_DOWN
#define MPEG_VOL_UP BUTTON_UP

#elif CONFIG_KEYPAD == IRIVER_H10_PAD
#define MPEG_MENU (BUTTON_REW | BUTTON_REL)
#define MPEG_STOP BUTTON_POWER
#define MPEG_PAUSE BUTTON_PLAY
#define MPEG_VOL_DOWN BUTTON_SCROLL_DOWN
#define MPEG_VOL_UP BUTTON_SCROLL_UP

#elif CONFIG_KEYPAD == SANSA_E200_PAD
#define MPEG_MENU BUTTON_SELECT
#define MPEG_STOP BUTTON_POWER
#define MPEG_PAUSE BUTTON_UP
#define MPEG_VOL_DOWN BUTTON_SCROLL_UP
#define MPEG_VOL_UP BUTTON_SCROLL_DOWN

#else
#error MPEGPLAYER: Unsupported keypad
#endif[/code]
[...]
[code] switch (button)
{
#if defined(MPEG_VOL_UP) && defined(MPEG_VOL_DOWN)
case MPEG_VOL_UP:
case MPEG_VOL_DOWN:
vol = rb->global_settings->volume;
minvol = rb->sound_min(SOUND_VOLUME);
maxvol = rb->sound_max(SOUND_VOLUME);

if (button == MPEG_VOL_DOWN)
vol -= vol_interval;
else
vol += vol_interval;

if ( vol > maxvol )
vol = maxvol;
else if( vol < minvol )
vol = minvol;

if ( vol != rb->global_settings->volume ) {
rb->sound_set( SOUND_VOLUME, vol );
rb->global_settings->volume = vol;
}
break;
#endif
case MPEG_MENU:
[/code]

Just for the record the patch is out of sync!

Pacal: Thanks, I'll put it in on sunday, when I get home, or you can do it if you like

Hepdog: It's my first patch.

Oh ya. No problem. I actually reported 4 different patches that went out of sync all at the same time!

I only have tested it on my x5, but the h10 and the sansa part should work.
The keymapping for the other players should work, but for the IPOD_3G_PAD and IPOD_4G_PAD I have no idea.
If the keymapping is ready the "#if defined(MPEG_VOL_UP) && defined(MPEG_VOL_DOWN)" can be removed.

latest SVN:
IAUDIO_X5_PAD changed to IAUDIO_X5M5_PAD

If you want this committed to SVN, please post a proper patch. If you checked out the code from svn, you just need to do:

svn diff apps/plugins/mpegplayer/mpegplayer.c > mpegplayer-volcontrol.diff

and then attach that .diff file to a comment.

For more information, see: http://www.rockbox.org/twiki/bin/view/Main/WorkingWithPatches

I had to remove some other patches, but there it is.

Thanks for the patch. A few comments:

1) Does it not work for ipods? The appropriate key mappings should be BUTTON_SCROLL_FWD to increase the volume, and BUTTON_SCROLL_BACK to decrease it (see the WPS definitions in keymap-ipod.c)

2) The patch doesn't appear to handle repeat events - it should check for MPEG_VOL_UP|BUTTON_REPEAT as a well as simply MPEG_VOL_UP

3) Looking at keymap-gigabeat.c, there are two sets of buttons mapped to volume changing in the WPS screen (which is what I think mpegplayer should mimic) - the normal UP/DOWN buttons, plus the dedicated VOL_UP and VOL_DOWN buttons.

now iPod 3G & iPod 4G are included.
I only have an iAudio x5, so please test.

OK, thanks. I'll fix up all the other issues and commit.

now it also displays the Volume and botton_repeat for every target.