Thanks Vinod for quick review,
On 02/07/2020 12:36, Vinod Koul wrote:
Hi Srini,
On 02-07-20, 12:11, Srinivas Kandagatla wrote:
For gapless playback its possible that each track can have different
s/its/it is
codec profile with same decoder, for example we have WMA album,
we may have different tracks as WMA v9, WMA v10 and so on
Existing code does not allow to change this profile while doing gapless
playback.
This patch adds new SNDRV_COMPRESS_SET_CODEC_PARAMS IOCTL to allow
userspace to set this new parameters required for new codec profile.
Thanks, this looks good and in line with discussions done in Audio uConf
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@xxxxxxxxxx>
---
include/sound/compress_driver.h | 5 +++
include/sound/soc-component.h | 3 ++
include/sound/soc-dai.h | 5 +++
include/uapi/sound/compress_offload.h | 1 +
sound/core/compress_offload.c | 54 ++++++++++++++++++++++++---
sound/soc/soc-compress.c | 30 +++++++++++++++
sound/soc/soc-dai.c | 14 +++++++
Can we split the ALSA changes and ASoC changes to different patches
please?
Also please post driver support for this API..
Yes, q6dsp gapless patches are the users for this api, I will include
them in next version.
Lastly, documentation needs update about this call
I will fix these in next version!
7 files changed, 106 insertions(+), 6 deletions(-)
diff --git a/include/sound/compress_driver.h b/include/sound/compress_driver.h
index 70cbc5095e72..8d23351f7ad7 100644
--- a/include/sound/compress_driver.h
+++ b/include/sound/compress_driver.h
@@ -93,6 +93,9 @@ struct snd_compr_stream {
* @set_params: Sets the compressed stream parameters, mandatory
* This can be called in during stream creation only to set codec params
* and the stream properties
+ * @set_codec_params: Sets the compressed stream codec parameters, mandatory
This should be optional as gapless is optional
--- a/include/uapi/sound/compress_offload.h
+++ b/include/uapi/sound/compress_offload.h
@@ -172,6 +172,7 @@ struct snd_compr_metadata {
struct snd_compr_metadata)
#define SNDRV_COMPRESS_GET_METADATA _IOWR('C', 0x15,\
struct snd_compr_metadata)
+#define SNDRV_COMPRESS_SET_CODEC_PARAMS _IOW('C', 0x16, struct snd_codec)
#define SNDRV_COMPRESS_TSTAMP _IOR('C', 0x20, struct snd_compr_tstamp)
#define SNDRV_COMPRESS_AVAIL _IOR('C', 0x21, struct snd_compr_avail)
#define SNDRV_COMPRESS_PAUSE _IO('C', 0x30)
I think we should bump the compress version too for checking in userland
about this support
Sure!
static int snd_compress_check_input(struct snd_compr_params *params)
{
/* first let's check the buffer parameter's */
@@ -574,14 +586,41 @@ static int snd_compress_check_input(struct snd_compr_params *params)
params->buffer.fragments == 0)
return -EINVAL;
- /* now codec parameters */
- if (params->codec.id == 0 || params->codec.id > SND_AUDIOCODEC_MAX)
- return -EINVAL;
+ return snd_compress_check_codec_params(¶ms->codec);
Can this be 1st patch to prepare for this change?
- if (params->codec.ch_in == 0 || params->codec.ch_out == 0)
- return -EINVAL;
+}
- return 0;
+static int snd_compr_set_codec_params(struct snd_compr_stream *stream,
+ unsigned long arg)
+{
+ struct snd_codec *params;
+ int retval;
+
+ if (!stream->ops->set_codec_params)
+ return -EPERM;
+
+ if (stream->runtime->state != SNDRV_PCM_STATE_RUNNING)
+ return -EPERM;
+
+ /* codec params can be only set when next track has been signalled */
+ if (stream->next_track == false)
+ return -EPERM;
+
+ params = memdup_user((void __user *)arg, sizeof(*params));
+ if (IS_ERR(params))
+ return PTR_ERR(params);
+
+ retval = snd_compress_check_codec_params(params);
+ if (retval)
+ goto out;
+
+ retval = stream->ops->set_codec_params(stream, params);
+ if (retval)
+ goto out;
this jump is superfluous
yes, we could just remove the check!
+
+out:
+ kfree(params);
+ return retval;
}
...
+static int soc_compr_set_codec_params(struct snd_compr_stream *cstream,
+ struct snd_codec *codec)
+{
+ struct snd_soc_pcm_runtime *rtd = cstream->private_data;
+ struct snd_soc_component *component;
+ struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
+ int i, ret;
+
+ mutex_lock_nested(&rtd->card->pcm_mutex, rtd->card->pcm_subclass);
+
+ ret = snd_soc_dai_compr_set_codec_params(cpu_dai, cstream, codec);
+ if (ret < 0)
+ goto err;
+
+ for_each_rtd_components(rtd, i, component) {
+ if (!component->driver->compress_ops ||
+ !component->driver->compress_ops->set_codec_params)
+ continue;
+
+ ret = component->driver->compress_ops->set_codec_params(component, cstream,
+ codec);
maybe use a compress_ops pointer to make this look and read better :)
Sure will do that!
--srini
