[alsa-devel] [PATCH 1/7] ALSA: hda: Allow for compress stream to hdac_ext_stream assignment

[Cezary Rojewski]

On 2019-12-17 11:19, Takashi Iwai wrote:
> On Tue, 17 Dec 2019 10:58:45 +0100,
> Cezary Rojewski wrote:
>>
>> Currently only PCM streams can enlist hdac_stream for their data
>> transfer. Add cstream field to hdac_ext_stream to expose possibility of
>> compress stream assignment in place of PCM one.
>> Limited to HOST-type only.
>>
>> Rather than copying entire hdac_ext_host_stream_assign, declare separate
>> PCM and compress wrappers and reuse it for both cases.
>>
>> Signed-off-by: Cezary Rojewski <cezary.rojewski at intel.com>
>> ---
>>   include/sound/hdaudio.h         |  1 +
>>   include/sound/hdaudio_ext.h     |  2 ++
>>   sound/hda/ext/hdac_ext_stream.c | 46 +++++++++++++++++++++++++++++----
>>   3 files changed, 44 insertions(+), 5 deletions(-)
>>
>> diff --git a/include/sound/hdaudio.h b/include/sound/hdaudio.h
>> index e05b95e83d5a..9a8bf1eb7d69 100644
>> --- a/include/sound/hdaudio.h
>> +++ b/include/sound/hdaudio.h
>> @@ -481,6 +481,7 @@ struct hdac_stream {
>>   	struct snd_pcm_substream *substream;	/* assigned substream,
>>   						 * set in PCM open
>>   						 */
>> +	struct snd_compr_stream *cstream;
>>   	unsigned int format_val;	/* format value to be set in the
>>   					 * controller and the codec
>>   					 */
> 
> One might use union for pointing to either PCM or compr stream and
> identify the type with some flag.
> 
> 	struct hdac_stream {
> 		union {
> 			struct snd_pcm_substream *substream;
> 			struct snd_compr_stream *cstream;
> 		};
> 		bool is_compr;
> 		....
> 
> But, I'm not advocating for this.  Although this makes the stream
> assignment more handy, it might lead to refer to a wrong object if you
> don't check the flag properly, too.  It really depends on the code.
> 

I'm happy with both - existing - and your variant. In essence, this 
causes simply: s/if (hstream->cstream)/if (hstream->is_compr)/g to occur.

In general, I'm strong supporter of a "PCM-compr marriage" idea - both 
being combined in sense of having similar base in the future so one 
could make use of "snd_base_stream", checkout the is_compr field and 
cast into actual type (_pcm_ -or- _compr_) via container_of macro.

This is more of a wish or song of the future for now, though. Compress 
and PCM ops streamlining is not within the scope of probes and requires 
much more work : )