Opened 4 years ago

Last modified 2 years ago

#6957 new defect

AUDIO: ARM ASM sound code causes distorted audio on 32 bit armv6

Reported by: SF/gaula92 Owned by:
Priority: high Component: Ports
Keywords: code-removal Cc:
Game:

Description

AUDIO: ARM ASM sound code causes distorted audio on 32 bit armv6. Most games are affected: every SCUMM game with voices, dreamweb, agos games...
Disabling ASM ARM sound makes sound correct and clear again.
It's supposed to be related to ARM ASM code recently supporting a wider range of input formats: it was woking before these changes.

Ticket imported from: bugs/6957.

Change History (8)

comment:1 by digitall, 4 years ago

Have taken a further look at the ARM ASM code here and it is likely that there are latent issues related to the change from 16:16 to 17:15 in the fractional representation i.e. the ASM will need far more extensive work to be 17:15.

Since this may result in actually worse performance anyway than just letting the ARM C compiler do it's job, it may be best to disable the ARM ASM in the audio system by default and let this be enabled and fixed for any ports which truely need this.

@fuzzie , @lordhoto: Thoughts?

comment:2 by digitall, 4 years ago

The best way to fix this code would probably be to add code which compiles both of the rate conversions together and compares the output i.e. on ARM, it compiles the C version of the rate convertor and the ASM version, using the C version output, but comparing the ASM output and warning if it doesn't match and what values were passed in... this will allow rework and debugging until the two match. Working on this is left as an exercise for the reader... We are Open Source after all :)

comment:3 by lordhoto, 4 years ago

I disabled the code now with 3fab9056296fbf491372f66f7fbb23d6312ad2ad. After looking a bit into it with digitall it seems not easy to fix it. If anybody with ARM assembly knowledge wants to look at it, that would be most appreciated.

I will keep this bug report open to remind people that there are actual issues with this code.

My personal opinion is: We might just want to drop this code (and thus an additional source of bugs) if there is no (strict) need for speeding up this code path for ARM targets.

comment:4 by wjp, 3 years ago

Component: Engine: ZVisionPorts
Game: Zork Grand Inquisitor

comment:5 by csnover, 2 years ago

Keywords: code-removal added
Priority: normalblocker

comment:6 by csnover, 2 years ago

Owner: set to csnover

comment:7 by csnover, 2 years ago

Priority: blockerhigh

The ARM assembly will now only ever be enabled for specific legacy device hosts, so I am removing these tickets from blocking the release. The broken parts of the ARM code still need to be fixed and re-enabled for these hosts or else finally removed by the next-next release.

comment:8 by csnover, 2 years ago

Owner: csnover removed

(The change was made in ddbfb8518b6fdfe58e3574421fd6b9243ad2c761.)

Note: See TracTickets for help on using tickets.