diff mbox series

Revert "ALSA: emu10k1: fix synthesizer sample playback position and caching"

Message ID 20240401145805.528794-1-oswald.buddenhagen@gmx.de
State Accepted
Commit 03f56ed4ead162551ac596c9e3076ff01f1c5836
Headers show
Series Revert "ALSA: emu10k1: fix synthesizer sample playback position and caching" | expand

Commit Message

Oswald Buddenhagen April 1, 2024, 2:58 p.m. UTC
As already anticipated in the original commit, playback was broken for
very short samples. I just didn't expect it to be an actual problem,
because we're talking about less than 1.5 milliseconds here. But clearly
such wavetable samples do actually exist.

The problem was that for such short samples we'd set the current
position beyond the end of the loop, so we'd run off the end of the
sample and play garbage.
This is a bigger (more audible) problem than the original one, which was
that we'd start playback with garbage (whatever was still in the cache),
which would be mostly masked by the note's attack phase.

So revert to the old behavior for now. We'll subsequently fix it
properly with a bigger patch series.
Note that this isn't a full revert - the dead code is not re-introduced,
because that would be silly.

Fixes: df335e9a8b (ALSA: emu10k1: fix synthesizer sample playback position and caching, 2023-05-18)
Link: https://bugzilla.kernel.org/show_bug.cgi?id=218625
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
---
 sound/pci/emu10k1/emu10k1_callback.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

--
2.42.0.419.g70bf8a5751

Comments

Takashi Iwai April 1, 2024, 5:02 p.m. UTC | #1
On Mon, 01 Apr 2024 16:58:05 +0200,
Oswald Buddenhagen wrote:
> 
> As already anticipated in the original commit, playback was broken for
> very short samples. I just didn't expect it to be an actual problem,
> because we're talking about less than 1.5 milliseconds here. But clearly
> such wavetable samples do actually exist.
> 
> The problem was that for such short samples we'd set the current
> position beyond the end of the loop, so we'd run off the end of the
> sample and play garbage.
> This is a bigger (more audible) problem than the original one, which was
> that we'd start playback with garbage (whatever was still in the cache),
> which would be mostly masked by the note's attack phase.
> 
> So revert to the old behavior for now. We'll subsequently fix it
> properly with a bigger patch series.
> Note that this isn't a full revert - the dead code is not re-introduced,
> because that would be silly.
> 
> Fixes: df335e9a8b (ALSA: emu10k1: fix synthesizer sample playback position and caching, 2023-05-18)
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=218625
> Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>

Thanks, applied now.


Takashi
diff mbox series

Patch

diff --git a/sound/pci/emu10k1/emu10k1_callback.c b/sound/pci/emu10k1/emu10k1_callback.c
index d36234b88fb4..941bfbf812ed 100644
--- a/sound/pci/emu10k1/emu10k1_callback.c
+++ b/sound/pci/emu10k1/emu10k1_callback.c
@@ -255,7 +255,7 @@  lookup_voices(struct snd_emux *emu, struct snd_emu10k1 *hw,
 		/* check if sample is finished playing (non-looping only) */
 		if (bp != best + V_OFF && bp != best + V_FREE &&
 		    (vp->reg.sample_mode & SNDRV_SFNT_SAMPLE_SINGLESHOT)) {
-			val = snd_emu10k1_ptr_read(hw, CCCA_CURRADDR, vp->ch) - 64;
+			val = snd_emu10k1_ptr_read(hw, CCCA_CURRADDR, vp->ch);
 			if (val >= vp->reg.loopstart)
 				bp = best + V_OFF;
 		}
@@ -362,7 +362,7 @@  start_voice(struct snd_emux_voice *vp)

 	map = (hw->silent_page.addr << hw->address_mode) | (hw->address_mode ? MAP_PTI_MASK1 : MAP_PTI_MASK0);

-	addr = vp->reg.start + 64;
+	addr = vp->reg.start;
 	temp = vp->reg.parm.filterQ;
 	ccca = (temp << 28) | addr;
 	if (vp->apitch < 0xe400)
@@ -430,9 +430,6 @@  start_voice(struct snd_emux_voice *vp)
 		/* Q & current address (Q 4bit value, MSB) */
 		CCCA, ccca,

-		/* cache */
-		CCR, REG_VAL_PUT(CCR_CACHEINVALIDSIZE, 64),
-
 		/* reset volume */
 		VTFT, vtarget | vp->ftarget,
 		CVCF, vtarget | CVCF_CURRENTFILTER_MASK,