diff mbox series

ALSA: seq: Add tempo base unit for MIDI2 Set Tempo messages

Message ID 20240704123549.13485-1-tiwai@suse.de (mailing list archive)
State Superseded
Headers show
Series ALSA: seq: Add tempo base unit for MIDI2 Set Tempo messages | expand

Commit Message

Takashi Iwai July 4, 2024, 12:35 p.m. UTC
MIDI2 Set Tempo message defines the tempo in 10ns unit for finer
accuracy, while MIDI1 was defined in 1us unit.  For adapting this
different unit, introduce "tempo_base" field to snd_seq_queue_tempo
struct so that user-space can pass the proper tempo base unit.

The accepted value is limited, it must be either 0, 10 or 1000.

The protocol version is bumped to 1.0.4 along with this.

The access with the older protocol version ignores the tempo-base
value in ioctls and always treats as 1000.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 include/uapi/sound/asequencer.h |  7 ++++---
 sound/core/seq/seq_clientmgr.c  |  4 ++++
 sound/core/seq/seq_queue.c      |  6 ++++--
 sound/core/seq/seq_timer.c      | 21 +++++++++++++++------
 sound/core/seq/seq_timer.h      |  4 +++-
 5 files changed, 30 insertions(+), 12 deletions(-)

Comments

Jaroslav Kysela July 5, 2024, 7:37 a.m. UTC | #1
On 04. 07. 24 14:35, Takashi Iwai wrote:
> MIDI2 Set Tempo message defines the tempo in 10ns unit for finer
> accuracy, while MIDI1 was defined in 1us unit.  For adapting this
> different unit, introduce "tempo_base" field to snd_seq_queue_tempo
> struct so that user-space can pass the proper tempo base unit.
> 
> The accepted value is limited, it must be either 0, 10 or 1000.
> 
> The protocol version is bumped to 1.0.4 along with this.
> 
> The access with the older protocol version ignores the tempo-base
> value in ioctls and always treats as 1000.
> 
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>   include/uapi/sound/asequencer.h |  7 ++++---
>   sound/core/seq/seq_clientmgr.c  |  4 ++++
>   sound/core/seq/seq_queue.c      |  6 ++++--
>   sound/core/seq/seq_timer.c      | 21 +++++++++++++++------
>   sound/core/seq/seq_timer.h      |  4 +++-
>   5 files changed, 30 insertions(+), 12 deletions(-)
> 
> diff --git a/include/uapi/sound/asequencer.h b/include/uapi/sound/asequencer.h
> index c85fdd8895d8..c7acfeb514a3 100644
> --- a/include/uapi/sound/asequencer.h
> +++ b/include/uapi/sound/asequencer.h
> @@ -10,7 +10,7 @@
>   #include <sound/asound.h>
>   
>   /** version of the sequencer */
> -#define SNDRV_SEQ_VERSION SNDRV_PROTOCOL_VERSION(1, 0, 3)
> +#define SNDRV_SEQ_VERSION SNDRV_PROTOCOL_VERSION(1, 0, 4)
>   
>   /**
>    * definition of sequencer event types
> @@ -523,11 +523,12 @@ struct snd_seq_queue_status {
>   /* queue tempo */
>   struct snd_seq_queue_tempo {
>   	int queue;			/* sequencer queue */
> -	unsigned int tempo;		/* current tempo, us/tick */
> +	unsigned int tempo;		/* current tempo, us/tick (or different time-base below) */
>   	int ppq;			/* time resolution, ticks/quarter */
>   	unsigned int skew_value;	/* queue skew */
>   	unsigned int skew_base;		/* queue skew base */
> -	char reserved[24];		/* for the future */
> +	unsigned int tempo_base;	/* tempo base in nsec unit; either 10 or 1000 */

Perhaps unsigned short value may be enough to carry this settings? The two 
bytes will be always zero for unsigned int.

					Jaroslav
Takashi Iwai July 5, 2024, 7:40 a.m. UTC | #2
On Fri, 05 Jul 2024 09:37:15 +0200,
Jaroslav Kysela wrote:
> 
> On 04. 07. 24 14:35, Takashi Iwai wrote:
> > MIDI2 Set Tempo message defines the tempo in 10ns unit for finer
> > accuracy, while MIDI1 was defined in 1us unit.  For adapting this
> > different unit, introduce "tempo_base" field to snd_seq_queue_tempo
> > struct so that user-space can pass the proper tempo base unit.
> > 
> > The accepted value is limited, it must be either 0, 10 or 1000.
> > 
> > The protocol version is bumped to 1.0.4 along with this.
> > 
> > The access with the older protocol version ignores the tempo-base
> > value in ioctls and always treats as 1000.
> > 
> > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> > ---
> >   include/uapi/sound/asequencer.h |  7 ++++---
> >   sound/core/seq/seq_clientmgr.c  |  4 ++++
> >   sound/core/seq/seq_queue.c      |  6 ++++--
> >   sound/core/seq/seq_timer.c      | 21 +++++++++++++++------
> >   sound/core/seq/seq_timer.h      |  4 +++-
> >   5 files changed, 30 insertions(+), 12 deletions(-)
> > 
> > diff --git a/include/uapi/sound/asequencer.h b/include/uapi/sound/asequencer.h
> > index c85fdd8895d8..c7acfeb514a3 100644
> > --- a/include/uapi/sound/asequencer.h
> > +++ b/include/uapi/sound/asequencer.h
> > @@ -10,7 +10,7 @@
> >   #include <sound/asound.h>
> >     /** version of the sequencer */
> > -#define SNDRV_SEQ_VERSION SNDRV_PROTOCOL_VERSION(1, 0, 3)
> > +#define SNDRV_SEQ_VERSION SNDRV_PROTOCOL_VERSION(1, 0, 4)
> >     /**
> >    * definition of sequencer event types
> > @@ -523,11 +523,12 @@ struct snd_seq_queue_status {
> >   /* queue tempo */
> >   struct snd_seq_queue_tempo {
> >   	int queue;			/* sequencer queue */
> > -	unsigned int tempo;		/* current tempo, us/tick */
> > +	unsigned int tempo;		/* current tempo, us/tick (or different time-base below) */
> >   	int ppq;			/* time resolution, ticks/quarter */
> >   	unsigned int skew_value;	/* queue skew */
> >   	unsigned int skew_base;		/* queue skew base */
> > -	char reserved[24];		/* for the future */
> > +	unsigned int tempo_base;	/* tempo base in nsec unit; either 10 or 1000 */
> 
> Perhaps unsigned short value may be enough to carry this settings? The
> two bytes will be always zero for unsigned int.

True, it's unlikely that the value will be more than 1000.


Takashi
diff mbox series

Patch

diff --git a/include/uapi/sound/asequencer.h b/include/uapi/sound/asequencer.h
index c85fdd8895d8..c7acfeb514a3 100644
--- a/include/uapi/sound/asequencer.h
+++ b/include/uapi/sound/asequencer.h
@@ -10,7 +10,7 @@ 
 #include <sound/asound.h>
 
 /** version of the sequencer */
-#define SNDRV_SEQ_VERSION SNDRV_PROTOCOL_VERSION(1, 0, 3)
+#define SNDRV_SEQ_VERSION SNDRV_PROTOCOL_VERSION(1, 0, 4)
 
 /**
  * definition of sequencer event types
@@ -523,11 +523,12 @@  struct snd_seq_queue_status {
 /* queue tempo */
 struct snd_seq_queue_tempo {
 	int queue;			/* sequencer queue */
-	unsigned int tempo;		/* current tempo, us/tick */
+	unsigned int tempo;		/* current tempo, us/tick (or different time-base below) */
 	int ppq;			/* time resolution, ticks/quarter */
 	unsigned int skew_value;	/* queue skew */
 	unsigned int skew_base;		/* queue skew base */
-	char reserved[24];		/* for the future */
+	unsigned int tempo_base;	/* tempo base in nsec unit; either 10 or 1000 */
+	char reserved[20];		/* for the future */
 };
 
 
diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index 42a705141050..8c4ee5066afe 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -1718,6 +1718,8 @@  static int snd_seq_ioctl_get_queue_tempo(struct snd_seq_client *client,
 	tempo->ppq = tmr->ppq;
 	tempo->skew_value = tmr->skew;
 	tempo->skew_base = tmr->skew_base;
+	if (client->user_pversion >= SNDRV_PROTOCOL_VERSION(1, 0, 4))
+		tempo->tempo_base = tmr->tempo_base;
 	queuefree(queue);
 
 	return 0;
@@ -1739,6 +1741,8 @@  static int snd_seq_ioctl_set_queue_tempo(struct snd_seq_client *client,
 	struct snd_seq_queue_tempo *tempo = arg;
 	int result;
 
+	if (client->user_pversion < SNDRV_PROTOCOL_VERSION(1, 0, 4))
+		tempo->tempo_base = 0;
 	result = snd_seq_set_queue_tempo(client->number, tempo);
 	return result < 0 ? result : 0;
 }
diff --git a/sound/core/seq/seq_queue.c b/sound/core/seq/seq_queue.c
index 500ee6b19c71..5df26788dda4 100644
--- a/sound/core/seq/seq_queue.c
+++ b/sound/core/seq/seq_queue.c
@@ -460,7 +460,8 @@  int snd_seq_queue_timer_set_tempo(int queueid, int client,
 		return -EPERM;
 	}
 
-	result = snd_seq_timer_set_tempo_ppq(q->timer, info->tempo, info->ppq);
+	result = snd_seq_timer_set_tempo_ppq(q->timer, info->tempo, info->ppq,
+					     info->tempo_base);
 	if (result >= 0 && info->skew_base > 0)
 		result = snd_seq_timer_set_skew(q->timer, info->skew_value,
 						info->skew_base);
@@ -724,7 +725,7 @@  void snd_seq_info_queues_read(struct snd_info_entry *entry,
 
 		tmr = q->timer;
 		if (tmr->tempo)
-			bpm = 60000000 / tmr->tempo;
+			bpm = (60000 * tmr->tempo_base) / tmr->tempo;
 		else
 			bpm = 0;
 
@@ -741,6 +742,7 @@  void snd_seq_info_queues_read(struct snd_info_entry *entry,
 		snd_iprintf(buffer, "timer state        : %s\n", tmr->running ? "Running" : "Stopped");
 		snd_iprintf(buffer, "timer PPQ          : %d\n", tmr->ppq);
 		snd_iprintf(buffer, "current tempo      : %d\n", tmr->tempo);
+		snd_iprintf(buffer, "tempo base         : %d ns\n", tmr->tempo_base);
 		snd_iprintf(buffer, "current BPM        : %d\n", bpm);
 		snd_iprintf(buffer, "current time       : %d.%09d s\n", tmr->cur_time.tv_sec, tmr->cur_time.tv_nsec);
 		snd_iprintf(buffer, "current tick       : %d\n", tmr->tick.cur_tick);
diff --git a/sound/core/seq/seq_timer.c b/sound/core/seq/seq_timer.c
index ad2b97e2762d..c9f0392ac7f1 100644
--- a/sound/core/seq/seq_timer.c
+++ b/sound/core/seq/seq_timer.c
@@ -20,14 +20,17 @@ 
 
 static void snd_seq_timer_set_tick_resolution(struct snd_seq_timer *tmr)
 {
-	if (tmr->tempo < 1000000)
-		tmr->tick.resolution = (tmr->tempo * 1000) / tmr->ppq;
+	unsigned int threshold =
+		tmr->tempo_base == 1000 ? 1000000 : 10000;
+
+	if (tmr->tempo < threshold)
+		tmr->tick.resolution = (tmr->tempo * tmr->tempo_base) / tmr->ppq;
 	else {
 		/* might overflow.. */
 		unsigned int s;
 		s = tmr->tempo % tmr->ppq;
-		s = (s * 1000) / tmr->ppq;
-		tmr->tick.resolution = (tmr->tempo / tmr->ppq) * 1000;
+		s = (s * tmr->tempo_base) / tmr->ppq;
+		tmr->tick.resolution = (tmr->tempo / tmr->ppq) * tmr->tempo_base;
 		tmr->tick.resolution += s;
 	}
 	if (tmr->tick.resolution <= 0)
@@ -79,6 +82,7 @@  void snd_seq_timer_defaults(struct snd_seq_timer * tmr)
 	/* setup defaults */
 	tmr->ppq = 96;		/* 96 PPQ */
 	tmr->tempo = 500000;	/* 120 BPM */
+	tmr->tempo_base = 1000;	/* 1us */
 	snd_seq_timer_set_tick_resolution(tmr);
 	tmr->running = 0;
 
@@ -164,8 +168,9 @@  int snd_seq_timer_set_tempo(struct snd_seq_timer * tmr, int tempo)
 	return 0;
 }
 
-/* set current tempo and ppq in a shot */
-int snd_seq_timer_set_tempo_ppq(struct snd_seq_timer *tmr, int tempo, int ppq)
+/* set current tempo, ppq and base in a shot */
+int snd_seq_timer_set_tempo_ppq(struct snd_seq_timer *tmr, int tempo, int ppq,
+				unsigned int tempo_base)
 {
 	int changed;
 
@@ -173,6 +178,9 @@  int snd_seq_timer_set_tempo_ppq(struct snd_seq_timer *tmr, int tempo, int ppq)
 		return -EINVAL;
 	if (tempo <= 0 || ppq <= 0)
 		return -EINVAL;
+	/* allow only 10ns or 1us tempo base for now */
+	if (tempo_base && tempo_base != 10 && tempo_base != 1000)
+		return -EINVAL;
 	guard(spinlock_irqsave)(&tmr->lock);
 	if (tmr->running && (ppq != tmr->ppq)) {
 		/* refuse to change ppq on running timers */
@@ -183,6 +191,7 @@  int snd_seq_timer_set_tempo_ppq(struct snd_seq_timer *tmr, int tempo, int ppq)
 	changed = (tempo != tmr->tempo) || (ppq != tmr->ppq);
 	tmr->tempo = tempo;
 	tmr->ppq = ppq;
+	tmr->tempo_base = tempo_base ? tempo_base : 1000;
 	if (changed)
 		snd_seq_timer_set_tick_resolution(tmr);
 	return 0;
diff --git a/sound/core/seq/seq_timer.h b/sound/core/seq/seq_timer.h
index 4bec57df8158..3b906064bde1 100644
--- a/sound/core/seq/seq_timer.h
+++ b/sound/core/seq/seq_timer.h
@@ -36,6 +36,7 @@  struct snd_seq_timer {
 
 	unsigned int skew;
 	unsigned int skew_base;
+	unsigned int tempo_base;
 
 	struct timespec64	last_update;	 /* time of last clock update, used for interpolation */
 
@@ -116,7 +117,8 @@  int snd_seq_timer_stop(struct snd_seq_timer *tmr);
 int snd_seq_timer_start(struct snd_seq_timer *tmr);
 int snd_seq_timer_continue(struct snd_seq_timer *tmr);
 int snd_seq_timer_set_tempo(struct snd_seq_timer *tmr, int tempo);
-int snd_seq_timer_set_tempo_ppq(struct snd_seq_timer *tmr, int tempo, int ppq);
+int snd_seq_timer_set_tempo_ppq(struct snd_seq_timer *tmr, int tempo, int ppq,
+				unsigned int tempo_base);
 int snd_seq_timer_set_position_tick(struct snd_seq_timer *tmr, snd_seq_tick_time_t position);
 int snd_seq_timer_set_position_time(struct snd_seq_timer *tmr, snd_seq_real_time_t position);
 int snd_seq_timer_set_skew(struct snd_seq_timer *tmr, unsigned int skew, unsigned int base);