diff mbox

[alsa-lib] pcm: add set and get methods for no-rewind flag

Message ID 1517304808-16649-1-git-send-email-sriramx.periyasamy@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Sriram Periyasamy Jan. 30, 2018, 9:33 a.m. UTC 
From: Ramesh Babu <ramesh.babu@intel.com>

Add functions to set/get no-rewind flag

Signed-off-by: Ramesh Babu <ramesh.babu@intel.com>
Signed-off-by: Sriram Periyasamy <sriramx.periyasamy@intel.com>
---
 include/pcm.h          |  2 ++
 include/sound/asound.h |  1 +
 src/pcm/pcm.c          | 40 ++++++++++++++++++++++++++++++++++++++++
 src/pcm/pcm_local.h    |  1 +
 4 files changed, 44 insertions(+)

Comments

Takashi Sakamoto Jan. 30, 2018, 10:42 a.m. UTC | #1 
Hi,

On Jan 30 2018 18:33, Sriram Periyasamy wrote:
> From: Ramesh Babu <ramesh.babu@intel.com>
> 
> Add functions to set/get no-rewind flag
> 
> Signed-off-by: Ramesh Babu <ramesh.babu@intel.com>
> Signed-off-by: Sriram Periyasamy <sriramx.periyasamy@intel.com>
> ---
>  include/pcm.h          |  2 ++
>  include/sound/asound.h |  1 +
>  src/pcm/pcm.c          | 40 ++++++++++++++++++++++++++++++++++++++++
>  src/pcm/pcm_local.h    |  1 +
>  4 files changed, 44 insertions(+)

For this new flag, I post my comment to your work for kernel land[1].
Please refer to it.

[1]
http://mailman.alsa-project.org/pipermail/alsa-devel/2018-January/131336.html

Regards


Takashi Sakamoto
Pierre-Louis Bossart Jan. 31, 2018, 12:21 a.m. UTC | #2 
On 1/30/18 11:42 AM, Takashi Sakamoto wrote:
> Hi,
> 
> On Jan 30 2018 18:33, Sriram Periyasamy wrote:
>> From: Ramesh Babu <ramesh.babu@intel.com>
>>
>> Add functions to set/get no-rewind flag
>>
>> Signed-off-by: Ramesh Babu <ramesh.babu@intel.com>
>> Signed-off-by: Sriram Periyasamy <sriramx.periyasamy@intel.com>
>> ---
>>   include/pcm.h          |  2 ++
>>   include/sound/asound.h |  1 +
>>   src/pcm/pcm.c          | 40 ++++++++++++++++++++++++++++++++++++++++
>>   src/pcm/pcm_local.h    |  1 +
>>   4 files changed, 44 insertions(+)
> 
> For this new flag, I post my comment to your work for kernel land[1].
> Please refer to it.
> 
> [1]
> http://mailman.alsa-project.org/pipermail/alsa-devel/2018-January/131336.html

I believed we missed your feedback, thanks for providing the pointer.

You wrote
"In my opinion, when drivers
return appropriate values at implementations of
"struct snd_pcm_ops.pointer" and "struct snd_pcm_ops.ack", your aim is
satisfied. In short, you can let ALSA PCM core to handle
rewinding/forwarding requests from userland for zero number of handled
frames in result. So the 'SNDRV_PCM_HW_PARAMS_NO_REWINDS' flag is
useless"

You may have missed that the hardware needs to know when the stream is 
opened if rewinds will be done or not. It's a contract between 
application and driver, just like the NO_IRQ mode, not a dynamic choice. 
If we enable the hardware capabilities and the application does a 
rewind, then it will be a true error with the stream having to be 
closed. That's not really desirable, is it?


> 
> Regards
> 
> 
> Takashi Sakamoto
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>
diff mbox

Patch

diff --git a/include/pcm.h b/include/pcm.h
index 2619c8cd8bd4..8c0b19c106b0 100644
--- a/include/pcm.h
+++ b/include/pcm.h
@@ -805,6 +805,8 @@  int snd_pcm_hw_params_set_export_buffer(snd_pcm_t *pcm, snd_pcm_hw_params_t *par
 int snd_pcm_hw_params_get_export_buffer(snd_pcm_t *pcm, snd_pcm_hw_params_t *params, unsigned int *val);
 int snd_pcm_hw_params_set_period_wakeup(snd_pcm_t *pcm, snd_pcm_hw_params_t *params, unsigned int val);
 int snd_pcm_hw_params_get_period_wakeup(snd_pcm_t *pcm, snd_pcm_hw_params_t *params, unsigned int *val);
+int snd_pcm_hw_params_set_no_rewind(snd_pcm_t *pcm, snd_pcm_hw_params_t *params, unsigned int val);
+int snd_pcm_hw_params_get_no_rewind(snd_pcm_t *pcm, snd_pcm_hw_params_t *params, unsigned int *val);
 
 int snd_pcm_hw_params_get_period_time(const snd_pcm_hw_params_t *params, unsigned int *val, int *dir);
 int snd_pcm_hw_params_get_period_time_min(const snd_pcm_hw_params_t *params, unsigned int *val, int *dir);
diff --git a/include/sound/asound.h b/include/sound/asound.h
index 6041cab27a23..35fd50ec5395 100644
--- a/include/sound/asound.h
+++ b/include/sound/asound.h
@@ -375,6 +375,7 @@  typedef int snd_pcm_hw_param_t;
 #define SNDRV_PCM_HW_PARAMS_NORESAMPLE	(1<<0)	/* avoid rate resampling */
 #define SNDRV_PCM_HW_PARAMS_EXPORT_BUFFER	(1<<1)	/* export buffer */
 #define SNDRV_PCM_HW_PARAMS_NO_PERIOD_WAKEUP	(1<<2)	/* disable period wakeups */
+#define SNDRV_PCM_HW_PARAMS_NO_REWINDS          (1<<3)  /* disable rewinds */
 
 struct snd_interval {
 	unsigned int min, max;
diff --git a/src/pcm/pcm.c b/src/pcm/pcm.c
index 69d7d66500ea..c7041be47be6 100644
--- a/src/pcm/pcm.c
+++ b/src/pcm/pcm.c
@@ -4763,6 +4763,46 @@  int snd_pcm_hw_params_get_period_wakeup(snd_pcm_t *pcm, snd_pcm_hw_params_t *par
 }
 
 /**
+ * \brief Configure no-rewind flag through configuration space
+ * \param pcm PCM handle
+ * \param params Configuration space
+ * \param val 0 = disable (default), 1 = enable (default) no-rewind flag
+ * \return Zero on success, otherwise a negative error code.
+ *
+ * This function can be called to inform driver about application uses rewind or not
+ *
+ * Based on no-rewind flag, underlying driver could take decision on buffering scheme.
+ */
+
+/* TODO: Invert the logic with function name set_rewind */
+int snd_pcm_hw_params_set_no_rewind(snd_pcm_t *pcm, snd_pcm_hw_params_t *params, unsigned int val)
+{
+	assert(pcm && params);
+
+	if (val) {
+		params->flags |= SND_PCM_HW_PARAMS_NO_REWINDS;
+	} else
+		params->flags &= ~SND_PCM_HW_PARAMS_NO_REWINDS;
+	params->rmask = ~0;
+
+	return snd_pcm_hw_refine(pcm, params);
+}
+
+/**
+ * \brief Extract no-rewind flag from a configuration space
+ * \param pcm PCM handle
+ * \param params Configuration space
+ * \param val 0 = disabled, 1 = enabled no-rewind flag
+ * \return Zero on success, otherwise a negative error code.
+ */
+int snd_pcm_hw_params_get_no_rewind(snd_pcm_t *pcm, snd_pcm_hw_params_t *params, unsigned int *val)
+{
+	assert(pcm && params && val);
+	*val = params->flags & SND_PCM_HW_PARAMS_NO_REWINDS ? 0 : 1;
+	return 0;
+}
+
+/**
  * \brief Extract period time from a configuration space
  * \param params Configuration space
  * \param val Returned approximate period duration in us
diff --git a/src/pcm/pcm_local.h b/src/pcm/pcm_local.h
index 3d95e1749169..91479cabff88 100644
--- a/src/pcm/pcm_local.h
+++ b/src/pcm/pcm_local.h
@@ -103,6 +103,7 @@ 
 #define SND_PCM_HW_PARAMS_NORESAMPLE SNDRV_PCM_HW_PARAMS_NORESAMPLE
 #define SND_PCM_HW_PARAMS_EXPORT_BUFFER SNDRV_PCM_HW_PARAMS_EXPORT_BUFFER
 #define SND_PCM_HW_PARAMS_NO_PERIOD_WAKEUP SNDRV_PCM_HW_PARAMS_NO_PERIOD_WAKEUP
+#define SND_PCM_HW_PARAMS_NO_REWINDS SNDRV_PCM_HW_PARAMS_NO_REWINDS
 
 #define SND_PCM_INFO_MONOTONIC	0x80000000