Opened 9 years ago

Closed 9 years ago

#4795 closed defect (fixed)

HE SAM1: Music in kitchen slightly off (regression)

Reported by: raziel- Owned by: lordhoto
Priority: high Component: Engine: SCUMM
Keywords: Cc:
Game: Pajama Sam 1

Description

ScummVM 1.1.0svn48158 (Mar 2 2010 06:58:51)
Features compiled in: Vorbis FLAC MP3 RGB zLib

When in the kitchen all the sounds and music are slightly off sync.
Every music piece loop after a while but the entry point is wrong and that can be heard.

Worst example is when one lets a kitchen tool sing.
First of all it takes quite some time before it starts (but maybe that is intended to wait for sync of the background music?), then, after the song is finished, the looping background music will have a piece of the singing voice of that last tool in it.
Also the whole background music seems "cut" when looping

Pajama Sam: No need to hide when it's Dark outside (Updated (English/US))

AmigaOS4
gcc version 4.2.4 (adtools build 20090118)

Ticket imported from: #2961787. Ticket imported from: bugs/4795.

Attachments (1)

pajama.sg1 (23.2 KB) - added by raziel- 9 years ago.
Standin in the kitchen

Download all attachments as: .zip

Change History (23)

Changed 9 years ago by raziel-

Attachment: pajama.sg1 added

Standin in the kitchen

comment:1 Changed 9 years ago by Kirben

This is a regression, caused by revision 47716. When Audio::makeRawMemoryStream was replaced by Audio::makeRawStream.

I expect the problem is related to HE games altering the sound data as it is been played, via createSound() function.

comment:2 Changed 9 years ago by Kirben

Owner: set to lordhoto
Priority: normalhigh

comment:3 Changed 9 years ago by lordhoto

Just for the logs: The problem should be that the new RawStream code uses caching of the data. Thus when the engine modifies the sound buffer while it is being played (like HE does via createSound as I got told by Kriben) that might lead to parts of the old buffer still being played, since it's currently in the preread buffer.

There are two possible solutions to this:

1) Modify the RawStream code to use a custom cache size and/or disabling caching completly.

2) Bring back the old LinearAudioStream code.

I'm actually all for the former, since there's lots of code duplication in the second approach.

comment:4 Changed 9 years ago by Kirben

Summary: HE SAM1: Music in kitchen slightly offHE SAM1: Music in kitchen slightly off (regression)

comment:5 Changed 9 years ago by lordhoto

This should be fixed in branch-1-1-0 with r48506 by reintroducing the old RawMemoryStream code.

A proper fix for the trunk will follow.

comment:6 Changed 9 years ago by raziel-

Judging from the comment in CIA you wanted me to retest? ;-)

Unfortunately this doesn't fix the problem. It got "slightly" better, but there is still a pause and after a tool has sang there is a little part of that song at the beginning of the kitchen background music

comment:7 Changed 9 years ago by lordhoto

Sure you are using a latest branch-1-1-0 checkout? The "fix" is not included in the trunk as stated in my last comment.

comment:8 Changed 9 years ago by raziel-

Judging from the comment in CIA you wanted me to retest? ;-)

Unfortunately this doesn't fix the problem. It got "slightly" better, but there is still a pause and after a tool has sang there is a little part of that song at the beginning of the kitchen background music

comment:9 Changed 9 years ago by raziel-

Whoops, sorry, mixed them up

facepalm

comment:10 Changed 9 years ago by lordhoto

Does that mean it is fixed in the branch now?

comment:11 Changed 9 years ago by raziel-

Fresh build of branch 1.1.0

Unfortunately not.
It's by far better than before, now the background music from the kitchen plays completely without any pauses, but when it's finished there is still this pause and a little bit of the kitchen tool song and only after that the background loops

comment:12 Changed 9 years ago by lordhoto

Could you try to check whether that happens with ScummVM 1.0.0 too?

comment:13 Changed 9 years ago by raziel-

Done.

In 1.1.0 it's perfect and flawless
Actually now that i heard it right, i have to add that the background music only loops the first loop.
In 1.1.0 there are many changing piano cues in the background played when a new loop is introduced, in trunk these cues are never played, only the first loop is played

comment:14 Changed 9 years ago by lordhoto

Guess you are talking about 1.0.0 in your statement and as I said before trunk is not covered by my attempt to fix this only the branch.

Anyway now the sound streaming code should be fine again, i.e. the old behavior should have been restored.

About that loop bit, do you mean in 1.0.0 the background music only plays one time and in branch-1-1-0 it loops?

comment:15 Changed 9 years ago by raziel-

Typo in my last comment, sorry

The other way round
It's perfect in 1.0.0

In 1.0.0 the background plays and is looping with much more differing piano cues on every loop
In 1.1.0 it's only looping once, looping the same loop over and over again without the differing piano cues

Sorry for the noise :-(

comment:16 Changed 9 years ago by lordhoto

I'm out of ideas then. Maybe Kirben should look into this one again.

comment:17 Changed 9 years ago by Kirben

No idea, the music seems fine again, at least under Windows XP.

The background music chosen is randomized, when songs aren't been played. So don't always expect the background music to match exactly.

If you set debug level 1 you can see what background music is been added to the current music (ie queing pad 7).

comment:18 Changed 9 years ago by raziel-

OK, i did a complete and fresh rebuild of branch 1.1.0 and the error is gone

My bad :-(

Sorry again for the noise

comment:19 Changed 9 years ago by fingolfin

So as I understand it, this issue is fixed in the 1.1.x branch but still present in trunk?

LordHoto, any plans to work on this? You seem to want to allow tweaking the caching -- as I understand it, if we play a memory stream, then no caching at all is needed, which would make this issue go away, correct?

comment:20 Changed 9 years ago by lordhoto

Yes, the idea is either to have two code paths as with 1.1.x or we just document that the reading should be fast, i.e. for slow disk access the caller should wrap it into a BufferedSeekableReadStream or the like.

Of course one might still consider to change how HE handles sounds, but that would be beyond my interest/time/skills.

Anyway I still plan to do that, when I have some more time. Of course in case someone else wants to take care of it meanwhile, that would be fine with me too.

comment:21 Changed 9 years ago by lordhoto

Ok I committed a patch for trunk now. Unlike the fix for branch-1-1-x I did not restore the old code there though, instead I removed the pre-buffering in RawStream. That should fix the issue.

raziel_: Please try again and in case the issue persists just reopen this tracker item please.

comment:22 Changed 9 years ago by lordhoto

Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.