[vitunes] Re: vitunes

  • From: Ryan Flannery <ryan.flannery@xxxxxxxxx>
  • To: Joachim Adi Schuetz <adi@xxxxxxxxxxx>, vitunes@xxxxxxxxxxxxx
  • Date: Thu, 24 Feb 2011 13:48:45 -0500

Hi List,

I recently received a diff from a colleauge of Daniel's to control the
volume from within vitunes.  See message below and some further
comments after.

On Sat, Feb 19, 2011 at 3:16 PM, Ryan Flannery <ryan.flannery@xxxxxxxxx> wrote:
> Hi Adi,
>
> On Sat, Feb 19, 2011 at 10:38 AM, Joachim Adi Schuetz <adi@xxxxxxxxxxx> wrote:
>> -----BEGIN PGP SIGNED MESSAGE-----
>> Hash: SHA1
>>
>> hi ryan, fyi daniel,
>>
>> so i took a look at vitunes because i heard about it from a colleague -
>> daniel.
>>
>> for me it was essential to have a possibility to tune the volume within the
>> software because i use an external edirol ua 25 usb-audio device without the
>> possibility of controling the volume with a standard software mixer - it got
>> a poti on the device for that, but its not very convenient to use it in
>> every situation.
>>
>> so i wrote a small patch to control the volume with 2 keys - i use the ,,<''
>> and ,,>'' keys - and a small enhancement to the player to re-tune the volume
>> to the selected value after the next song has started ...
>
> I really like the idea, and will definitely import the patch.
>
>> this seems a little bit funny to me, i thought mplayer keeps the old volume
>> settings for a fresh loaded audiofile ... but apparently it does not.
>
> Gah!  That is weird... mplayer can be down-right odd at times.
>
>> ok ... its only a small and ,,just make it work'' style patch, because i
>> think the mplayer backend is not the best solution - see the problems
>> mentioned above - and so i did not try to include changes to for example
>> keep the volume settings persistent between restarts of vitunes or change
>> the commandline for the mplayer itself to tune the volume at startup ...
>
> The original intention was to support multiple backends, mplayer being
> one of them.  This has been on my "todo" for years... since creating
> vitunes.  Sadly, I just don't have the time to really investigate &
> develop this.
>
>> the big drawback: i re-set the volume to the former value after a new audio
>> file is loaded to the mplayer instance and so there is a short moment at the
>> start of every songtitle where the volume is at 100% ... its only a fraction
>> of a second, but it can be annoying :/
>
> That's not ideal, but for now, I think it's acceptable.  We can
> investigate it further later.
>
>> btw. daniel had a few good ideas to improve the setting - for example to
>> vary the stepswidth and use other keyboard shirtcuts etc. - but i think i
>> only include my changes in this patch ...
>
> OK.  I'd like to commit your patch with a few changes also.  Mostly to
> allow count-prefixes (e.g. '10<' would decrease the audio by 10
> points).  This should be a quick fix.
>

I've re-worked the diff to try and support the following:
1. Numeric prefixes (e.g. '10<' will decrease volume by 10 percent).
2. Try to accurately track the volume, so it can be set later.

Mplayer doesn't track the volume level between files, so it has to be
reset each time.
Worse, the slave-mode commands for working with volume don't *quite*
work as documented, at least on my end.

The commands mplayer has are:

1. volume X [abs]
This is supposed to increase/decrease volume by X percent.  If abs is
provided and non-zero, it should set the volume to X percent exactly.
Problem: I can only get it to increase/decrease, and only by 1
percent/level.  That is, "volume +1" and "volume +50" do the same
thing.

2. step_property volume X [dir]
Same problem with this as above.

3. set_property volume X
This works just fine, setting the volume to the absolute percent.

4. get_property volume
This works fine, except that mplayer doesn't track volume between
songs and if no song is playing, you get an 'property unavailable'
error.


Anyway, the following diff works by tracking the volume in the
player_status struct.
1. volume is initially set to -1
2. Anytime a volume keybinding ('<' and '>') is issued, it steps the
volume by the specified percent.
3. It then queries the volume, which is read in player_monitor.
4. Once a valid volume is read from player_monitor, it's stored in the
player_status
5. After that, numeric prefixes work and songs will be restarted with
the new volume.

For right now, the volume is also reported in the player window
(top-left).  I'll remove this later, but it's handy for testing this,
to know what vitunes thinks the volume is.

cheers,
-ryan


diff --git a/keybindings.c b/keybindings.c
--- a/keybindings.c
+++ b/keybindings.c
@@ -65,6 +65,8 @@ const KeyActionName KeyActionNames[] = {
    { media_stop,              "media_stop" },
    { media_next,              "media_next" },
    { media_prev,              "media_prev" },
+   { volume_increase,         "volume_increase" },
+   { volume_decrease,         "volume_decrease" },
    { seek_forward_seconds,    "seek_forward_seconds" },
    { seek_backward_seconds,   "seek_backward_seconds" },
    { seek_forward_minutes,    "seek_forward_minutes" },
@@ -126,6 +128,8 @@ const KeyActionHandler KeyActionHandlers
 { media_stop,            kba_stop,           false, ARG_NOT_USED },
 { media_next,            kba_play_next,      false, ARG_NOT_USED },
 { media_prev,            kba_play_prev,      false, ARG_NOT_USED },
+{ volume_increase,       kba_volume,         false, { .direction = FORWARDS }},
+{ volume_decrease,       kba_volume,         false, { .direction =
BACKWARDS }},
 { seek_forward_seconds,  kba_seek,           false, { .direction =
FORWARDS,  .scale = SECONDS, .num = 10 }},
 { seek_backward_seconds, kba_seek,           false, { .direction =
BACKWARDS, .scale = SECONDS, .num = 10 }},
 { seek_forward_minutes,  kba_seek,           false, { .direction =
FORWARDS,  .scale = MINUTES, .num = 1 }},
@@ -210,6 +214,8 @@ const KeyBinding DefaultKeyBindings[] =
    { '{',               seek_backward_minutes },
    { '(',               media_prev },
    { ')',               media_next },
+   { '<',               volume_decrease },
+   { '>',               volume_increase },
    { 't',               toggle_forward },
    { 'T',               toggle_backward }
 };
@@ -1469,6 +1475,29 @@ kba_play_prev(KbaArgs a UNUSED)
 }

 void
+kba_volume(KbaArgs a)
+{
+   float pcnt;
+
+   if (gnum_get() > 0)
+      pcnt = gnum_retrieve();
+   else
+      pcnt = 1;
+
+   switch (a.direction) {
+   case FORWARDS:
+      break;
+   case BACKWARDS:
+      pcnt *= -1;
+      break;
+   default:
+      errx(1, "kba_volume: invalid direction");
+   }
+
+   player_volume_step(pcnt);
+}
+
+void
 kba_seek(KbaArgs a)
 {
    int n, secs;
diff --git a/keybindings.h b/keybindings.h
--- a/keybindings.h
+++ b/keybindings.h
@@ -78,6 +78,8 @@ typedef enum {
    media_stop,
    media_next,
    media_prev,
+   volume_increase,
+   volume_decrease,
    seek_forward_seconds,
    seek_backward_seconds,
    seek_forward_minutes,
@@ -150,6 +152,7 @@ void kba_pause(KbaArgs a);
 void kba_stop(KbaArgs a);
 void kba_play_next(KbaArgs a);
 void kba_play_prev(KbaArgs a);
+void kba_volume(KbaArgs a);
 void kba_seek(KbaArgs a);
 void kba_toggle(KbaArgs a);

diff --git a/paint.c b/paint.c
--- a/paint.c
+++ b/paint.c
@@ -183,7 +183,8 @@ paint_player()
    wattron(ui.player, COLOR_PAIR(colors.player));
    mvwprintw(ui.player, 0, 0, num2fmt(w, LEFT), " "); /* this fills
the bg color */
    mvwprintw(ui.player, 0, 0,
-      "[%s] %8.8s +%2.2d:%2.2d:%2.2d (%d%%) %-49.49s",
+      "(%3.1f%%) [%s] %8.8s +%2.2d:%2.2d:%2.2d (%d%%) %-49.49s",
+      player_status.volume,
       playmode,
       (player_status.paused ? "-PAUSED-" : ""),
       in_hour, in_minute, in_second,
diff --git a/player.c b/player.c
--- a/player.c
+++ b/player.c
@@ -33,6 +33,7 @@ player_init(char *prog, char *pargs[], p
    player_status.playing = false;
    player_status.paused = false;
    player_status.position = -1;
+   player_status.volume = -1;

    /* player init */
    player.program = prog;
@@ -181,6 +182,10 @@ player_play()
    player_status.playing = true;
    player_status.paused = false;
    player_status.position = 0;
+
+   /* if we have a volume, reset it */
+   if (player_status.volume > -1)
+      player_volume_set(player_status.volume);
 }

 /*
@@ -303,6 +308,71 @@ player_seek(int seconds)
       player_status.paused = false;
 }

+/* set the volume to the specified percent */
+void
+player_volume_set(float percent)
+{
+   static const char *cmd_fmt = "\npausing_keep set_property volume %f\n";
+   char *cmd;
+
+   /* mplayer doesn't retain volume levels between files? */
+   if (!player_status.playing)
+      return;
+
+   if (percent > 100) percent = 100;
+   if (percent < 0)   percent = 0;
+
+   asprintf(&cmd, cmd_fmt, percent);
+   if (cmd == NULL)
+      err(1, "player_volume_set: asprintf failed");
+
+
+   player_send_cmd(cmd);
+   free(cmd);
+
+   player_volume_query();
+}
+
+/* increase/decrease the volume by the specified percent */
+void
+player_volume_step(float percent)
+{
+   static const char *cmd_fmt = "\npausing_keep volume %f\n";
+   char *cmd;
+
+   if (!player_status.playing)
+      return;
+
+   /* this is a hack, since mplayer's volume command doesn't seem to work
+    * as documented.
+    */
+   if (player_status.volume > -1) {
+      percent += player_status.volume;
+      player_volume_set(percent);
+      return;
+   }
+
+   asprintf(&cmd, cmd_fmt, percent);
+   if (cmd == NULL)
+      err(1, "player_volume_step: asprintf failed");
+
+   player_send_cmd(cmd);
+   free(cmd);
+
+   player_volume_query();
+}
+
+void
+player_volume_query()
+{
+   static const char *cmd = "\npausing_keep get_property volume\n";
+
+   if (!player_status.playing)
+      return;
+
+   player_send_cmd(cmd);
+}
+
 /*****************************************************************************
  * Player monitor function, called repeatedly via the signal handler in the
  * vitunes main loop.
@@ -354,4 +424,14 @@ player_monitor()
    }

    player_send_cmd(query_cmd);
+
+   /* check for recent volume */
+   static const char *volume_good  = "ANS_volume";
+   if ((s = strstr(response, volume_good)) != NULL) {
+      while (strstr(s + 1, volume_good) != NULL)
+         s = strstr(s + 1, volume_good);
+
+      if (sscanf(s, "ANS_volume=%f", &player_status.volume) != 1)
+         errx(1, "player_monitor: player child is misbehaving.");
+   }
 }
diff --git a/player.h b/player.h
--- a/player.h
+++ b/player.h
@@ -60,6 +60,7 @@ typedef struct {
    bool  playing;    /* playing or not (still true if paused) */
    bool  paused;     /* if paused or not */
    float position;   /* position, in seconds, into currently playing file */
+   float volume;     /* volume, in percent */
 } player_status_t;
 extern player_status_t player_status;

@@ -109,6 +110,9 @@ void player_play_prev_song(int skip);
 void player_stop();
 void player_pause();
 void player_seek(int seconds);
+void player_volume_set(float percent);
+void player_volume_step(float percent);
+void player_volume_query();

 /*
  * This should be called periodically to monitor the child media player

Other related posts: