Messages in this thread | | | Subject | Re: [PATCH v2 8/9] ASoC: qdsp6-dai: add gapless support | From | Pierre-Louis Bossart <> | Date | Tue, 21 Jul 2020 14:53:51 -0500 |
| |
> case ASM_CLIENT_EVENT_CMD_EOS_DONE: > - prtd->state = Q6ASM_STREAM_STOPPED; > + spin_lock_irqsave(&prtd->lock, flags); > + if (prtd->notify_on_drain) { > + if (substream->partial_drain) { > + /**
why the kernel-doc style comment?
[...]
> -static int q6asm_dai_compr_set_params(struct snd_soc_component *component, > - struct snd_compr_stream *stream, > - struct snd_compr_params *params) > +static int __q6asm_dai_compr_set_codec_params(struct snd_soc_component *component, > + struct snd_compr_stream *stream, > + struct snd_codec *codec, > + int stream_id)
not sure I get why you added the __ prefix, does it have any semantic meaning?
> { > struct snd_compr_runtime *runtime = stream->runtime; > struct q6asm_dai_rtd *prtd = runtime->private_data; > - struct snd_soc_pcm_runtime *rtd = stream->private_data; > - int dir = stream->direction; > - struct q6asm_dai_data *pdata; > struct q6asm_flac_cfg flac_cfg; > struct q6asm_wma_cfg wma_cfg; > struct q6asm_alac_cfg alac_cfg; > @@ -667,53 +718,18 @@ static int q6asm_dai_compr_set_params(struct snd_soc_component *component, > struct snd_dec_alac *alac; > struct snd_dec_ape *ape; > > - codec_options = &(prtd->codec_param.codec.options); > - > - > - memcpy(&prtd->codec_param, params, sizeof(*params)); > - > - pdata = snd_soc_component_get_drvdata(component); > - if (!pdata) > - return -EINVAL; > - > - if (!prtd || !prtd->audio_client) { > - dev_err(dev, "private data null or audio client freed\n"); > - return -EINVAL; > - } > - > - prtd->periods = runtime->fragments; > - prtd->pcm_count = runtime->fragment_size; > - prtd->pcm_size = runtime->fragments * runtime->fragment_size; > - prtd->bits_per_sample = 16; > - if (dir == SND_COMPRESS_PLAYBACK) { > - ret = q6asm_open_write(prtd->audio_client, prtd->stream_id, > - params->codec.id, params->codec.profile, > - prtd->bits_per_sample, true); > - > - if (ret < 0) { > - dev_err(dev, "q6asm_open_write failed\n"); > - q6asm_audio_client_free(prtd->audio_client); > - prtd->audio_client = NULL; > - return ret; > - } > - } > + codec_options = &(prtd->codec.options); > > - prtd->session_id = q6asm_get_session_id(prtd->audio_client); > - ret = q6routing_stream_open(rtd->dai_link->id, LEGACY_PCM_MODE, > - prtd->session_id, dir); > - if (ret) { > - dev_err(dev, "Stream reg failed ret:%d\n", ret); > - return ret; > - } > + memcpy(&prtd->codec, codec, sizeof(*codec)); > > - switch (params->codec.id) { > + switch (codec->id) { > case SND_AUDIOCODEC_FLAC: > > memset(&flac_cfg, 0x0, sizeof(struct q6asm_flac_cfg)); > flac = &codec_options->flac_d; > > - flac_cfg.ch_cfg = params->codec.ch_in; > - flac_cfg.sample_rate = params->codec.sample_rate;
all these indirection changes could have gone in a earlier patch, this adds a lot of changes that make this patch long to review. Same comment for all formats
> + flac_cfg.ch_cfg = codec->ch_in; > + flac_cfg.sample_rate = codec->sample_rate; > flac_cfg.stream_info_present = 1; > flac_cfg.sample_size = flac->sample_size; > flac_cfg.min_blk_size = flac->min_blk_size;
[...]
> -static int q6asm_dai_compr_set_metadata(struct snd_compr_stream *stream, > +static int q6asm_dai_compr_set_metadata(struct snd_soc_component *component, > + struct snd_compr_stream *stream, > struct snd_compr_metadata *metadata) > { > struct snd_compr_runtime *runtime = stream->runtime; > @@ -884,6 +959,32 @@ static int q6asm_dai_compr_set_metadata(struct snd_compr_stream *stream, > break; > case SNDRV_COMPRESS_ENCODER_DELAY: > prtd->initial_samples_drop = metadata->value[0]; > + if (prtd->next_track_stream_id) { > + ret = q6asm_open_write(prtd->audio_client, > + prtd->next_track_stream_id, > + prtd->codec.id, > + prtd->codec.profile, > + prtd->bits_per_sample, > + true); > + if (ret < 0) { > + dev_err(component->dev, "q6asm_open_write failed\n"); > + return ret; > + } > + ret = __q6asm_dai_compr_set_codec_params(component, stream, > + &prtd->codec, > + prtd->next_track_stream_id); > + if (ret < 0) { > + dev_err(component->dev, "q6asm_open_write failed\n"); > + return ret; > + } > + > + ret = q6asm_stream_remove_initial_silence(prtd->audio_client, > + prtd->next_track_stream_id, > + prtd->initial_samples_drop); > + prtd->next_track_stream_id = 0; > + > + } > + > break; > default: > ret = -EINVAL; > @@ -917,6 +1018,14 @@ static int q6asm_dai_compr_trigger(struct snd_soc_component *component, > ret = q6asm_cmd_nowait(prtd->audio_client, prtd->stream_id, > CMD_PAUSE); > break; > + case SND_COMPR_TRIGGER_NEXT_TRACK: > + prtd->next_track = true; > + prtd->next_track_stream_id = (prtd->stream_id == 1 ? 2 : 1);
it's rather odd, the initialization above uses next_track_stream_id = 0?
> + break; > + case SND_COMPR_TRIGGER_DRAIN: > + case SND_COMPR_TRIGGER_PARTIAL_DRAIN: > + prtd->notify_on_drain = true; > + break; > default: > ret = -EINVAL; > break; > diff --git a/sound/soc/qcom/qdsp6/q6asm.h b/sound/soc/qcom/qdsp6/q6asm.h > index 69513ac1fa55..a8dddfeb28da 100644 > --- a/sound/soc/qcom/qdsp6/q6asm.h > +++ b/sound/soc/qcom/qdsp6/q6asm.h > @@ -34,6 +34,7 @@ enum { > #define MAX_SESSIONS 8 > #define NO_TIMESTAMP 0xFF00 > #define FORMAT_LINEAR_PCM 0x0000 > +#define ASM_LAST_BUFFER_FLAG BIT(30) > > struct q6asm_flac_cfg { > u32 sample_rate; >
| |