diff mbox series

[1/9] ALSA: emu10k1: pass frame instead of byte addresses

Message ID 20230517174256.3657060-1-oswald.buddenhagen@gmx.de
State Superseded
Headers show
Series [1/9] ALSA: emu10k1: pass frame instead of byte addresses | expand

Commit Message

Oswald Buddenhagen May 17, 2023, 5:42 p.m. UTC 
... to snd_emu10k1_pcm_init_voice(). This makes the code arguably less
convoluted.

Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
---
 sound/pci/emu10k1/emupcm.c | 31 +++++++++----------------------
 1 file changed, 9 insertions(+), 22 deletions(-)

Comments

Takashi Iwai May 18, 2023, 5:32 a.m. UTC | #1 
On Wed, 17 May 2023 19:42:48 +0200,
Oswald Buddenhagen wrote:
> 
> ... to snd_emu10k1_pcm_init_voice(). This makes the code arguably less
> convoluted.
> 
> Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>

Now applied all patches except for patch#6 ("ALSA: emu10k1: fix PCM
playback buffer size constraints").


thanks,

Takashi
Oswald Buddenhagen May 18, 2023, 7:57 a.m. UTC | #2 
On Wed, May 17, 2023 at 10:25:00PM +0200, Takashi Iwai wrote:
>On Wed, 17 May 2023 19:42:53 +0200,
>Oswald Buddenhagen wrote:
>> 
>> The period_bytes_min parameter made no sense at all, as it didn't
>> correlate with any hardware limitation.
>
>Does the device really work with less than 64 bytes period size?
>I meant not in theory but in practice.
>
somewhat predictably, not.

>Without any value set,
>dumb applications may try to set 4 bytes for period size,
>
the "try to" is key here. it will fail, because the frame-based 
constraint will prevent it from doing so.

alsa's constraint system is really quite impressive, probably the 
technically most interesting part of the whole. :-)

regards
Takashi Iwai May 18, 2023, 8:17 a.m. UTC | #3 
On Thu, 18 May 2023 09:57:51 +0200,
Oswald Buddenhagen wrote:
> 
> On Wed, May 17, 2023 at 10:25:00PM +0200, Takashi Iwai wrote:
> > On Wed, 17 May 2023 19:42:53 +0200,
> > Oswald Buddenhagen wrote:
> >> 
> >> The period_bytes_min parameter made no sense at all, as it didn't
> >> correlate with any hardware limitation.
> > 
> > Does the device really work with less than 64 bytes period size?
> > I meant not in theory but in practice.
> > 
> somewhat predictably, not.
> 
> > Without any value set,
> > dumb applications may try to set 4 bytes for period size,
> > 
> the "try to" is key here. it will fail, because the frame-based
> constraint will prevent it from doing so.

Ah OK, this should be commented that the lower bound is set in a
different way.


Takashi
diff mbox series

Patch

diff --git a/sound/pci/emu10k1/emupcm.c b/sound/pci/emu10k1/emupcm.c
index d669f93d8930..9f151a0a7756 100644
--- a/sound/pci/emu10k1/emupcm.c
+++ b/sound/pci/emu10k1/emupcm.c
@@ -270,15 +270,6 @@  static void snd_emu10k1_pcm_init_voice(struct snd_emu10k1 *emu,
 	stereo = runtime->channels == 2;
 	w_16 = snd_pcm_format_width(runtime->format) == 16;
 
-	if (!extra && stereo) {
-		start_addr >>= 1;
-		end_addr >>= 1;
-	}
-	if (w_16) {
-		start_addr >>= 1;
-		end_addr >>= 1;
-	}
-
 	spin_lock_irqsave(&emu->reg_lock, flags);
 
 	/* volume parameters */
@@ -424,19 +415,16 @@  static int snd_emu10k1_playback_prepare(struct snd_pcm_substream *substream)
 	struct snd_emu10k1 *emu = snd_pcm_substream_chip(substream);
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	struct snd_emu10k1_pcm *epcm = runtime->private_data;
+	bool w_16 = snd_pcm_format_width(runtime->format) == 16;
+	bool stereo = runtime->channels == 2;
 	unsigned int start_addr, end_addr;
 
-	start_addr = epcm->start_addr;
-	end_addr = snd_pcm_lib_period_bytes(substream);
-	if (runtime->channels == 2) {
-		start_addr >>= 1;
-		end_addr >>= 1;
-	}
-	end_addr += start_addr;
+	start_addr = epcm->start_addr >> w_16;
+	end_addr = start_addr + runtime->period_size;
 	snd_emu10k1_pcm_init_voice(emu, 1, 1, epcm->extra,
 				   start_addr, end_addr, NULL);
-	start_addr = epcm->start_addr;
-	end_addr = epcm->start_addr + snd_pcm_lib_buffer_bytes(substream);
+	start_addr >>= stereo;
+	end_addr = start_addr + runtime->buffer_size;
 	snd_emu10k1_pcm_init_voice(emu, 1, 0, epcm->voices[0],
 				   start_addr, end_addr,
 				   &emu->pcm_mixer[substream->number]);
@@ -452,14 +440,13 @@  static int snd_emu10k1_efx_playback_prepare(struct snd_pcm_substream *substream)
 	struct snd_emu10k1 *emu = snd_pcm_substream_chip(substream);
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	struct snd_emu10k1_pcm *epcm = runtime->private_data;
-	unsigned int start_addr, end_addr;
+	unsigned int start_addr;
 	unsigned int channel_size;
 	int i;
 
-	start_addr = epcm->start_addr;
-	end_addr = epcm->start_addr + snd_pcm_lib_buffer_bytes(substream);
+	start_addr = epcm->start_addr >> 1;  // 16-bit voices
 
-	channel_size = ( end_addr - start_addr ) / NUM_EFX_PLAYBACK;
+	channel_size = runtime->buffer_size;
 
 	snd_emu10k1_pcm_init_voice(emu, 1, 1, epcm->extra,
 				   start_addr, start_addr + (channel_size / 2), NULL);