Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#6672 closed defect (fixed)

NEVERHOOD: Too many simultaneous sound effects in "memory" puzzle

Reported by: eriktorbjorn Owned by: bluegr
Priority: normal Component: Engine: Neverhood
Keywords: Cc:
Game: The Neverhood

Description

When failing at the "memory" puzzle, it (annoyingly) reverts all squares you've discovered so far. When it does so, I guess it must be playing a sound effect for each and every one of them, because after a while I start getting "WARNING: MixerImpl::out of mixer slots!"

Now, this is probably quite harmless. It's unlikely that it's leaking sound handles or anything like that. I'm just worried that if it could actually play all the sounds, the result would get so loud that it gets distorted. But if we want to limit it, it doesn't feel very elegant to rely on the mixer to enforce that limit.

(Oh, about the name of the save game? I'm currently testing The Neverhood in Valgrind. It's tedious.)

Ticket imported from: bugs/6672.

Attachments (1)

neverhood-win.007 (21.4 KB) - added by eriktorbjorn 5 years ago.

Download all attachments as: .zip

Change History (9)

Changed 5 years ago by eriktorbjorn

Attachment: neverhood-win.007 added

comment:1 Changed 5 years ago by digitall

eriktorbjorn: How does this behave in the original interpreter? i.e.
1. Are sound effects played for all the reverted squares?
2. Are the sound effects played in sequence waiting for the previous one to finish first? i.e. do we have a sequencing issue where the engine is not waiting for the previous effect to finish before queueing up the next one...

comment:2 Changed 5 years ago by eriktorbjorn

Since the sounds are played all at once I don't know if the original plays one sound for each reverted square. It sounds like a single sound effect, but it gets louder when there are more squares to revert. What I can say is that ScummVM and the original behave so similar that I'm not sure I could tell them apart.

So it's not that it behaves wrong, it's just that it seems a bit ugly to me that it spams the mixer with sounds like that.

I believe the sounds are triggered by Scene1405::update(), where it iterates over all the tiles.

comment:3 Changed 5 years ago by digitall

eriktorbjorn: As we do want this to sound as per the original, I would suggest that limiting or changing the maximum number of sounds played is not desirable.

However, I think we can stop the spamming and limit such that the mixer implementation does not run out of slots without breaking this.

The mixer interface has a method to see if a sound id is still playing:
https://github.com/scummvm/scummvm/blob/master/audio/mixer.h#L171

I suggest that the engine code be modified so that it only sends to mixer::Playstream a maximum number of simulatenous sounds defined by a constant in the engine, Neverhood::MAX_SIMULTANEOUS_SOUND.

The rest are delayed and as a previous sound id finishes playing the next one "queued" is sent to PlayStream...

The rest will be to stretch out the sound playback slightly, but probably not noticeably without causing the mixer implementation to run out of slots or rely on it for the limitation.

Thoughts?

comment:4 Changed 5 years ago by eriktorbjorn

The rest are delayed and as a previous sound id finishes playing
the next one "queued" is sent to PlayStream...

Maybe I'm misunderstanding, but that to me sounds like it would change the behaviour of ScummVM compared to the original. As I said, when you play it it will sound like one single sound effect, but since it's actually several sound effects it will sound louder depending on how many are playing.

But if you queue them, wouldn't you get a situation where it first plays 16 simultaneous sounds (because NUM_CHANNELS in mixer.h is 16) at once, then another 16, and so on until it's exhausted them all? You'd get up to three distinct sounds, instead of the one you hear in the original, wouldn't you?

comment:5 Changed 5 years ago by digitall

Firstly, I am suggesting that we slightly change the behaviour compared to the original to avoid running out of mixer slots.

However, that isn't quite what I intended. I meant as follows:
1. You send the first sound to the mixer and add it's id to a list of sounds playing.
2. Wait a short delay
3. You send the next sound to the mixer and add it's id to a list of sounds playing.
4. You repeat steps 2 and 3 until either you run out of sounds to send or you hit MAX_SIMULTANEOUS SOUNDS.
5. If you hit MAX_SIMULTANEOUS SOUNDS, you check to see if the first sound has finished playing. If you remove it from the currently playing list and then repeat from 4.

The result should be with the right value of delay to slightly stretch out the sounds, but not noticeably without breaking the mixer limit.

The other option is to premix the sound data from the files within the engine into a RawAudioStream based on a allocated memory buffer and then send the result to the mixer as a single sound.

comment:6 Changed 5 years ago by bluegr

That sounds way over-engineered. A single sound effect would suffice here, since it's a "click" sound anyway, so we do not really need simultaneous clicking sounds.

comment:7 Changed 5 years ago by bluegr

Owner: set to bluegr
Resolution: fixed
Status: newclosed

comment:8 Changed 5 years ago by bluegr

Fixed with commits 4207fbd2d6 (master) / a3ec14eb76 (branch-1-7)

Note: See TracTickets for help on using tickets.