[RFC,0/3] ALSA: compress: Add support for FLAC

Message

Vinod Koul Nov. 15, 2019, 10:27 a.m. UTC

The current design of sending codec parameters assumes that decoders
will have parsers so they can parse the encoded stream for parameters
and configure the decoder.

But this assumption may not be universally true and we know some DSPs
which do not contain the parsers so additional parameters are required
to be passed.

So add these parameters starting with FLAC decoder. The size of
snd_codec_options is still 120 bytes after this change (due to this
being a union)

I think we should also bump the (minor) version if this proposal is
acceptable so the userspace can check and populate flac specific structure.

Along, with the core header change, patches are added to support FLAC
in Qualcomm drivers. This was tested on 96boards db845c

Srinivas Kandagatla (1):
  ASoC: qcom: q6asm: add support to flac config

Vinod Koul (2):
  ALSA: compress: add flac decoder params
  ASoC: qcom: q6asm-dai: add support to flac decoder

 include/uapi/sound/compress_params.h | 10 +++++
 sound/soc/qcom/qdsp6/q6asm-dai.c     | 35 +++++++++++++++++-
 sound/soc/qcom/qdsp6/q6asm.c         | 55 ++++++++++++++++++++++++++++
 sound/soc/qcom/qdsp6/q6asm.h         | 15 ++++++++
 4 files changed, 114 insertions(+), 1 deletion(-)

Comments

Takashi Iwai Nov. 15, 2019, 1:21 p.m. UTC | #1

On Fri, 15 Nov 2019 11:27:02 +0100,
Vinod Koul wrote:
> 
> The current design of sending codec parameters assumes that decoders
> will have parsers so they can parse the encoded stream for parameters
> and configure the decoder.
> 
> But this assumption may not be universally true and we know some DSPs
> which do not contain the parsers so additional parameters are required
> to be passed.
> 
> So add these parameters starting with FLAC decoder. The size of
> snd_codec_options is still 120 bytes after this change (due to this
> being a union)
> 
> I think we should also bump the (minor) version if this proposal is
> acceptable so the userspace can check and populate flac specific structure.
> 
> Along, with the core header change, patches are added to support FLAC
> in Qualcomm drivers. This was tested on 96boards db845c
> 
> Srinivas Kandagatla (1):
>   ASoC: qcom: q6asm: add support to flac config
> 
> Vinod Koul (2):
>   ALSA: compress: add flac decoder params
>   ASoC: qcom: q6asm-dai: add support to flac decoder

Feel free to take my ACK for ALSA core part:
  Acked-by: Takashi Iwai <[e-mail]>


thanks,

Takashi

Vinod Koul Nov. 15, 2019, 1:36 p.m. UTC | #2

On 15-11-19, 14:21, Takashi Iwai wrote:
> On Fri, 15 Nov 2019 11:27:02 +0100,
> Vinod Koul wrote:
> > 
> > The current design of sending codec parameters assumes that decoders
> > will have parsers so they can parse the encoded stream for parameters
> > and configure the decoder.
> > 
> > But this assumption may not be universally true and we know some DSPs
> > which do not contain the parsers so additional parameters are required
> > to be passed.
> > 
> > So add these parameters starting with FLAC decoder. The size of
> > snd_codec_options is still 120 bytes after this change (due to this
> > being a union)
> > 
> > I think we should also bump the (minor) version if this proposal is
> > acceptable so the userspace can check and populate flac specific structure.
> > 
> > Along, with the core header change, patches are added to support FLAC
> > in Qualcomm drivers. This was tested on 96boards db845c
> > 
> > Srinivas Kandagatla (1):
> >   ASoC: qcom: q6asm: add support to flac config
> > 
> > Vinod Koul (2):
> >   ALSA: compress: add flac decoder params
> >   ASoC: qcom: q6asm-dai: add support to flac decoder
> 
> Feel free to take my ACK for ALSA core part:
>   Acked-by: Takashi Iwai <[e-mail]>

Thanks Takashi, should we bump the version for the header to check for.
Btw I plan to add other decoders required as well. I have mp3 working
without any additional params but rest need additional info

Thanks

Pierre-Louis Bossart Nov. 15, 2019, 2:55 p.m. UTC | #3

On 11/15/19 4:27 AM, Vinod Koul wrote:
> The current design of sending codec parameters assumes that decoders
> will have parsers so they can parse the encoded stream for parameters
> and configure the decoder.

that's not quite correct. It's rather than there was no need so far for 
existing implementations to have parameters on decode, this was never a 
limitation of the design, see e.g. the comments below:

/* AAC modes are required for encoders and decoders */

/*
  * IEC modes are mandatory for decoders. Format autodetection
  * will only happen on the DSP side with mode 0. The PCM mode should
  * not be used, the PCM codec should be used instead.
  */

Vinod Koul Nov. 19, 2019, 4:15 a.m. UTC | #4

On 15-11-19, 08:55, Pierre-Louis Bossart wrote:
> 
> 
> On 11/15/19 4:27 AM, Vinod Koul wrote:
> > The current design of sending codec parameters assumes that decoders
> > will have parsers so they can parse the encoded stream for parameters
> > and configure the decoder.
> 
> that's not quite correct. It's rather than there was no need so far for
> existing implementations to have parameters on decode, this was never a
> limitation of the design, see e.g. the comments below:

You might be correct here as this is implementation based and in this
case looks like decoders also need the additional params