diff mbox series

[alsa-lib,3/4] pcm: hw: introduce SNDRV_PCM_INFO_PERFECT_DRAIN

Message ID 20230502115010.986325-4-perex@perex.cz (mailing list archive)
State New, archived
Headers show
Series pcm: hw: implement explicit silencing for snd_pcm_drain | expand

Commit Message

Jaroslav Kysela May 2, 2023, 11:50 a.m. UTC
The driver may not require to touch the sample stream
for the drain operation at all. Handle this situation
in alsa-lib.

Signed-off-by: Jaroslav Kysela <perex@perex.cz>
---
 include/pcm.h               |  1 +
 include/sound/uapi/asound.h |  1 +
 src/pcm/pcm.c               | 23 +++++++++++++++++++++++
 src/pcm/pcm_hw.c            |  4 +++-
 src/pcm/pcm_local.h         |  2 ++
 5 files changed, 30 insertions(+), 1 deletion(-)

Comments

Oswald Buddenhagen May 3, 2023, 11:25 a.m. UTC | #1
On Tue, May 02, 2023 at 01:50:09PM +0200, Jaroslav Kysela wrote:
>The driver may not require to touch the sample stream
>
"touching"

>for the drain operation at all.
>Handle this situation in alsa-lib.
>
this is weird without already knowing the context. i'd instead write:

   Handle the driver informing us that it is not necessary to set up 
   silencing upon playback draining. This will be the case for drivers 
   which are guaranteed to not read any samples beyond the application 
   pointer.

>--- a/src/pcm/pcm.c
>+++ b/src/pcm/pcm.c
>@@ -3707,6 +3707,29 @@ int snd_pcm_hw_params_can_disable_period_wakeup(const snd_pcm_hw_params_t *param
> 	return !!(params->info & SNDRV_PCM_INFO_NO_PERIOD_WAKEUP);
> }
> 
>+/**
>+ * \brief Check if hardware does perfect drain
>
"(is a) perfect drain" vs. "does draining".

>+ * \param params Configuration space
>+ * \retval 0 Hardware doesn't do perfect drain
>+ * \retval 1 Hardware does perfect drain
>+ *
>+ * This function should only be called when the configuration space
>
"should be called only when"

>+ * contains a single configuration. Call #snd_pcm_hw_params to choose
>+ * a single configuration from the configuration space.
>+ *
>+ * The perfect drain means that the hardware does not use samples
>
see above. i guess one way to write it here would be

   "Perfect drain" means [...]

>+ * beyond the stream application pointer.
>+ */

regards
Takashi Iwai May 4, 2023, 8:18 a.m. UTC | #2
On Tue, 02 May 2023 13:50:09 +0200,
Jaroslav Kysela wrote:
> 
> The driver may not require to touch the sample stream
> for the drain operation at all. Handle this situation
> in alsa-lib.
> 
> Signed-off-by: Jaroslav Kysela <perex@perex.cz>

Ideally speaking, the checks and the setups of those new bits should
be coupled with the PCM protocol version check (and the version
bump).

But it seems that you've already applied the series, and practically
seen, those bits should be either not set or harmless, so let's cross
fingers.


thanks,

Takashi
Jaroslav Kysela May 4, 2023, 8:31 a.m. UTC | #3
On 04. 05. 23 10:18, Takashi Iwai wrote:
> On Tue, 02 May 2023 13:50:09 +0200,
> Jaroslav Kysela wrote:
>>
>> The driver may not require to touch the sample stream
>> for the drain operation at all. Handle this situation
>> in alsa-lib.
>>
>> Signed-off-by: Jaroslav Kysela <perex@perex.cz>
> 
> Ideally speaking, the checks and the setups of those new bits should
> be coupled with the PCM protocol version check (and the version
> bump).
> 
> But it seems that you've already applied the series, and practically
> seen, those bits should be either not set or harmless, so let's cross
> fingers.

Exactly, the current kernel code should skip those new flags, so they are used 
in alsa-lib only. It's just something like a "reservation" for the kernel 
space until things are really used there. We can bump the protocol version 
later (perhaps with other changes).

			Jaroslav
Takashi Iwai May 4, 2023, 12:50 p.m. UTC | #4
On Thu, 04 May 2023 10:31:50 +0200,
Jaroslav Kysela wrote:
> 
> On 04. 05. 23 10:18, Takashi Iwai wrote:
> > On Tue, 02 May 2023 13:50:09 +0200,
> > Jaroslav Kysela wrote:
> >> 
> >> The driver may not require to touch the sample stream
> >> for the drain operation at all. Handle this situation
> >> in alsa-lib.
> >> 
> >> Signed-off-by: Jaroslav Kysela <perex@perex.cz>
> > 
> > Ideally speaking, the checks and the setups of those new bits should
> > be coupled with the PCM protocol version check (and the version
> > bump).
> > 
> > But it seems that you've already applied the series, and practically
> > seen, those bits should be either not set or harmless, so let's cross
> > fingers.
> 
> Exactly, the current kernel code should skip those new flags, so they
> are used in alsa-lib only. It's just something like a "reservation"
> for the kernel space until things are really used there. We can bump
> the protocol version later (perhaps with other changes).

If you'd like to include the new drain stuff in ALSA 1.2.9 release, we
can take the uapi change to 6.4-rc1, too.  But then let's bump the PCM
protocol version along with it.

So, if you think it's worth aligning with 6.4-rc1, let me know ASAP.
I planned the submission to Linus in tomorrow, and I'd need to prepare
and merge the uapi updates soon.


thanks,

Takashi
diff mbox series

Patch

diff --git a/include/pcm.h b/include/pcm.h
index b5a514fa..f15462d9 100644
--- a/include/pcm.h
+++ b/include/pcm.h
@@ -722,6 +722,7 @@  int snd_pcm_hw_params_is_half_duplex(const snd_pcm_hw_params_t *params);
 int snd_pcm_hw_params_is_joint_duplex(const snd_pcm_hw_params_t *params);
 int snd_pcm_hw_params_can_sync_start(const snd_pcm_hw_params_t *params);
 int snd_pcm_hw_params_can_disable_period_wakeup(const snd_pcm_hw_params_t *params);
+int snd_pcm_hw_params_does_perfect_drain(const snd_pcm_hw_params_t *params);
 int snd_pcm_hw_params_supports_audio_wallclock_ts(const snd_pcm_hw_params_t *params); /* deprecated, use audio_ts_type */
 int snd_pcm_hw_params_supports_audio_ts_type(const snd_pcm_hw_params_t *params, int type);
 int snd_pcm_hw_params_get_rate_numden(const snd_pcm_hw_params_t *params,
diff --git a/include/sound/uapi/asound.h b/include/sound/uapi/asound.h
index fc18c024..0b8834f2 100644
--- a/include/sound/uapi/asound.h
+++ b/include/sound/uapi/asound.h
@@ -281,6 +281,7 @@  typedef int __bitwise snd_pcm_subformat_t;
 #define SNDRV_PCM_INFO_DOUBLE		0x00000004	/* Double buffering needed for PCM start/stop */
 #define SNDRV_PCM_INFO_BATCH		0x00000010	/* double buffering */
 #define SNDRV_PCM_INFO_SYNC_APPLPTR	0x00000020	/* need the explicit sync of appl_ptr update */
+#define SNDRV_PCM_INFO_PERFECT_DRAIN	0x00000040	/* silencing at the end of stream is not required */
 #define SNDRV_PCM_INFO_INTERLEAVED	0x00000100	/* channels are interleaved */
 #define SNDRV_PCM_INFO_NONINTERLEAVED	0x00000200	/* channels are not interleaved */
 #define SNDRV_PCM_INFO_COMPLEX		0x00000400	/* complex frame organization (mmap only) */
diff --git a/src/pcm/pcm.c b/src/pcm/pcm.c
index 88b13ed4..099d83ee 100644
--- a/src/pcm/pcm.c
+++ b/src/pcm/pcm.c
@@ -3707,6 +3707,29 @@  int snd_pcm_hw_params_can_disable_period_wakeup(const snd_pcm_hw_params_t *param
 	return !!(params->info & SNDRV_PCM_INFO_NO_PERIOD_WAKEUP);
 }
 
+/**
+ * \brief Check if hardware does perfect drain
+ * \param params Configuration space
+ * \retval 0 Hardware doesn't do perfect drain
+ * \retval 1 Hardware does perfect drain
+ *
+ * This function should only be called when the configuration space
+ * contains a single configuration. Call #snd_pcm_hw_params to choose
+ * a single configuration from the configuration space.
+ *
+ * The perfect drain means that the hardware does not use samples
+ * beyond the stream application pointer.
+ */
+int snd_pcm_hw_params_does_perfect_drain(const snd_pcm_hw_params_t *params)
+{
+	assert(params);
+	if (CHECK_SANITY(params->info == ~0U)) {
+		SNDMSG("invalid PCM info field");
+		return 0; /* FIXME: should be a negative error? */
+	}
+	return !!(params->info & SNDRV_PCM_INFO_PERFECT_DRAIN);
+}
+
 /**
  * \brief Check if hardware supports audio wallclock timestamps
  * \param params Configuration space
diff --git a/src/pcm/pcm_hw.c b/src/pcm/pcm_hw.c
index 30ff843c..ea0c2ef2 100644
--- a/src/pcm/pcm_hw.c
+++ b/src/pcm/pcm_hw.c
@@ -99,6 +99,7 @@  typedef struct {
 	struct snd_pcm_sync_ptr *sync_ptr;
 
 	bool prepare_reset_sw_params;
+	bool perfect_drain;
 
 	int period_event;
 	snd_timer_t *period_timer;
@@ -398,6 +399,7 @@  static int snd_pcm_hw_hw_params(snd_pcm_t *pcm, snd_pcm_hw_params_t * params)
 	params->info &= ~0xf0000000;
 	if (pcm->tstamp_type != SND_PCM_TSTAMP_TYPE_GETTIMEOFDAY)
 		params->info |= SND_PCM_INFO_MONOTONIC;
+	hw->perfect_drain = !!(params->info & SND_PCM_INFO_PERFECT_DRAIN);
 	return query_status_data(hw);
 }
 
@@ -739,7 +741,7 @@  static int snd_pcm_hw_drain(snd_pcm_t *pcm)
 
 	if (pcm->stream != SND_PCM_STREAM_PLAYBACK)
 		goto __skip_silence;
-	if (hw->drain_silence == 0)
+	if (hw->drain_silence == 0 || hw->perfect_drain)
 		goto __skip_silence;
 	snd_pcm_sw_params_current_no_lock(pcm, &sw_params);
 	if (hw->drain_silence > 0) {
diff --git a/src/pcm/pcm_local.h b/src/pcm/pcm_local.h
index 4a859cd1..b039dda0 100644
--- a/src/pcm/pcm_local.h
+++ b/src/pcm/pcm_local.h
@@ -79,6 +79,8 @@ 
 #define SND_PCM_INFO_DOUBLE SNDRV_PCM_INFO_DOUBLE
 /** device transfers samples in batch */
 #define SND_PCM_INFO_BATCH SNDRV_PCM_INFO_BATCH
+/** device does perfect drain (silencing not required) */
+#define SND_PCM_INFO_PERFECT_DRAIN SNDRV_PCM_INFO_PERFECT_DRAIN
 /** device accepts interleaved samples */
 #define SND_PCM_INFO_INTERLEAVED SNDRV_PCM_INFO_INTERLEAVED
 /** device accepts non-interleaved samples */