Re: [RFC 01/xx] ALSA: add snd_stream_is_playback/capture() macro

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 7/22/2024 7:58 AM, Kuninori Morimoto wrote:

Hi Amadeusz, Takashi

Perhaps you could use generics here, so you could have one caller for
both cases?

Something like:
#define snd_pcm_is_playback(x) _Generic((x),                   \
         int :         (x == SNDRV_PCM_STREAM_PLAYBACK), \
         struct snd_pcm_substream *substream * : (x->stream ==
SNDRV_PCM_STREAM_PLAYBACK))(x)
or just call the above functions in it?

Hmm... I couldn't compile above inline style.
I need to create function, and use it on _Generic().

And then, _Generic() is very picky for variable sytle.
This means I need to create function for "int" / "const int",
"unsigned int", "const unsigned int"

static inline int snd_pcm_stream_is_playback_(const int stream)
{
	return stream == SNDRV_PCM_STREAM_PLAYBACK;
}

static inline int snd_pcm_stream_is_playback(int stream)
{
	return stream == SNDRV_PCM_STREAM_PLAYBACK;
}

static inline int snd_pcm_stream_is_playback_u(const unsigned int stream)
{
	return stream == SNDRV_PCM_STREAM_PLAYBACK;
}

static inline int snd_pcm_stream_is_playbacku(unsigned int stream)
{
	return stream == SNDRV_PCM_STREAM_PLAYBACK;
}

static inline int snd_pcm_substream_is_playback_(const struct snd_pcm_substream *substream)
{
	return snd_pcm_stream_is_playback(substream->stream);
}

static inline int snd_pcm_substream_is_playback(struct snd_pcm_substream *substream)
{
	return snd_pcm_stream_is_playback(substream->stream);
}

#define snd_pcm_is_playback(x) _Generic((x), \
	const int : snd_pcm_stream_is_playback_, \
	      int : snd_pcm_stream_is_playback, \
	const unsigned int : snd_pcm_stream_is_playback_u, \
	      unsigned int : snd_pcm_stream_is_playbacku, \
	const struct snd_pcm_substream * : snd_pcm_substream_is_playback_, \
	      struct snd_pcm_substream * : snd_pcm_substream_is_playback)(x)

And I'm not sure how to handle "bit field" by _Generic.

	${LINUX}/sound/pci/ac97/ac97_pcm.c:153:13: note: in expansion of macro 'snd_pcm_is_playback'
	  153 |         if (snd_pcm_is_playback(pcm->stream))
	      |             ^~~~~~~~~~~~~~~~~~~
	${LINUX}/sound/pci/ac97/ac97_pcm.c: In function 'snd_ac97_pcm_assign':
	${LINUX}/include/sound/pcm.h:544:41: error: '_Generic' selector of type 'unsigned char:1' is not compatible with any association
	  544 | #define snd_pcm_is_playback(x) _Generic((x), \

Not using _Generic() is easy, "const int" version can handle everything
(= "int", "const int", "unsigned int", "const unsigned int", "short",
"const short", "bit field", etc).

If there is better _Generic() grammar, I can follow it.
If there wasn't, unfortunately let's give up this conversion...

Thank you for your help !!


My mistake in example, I've used function syntax, notice (x) at the end, if you remove it, it seems to build without need to call inline functions:

diff --git a/include/sound/pcm.h b/include/sound/pcm.h
index 210096f124eed..e914fea59445e 100644
--- a/include/sound/pcm.h
+++ b/include/sound/pcm.h
@@ -496,6 +496,10 @@ struct snd_pcm_substream {

 #define SUBSTREAM_BUSY(substream) ((substream)->ref_count > 0)

+#define snd_pcm_is_playback(x) _Generic((x), \ + int : (x == SNDRV_PCM_STREAM_PLAYBACK), \ + struct snd_pcm_substream * : (x->stream == SNDRV_PCM_STREAM_PLAYBACK))
+

 struct snd_pcm_str {
        int stream;                             /* stream (direction) */
diff --git a/sound/soc/intel/avs/pcm.c b/sound/soc/intel/avs/pcm.c
index 68aa8de2b0c4e..7e9f0ac6a5bc6 100644
--- a/sound/soc/intel/avs/pcm.c
+++ b/sound/soc/intel/avs/pcm.c
@@ -351,7 +351,7 @@ static int avs_dai_hda_be_hw_free(struct snd_pcm_substream *substream, struct sn
        data->path = NULL;

        /* clear link <-> stream mapping */
-       if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+       if (snd_pcm_is_playback(substream))
                snd_hdac_ext_bus_link_clear_stream_id(data->link,

hdac_stream(link_stream)->stream_tag);

@@ -383,7 +383,7 @@ static int avs_dai_hda_be_prepare(struct snd_pcm_substream *substream, struct sn
        snd_hdac_ext_stream_reset(link_stream);
        snd_hdac_ext_stream_setup(link_stream, format_val);

-       if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+       if (snd_pcm_is_playback(substream))
                snd_hdac_ext_bus_link_set_stream_id(data->link,

hdac_stream(link_stream)->stream_tag);


I've test compiled the above fine.

As for ac97, seems like _Generic is impossible on bitfields, so perhaps just move it out of bitfield, to int?

Thanks,
Amadeusz



[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux