Opened 8 years ago
Closed 15 months ago
#6957 closed defect (outdated)
BACKENDS: ARM - ARM ASM sound code causes distorted audio on 32 bit armv6
| Reported by: | SF/gaula92 | Owned by: | lephilousophe |
|---|---|---|---|
| Priority: | normal | Component: | Ports |
| Version: | 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 (11)
comment:1 by , 8 years ago
comment:2 by , 8 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 , 8 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 , 8 years ago
| Component: | Engine: ZVision → Ports |
|---|---|
| Game: | Zork Grand Inquisitor |
comment:5 by , 6 years ago
| Keywords: | code-removal added |
|---|---|
| Priority: | normal → blocker |
comment:6 by , 6 years ago
| Owner: | set to |
|---|
comment:7 by , 6 years ago
| Priority: | blocker → high |
|---|
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 , 6 years ago
| Owner: | removed |
|---|
(The change was made in ddbfb8518b6fdfe58e3574421fd6b9243ad2c761.)
comment:9 by , 4 years ago
| Summary: | AUDIO: ARM ASM sound code causes distorted audio on 32 bit armv6 → BACKENDS: ARM - ARM ASM sound code causes distorted audio on 32 bit armv6 |
|---|
comment:10 by , 4 years ago
| Priority: | high → normal |
|---|
comment:11 by , 15 months ago
| Owner: | set to |
|---|---|
| Resolution: | → outdated |
| Status: | new → closed |
The code has been completely removed in 99e5de932e1842f1ac48e49f0f7d0a717b58ed8b.
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?