Re: [alsa-devel] compress: adding support for hardware dependent params
We have a requirement, where we need to set a number of hardware specific parameters for compressed offload use-cases, before starting a stream. As these parameters need to be set once for the entire session, we would like to explore the best way to set these additional parameters somewhere before calling trigger() function. Below are few of the proposed solutions that we could think of at this point for getting this done-
As you have not mentioned what kind of metadata you are talking about I am assuming if your meta data can fit in below structure then we already have a code in upstream which takes care of your need.
140 struct snd_compr_metadata { 141 __u32 key; 142 __u32 value[8]; 143 };
Structure snd_compr_ops has set_metadata and get_metadata callbacks which you can use for your need.
88 * struct snd_compr_ops: compressed path DSP operations 89 * @open: Open the compressed stream 90 * This callback is mandatory and shall keep dsp ready to receive the stream 91 * parameter 92 * @free: Close the compressed stream, mandatory 93 * @set_params: Sets the compressed stream parameters, mandatory 94 * This can be called in during stream creation only to set codec params 95 * and the stream properties 96 * @get_params: retrieve the codec parameters, mandatory 97 * @trigger: Trigger operations like start, pause, resume, drain, stop. 98 * This callback is mandatory 99 * @pointer: Retrieve current h/w pointer information. Mandatory 100 * @copy: Copy the compressed data to/from userspace, Optional 101 * Can't be implemented if DSP supports mmap 102 * @mmap: DSP mmap method to mmap DSP memory 103 * @ack: Ack for DSP when data is written to audio buffer, Optional 104 * Not valid if copy is implemented 105 * @get_caps: Retrieve DSP capabilities, mandatory 106 * @get_codec_caps: Retrieve capabilities for a specific codec, mandatory 107 */
It would be nice if someone can send a patch here to describe set and get_metadata callbacks in compress_driver.h
- Adding new IOCTL call: Add a new ioctl control in compress-offload.c
file to handle hardware specific parameters setting. This new ioctl control will call into soc-compress.c and eventually call into platform driver ioctl to set those hardware specific parameters. In this case, we will add a new structure, let's say "snd_compr_hw_ctrl_params", with the same fields as "snd_compr_metadata" struct.
- Adding a function hw_dependant_hw_params(), which will be called after
hw_params() and before prepare() function. The new function will eventually call into the platform driver to get the hardware specific parameters set. Once implemented, this would look something like- hw_params(); hw_dependant_hw_params(); prepare();
- Add a function "hw_dependant_hw_params()", as _part_ of the hw_params()
function. This new function will be called after the (existing) generic parameters are set. So once implemented, the proposed function calls would look like- hw_params() { generic_hw_params(); hw_dependant_hw_params(); } prepare();
We would like to get feedback on these proposed solutions and get a discussion going to see how best these requirements can be fitted in the current Compressed ALSA framework (for mainlining).
Can you explain your meta data?
Be the change you want to see in the world:Gandhi
On Mon, 2014-02-03 at 01:01 -0800, noman pouigt wrote:
We have a requirement, where we need to set a number of hardware specific parameters for compressed offload use-cases, before starting a stream. As these parameters need to be set once for the entire session, we would like to explore the best way to set these additional parameters somewhere before calling trigger() function. Below are few of the proposed solutions that we could think of at this point for getting this done-
As you have not mentioned what kind of metadata you are talking about I am assuming if your meta data can fit in below structure then we already have a code in upstream which takes care of your need.
No this is for audio file metadata which should be passed to decoder running in your DSP.
Yes as you and Pierre said in the absence of any specifics in the parameters it would be very difficult to comment on how to go about it. For offload specific case, you only need to know how to configure the decoder, which this API tries to do quite well. If you have something HW specific that should be same issue in PCM handling too, how do you do it there. I am suspecting the problem statement isn't correct here!
140 struct snd_compr_metadata { 141 __u32 key; 142 __u32 value[8]; 143 };
Structure snd_compr_ops has set_metadata and get_metadata callbacks which you can use for your need.
88 * struct snd_compr_ops: compressed path DSP operations 89 * @open: Open the compressed stream 90 * This callback is mandatory and shall keep dsp ready to receive the stream 91 * parameter 92 * @free: Close the compressed stream, mandatory 93 * @set_params: Sets the compressed stream parameters, mandatory 94 * This can be called in during stream creation only to set codec params 95 * and the stream properties 96 * @get_params: retrieve the codec parameters, mandatory 97 * @trigger: Trigger operations like start, pause, resume, drain, stop. 98 * This callback is mandatory 99 * @pointer: Retrieve current h/w pointer information. Mandatory 100 * @copy: Copy the compressed data to/from userspace, Optional 101 * Can't be implemented if DSP supports mmap 102 * @mmap: DSP mmap method to mmap DSP memory 103 * @ack: Ack for DSP when data is written to audio buffer, Optional 104 * Not valid if copy is implemented 105 * @get_caps: Retrieve DSP capabilities, mandatory 106 * @get_codec_caps: Retrieve capabilities for a specific codec, mandatory 107 */
It would be nice if someone can send a patch here to describe set and get_metadata callbacks in compress_driver.h
Thanks for pointing, will do the needful
- Adding new IOCTL call: Add a new ioctl control in compress-offload.c
file to handle hardware specific parameters setting. This new ioctl control will call into soc-compress.c and eventually call into platform driver ioctl to set those hardware specific parameters. In this case, we will add a new structure, let's say "snd_compr_hw_ctrl_params", with the same fields as "snd_compr_metadata" struct.
- Adding a function hw_dependant_hw_params(), which will be called after
hw_params() and before prepare() function. The new function will eventually call into the platform driver to get the hardware specific parameters set. Once implemented, this would look something like- hw_params(); hw_dependant_hw_params(); prepare();
- Add a function "hw_dependant_hw_params()", as _part_ of the hw_params()
function. This new function will be called after the (existing) generic parameters are set. So once implemented, the proposed function calls would look like- hw_params() { generic_hw_params(); hw_dependant_hw_params(); } prepare();
We would like to get feedback on these proposed solutions and get a discussion going to see how best these requirements can be fitted in the current Compressed ALSA framework (for mainlining).
Can you explain your meta data?
Be the change you want to see in the world:Gandhi
Hi,
We have a requirement, where we need to set a number of hardware specific parameters for compressed offload use-cases, before starting a stream. As these parameters need to be set once for the entire session, we would like to explore the best way to set these additional parameters somewhere before calling trigger() function. Below are few of the proposed solutions that we could think of at this point for getting this done-
As you have not mentioned what kind of metadata you are talking about I am assuming if your meta data can fit in below structure then we already have a code in upstream which takes care of your need.
140 struct snd_compr_metadata { 141 __u32 key; 142 __u32 value[8]; 143 };
Structure snd_compr_ops has set_metadata and get_metadata callbacks which you can use for your need.
We can probably use the same snd_compr_metadata structure for setting the hardware specific parameters, but the only issue is that, currently this structure is being used only from compress_set_gapless_metadata() in external/tinycompress/compress.c to set metadata specific to gapless audio. The parameters we are looking to set is not specific to gapless audio, but rather a few hardware/board specific parameters (for passing to our driver)
I think, the best way to use the same structure and have the new requirement fulfilled is that-
1. Define a new KEY inside the enum in external/kernel-headers/original/sound/compress_offload.h
enum { SNDRV_COMPRESS_ENCODER_PADDING = 1, SNDRV_COMPRESS_ENCODER_DELAY = 2, SNDRV_COMPRESS_HW_SPECIFIC_METADATA = 3, };
2. Have a new generic function (not specific to gapless audio) in external/tinycompress/compress.c to call driver IOCTL function, to set the H/W specific parameters-
int compress_set_metadata(struct compress *compress, struct compr_gapless_mdata *mdata) { struct snd_compr_metadata metadata; int version;
if (!is_compress_ready(compress)) return oops(compress, ENODEV, "device not ready");
version = get_compress_version(compress); if (version <= 0) return -1;
if (version < SNDRV_PROTOCOL_VERSION(0, 1, 1)) return oops(compress, ENXIO, "gapless apis not supported in kernel");
metadata.key = SNDRV_COMPRESS_HW_SPECIFIC_METADATA; metadata.value[0] = mdata->hw_specific_metadata; if (ioctl(compress->fd, SNDRV_COMPRESS_SET_METADATA, &metadata)) return oops(compress, errno, "can't set metadata for stream\n");
return 0; }
Let me know your comment on this.
Thanks, Banajit
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
On Sun, Mar 16, 2014 at 05:55:37AM -0000, Banajit Goswami wrote:
audio. The parameters we are looking to set is not specific to gapless audio, but rather a few hardware/board specific parameters (for passing to our driver)
Why not just use ALSA controls for this - that's the normal way of doing things like this, if the parameters aren't part of the stream data it's not obvious to me why they're being set on the stream. I'm also a bit concerned about the idea that applications would need to support another custom API for passing opaque blobs to drivers, we already have bytes controls. Can you give any examples of the sort of thing that would be configured here?
participants (4)
-
Banajit Goswami -
Mark Brown -
noman pouigt -
Vinod Koul