Opened 3 years ago

Last modified 16 months 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 Changed 3 years ago by digitall

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 Changed 3 years ago by digitall

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 Changed 3 years ago by lordhoto

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 Changed 3 years ago by wjp

Component: Engine: ZVisionPorts
Game: Zork Grand Inquisitor

comment:5 Changed 17 months ago by csnover

Keywords: code-removal added
Priority: normalblocker

comment:6 Changed 17 months ago by csnover

Owner: set to csnover

comment:7 Changed 16 months ago by csnover

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 Changed 16 months ago by csnover

Owner: csnover deleted

(The change was made in ddbfb8518b6fdfe58e3574421fd6b9243ad2c761.)

Note: See TracTickets for help on using tickets.