Hi, Currently liba52 is broken for ARM processors which require strict alignment for accessing memory (ARM9, XScale, ...). AC3 audio playback just produces noise. Interestingly enough, the problem was introduced in 2003 (SVN revision 10314), and it was supposedly a fix for ARM: http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/2003-June/019377.html I wonder if it was a fix that turned compilation error into runtime failure ;) Anyway, it should be mentioned that modern ARM cores (such as ARM11) support unaligned memory access natively. Older cores can emulate unaligned memory accesses by handling alignment faults in the kernel, but this is very slow. PS. liba52 uses floating point math which is very slow on old ARM cores without hardware FPU, so using liba52 on such old cores is not very practical anyway. -- Best regards, Siarhei Siamashka
On Monday 28 April 2008, Siarhei Siamashka wrote:
Hi,
Currently liba52 is broken for ARM processors which require strict alignment for accessing memory (ARM9, XScale, ...). AC3 audio playback just produces noise.
Interestingly enough, the problem was introduced in 2003 (SVN revision 10314), and it was supposedly a fix for ARM: http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/2003-June/019377.html
I wonder if it was a fix that turned compilation error into runtime failure ;)
Anyway, it should be mentioned that modern ARM cores (such as ARM11) support unaligned memory access natively. Older cores can emulate unaligned memory accesses by handling alignment faults in the kernel, but this is very slow.
PS. liba52 uses floating point math which is very slow on old ARM cores without hardware FPU, so using liba52 on such old cores is not very practical anyway.
Ping. Can anybody consider reverting r10314 change? Or alternatively you can try to use the attached patch. Anyway, some kind of action needs to be taken to fix this breakage. Also I strongly suspect that r10490 was a proper fix for that ARM compilation issue with gcc 2.95 that r10314 tried to address but introduced a bigger problem instead. ------------------------------------------------------------------------ r10490 | alex | 2003-07-28 02:02:58 +0300 (Mon, 28 Jul 2003) | 2 lines Changed paths: M /trunk/liba52/bitstream.h Changed swab32 from macro to inline function, this fixes compilation on alpha (with gcc2.95). Based on patch by KotH -- Best regards, Siarhei Siamashka
On Thu, 2008-05-01 at 13:05 +0300, Siarhei Siamashka wrote:
On Monday 28 April 2008, Siarhei Siamashka wrote:
Anyway, it should be mentioned that modern ARM cores (such as ARM11) support unaligned memory access natively. Older cores can emulate unaligned memory accesses by handling alignment faults in the kernel, but this is very slow.
PS. liba52 uses floating point math which is very slow on old ARM cores without hardware FPU, so using liba52 on such old cores is not very practical anyway.
Ping.
Can anybody consider reverting r10314 change? Or alternatively you can try to use the attached patch. Anyway, some kind of action needs to be taken to fix this breakage.
So this problem only occurs on cores where the code is "not very practical" anyway? Reverting it should be OK though - gcc seems to generate the same code for both versions on x86 where unaligned access works, so there should be little reason to use the raw unaligned access code except possibly to force unaligned access on a target which may support it but isn't guaranteed to.
Also I strongly suspect that r10490 was a proper fix for that ARM compilation issue with gcc 2.95 that r10314 tried to address but introduced a bigger problem instead.
It doesn't change the same code except changing "inline" to "always_inline" so I don't see how it could be a fix for the same compilation issue.
On Thu, May 1, 2008 at 1:05 PM, Siarhei Siamashka <siarhei.siamashka@gmail.com> wrote:
On Monday 28 April 2008, Siarhei Siamashka wrote:
Hi,
Currently liba52 is broken for ARM processors which require strict alignment for accessing memory (ARM9, XScale, ...). AC3 audio playback just produces noise.
Interestingly enough, the problem was introduced in 2003 (SVN revision 10314), and it was supposedly a fix for ARM: http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/2003-June/019377.html
I wonder if it was a fix that turned compilation error into runtime failure ;)
Anyway, it should be mentioned that modern ARM cores (such as ARM11) support unaligned memory access natively. Older cores can emulate unaligned memory accesses by handling alignment faults in the kernel, but this is very slow.
PS. liba52 uses floating point math which is very slow on old ARM cores without hardware FPU, so using liba52 on such old cores is not very practical anyway.
Ping.
Can anybody consider reverting r10314 change? Or alternatively you can try to use the attached patch. Anyway, some kind of action needs to be taken to fix this breakage.
Also I strongly suspect that r10490 was a proper fix for that ARM compilation issue with gcc 2.95 that r10314 tried to address but introduced a bigger problem instead.
------------------------------------------------------------------------ r10490 | alex | 2003-07-28 02:02:58 +0300 (Mon, 28 Jul 2003) | 2 lines Changed paths: M /trunk/liba52/bitstream.h
Changed swab32 from macro to inline function, this fixes compilation on alpha (with gcc2.95). Based on patch by KotH
I think the proper fix would be to don't use ALT_BITSTREAM_READER on architectures that can't read unaligned words. It seems that the code in question uses some kind of black magic trying to make unaligned access problem of the compiler, instead of reading it byte by byte. I think a patch that disables ALT_* on all ARMs without native unaligned access would be quickly committed. p.s. the black magic code is present in lavc bitreader too, but it looks like it is not used anywhere. Time for cleanup :)
On Thursday 01 May 2008, Ivan Kalvachev wrote:
------------------------------------------------------------------------ r10490 | alex | 2003-07-28 02:02:58 +0300 (Mon, 28 Jul 2003) | 2 lines Changed paths: M /trunk/liba52/bitstream.h
Changed swab32 from macro to inline function, this fixes compilation on alpha (with gcc2.95). Based on patch by KotH
I think the proper fix would be to don't use ALT_BITSTREAM_READER on architectures that can't read unaligned words. It seems that the code in question uses some kind of black magic trying to make unaligned access problem of the compiler, instead of reading it byte by byte.
That's a different topic (more related to ffmpeg), but ALT_BITSTREAM_READER is fine on ARM and other architectures. As you need to swap bytes in word on little endian architecture, just reading 4 bytes and doing 3 shifted AND operations is quite fast. With a small modification this code can be turned into 3 byte reads and two ANDs as we don't need all 32 bits and can make code smaller and faster. That's what I'm looking at now, and that's why this problem in liba52 got detected (initially not by testing with real AC3 files, but just spotting this part of suspicious code on reading it). In addition, there is ALT_BITSTREAM_READER_LE reader which is used unconditionally for some decoders.
I think a patch that disables ALT_* on all ARMs without native unaligned access would be quickly committed.
What's the problem with just reverting old broken commit? A change of bitstream reader will require some benchmarks done. And it would be optimization and not a bugfix.
p.s. the black magic code is present in lavc bitreader too, but it looks like it is not used anywhere. Time for cleanup :)
Don't hurry with the cleanup. Better try to understand the code first :) -- Best regards, Siarhei Siamashka
On Thursday 01 May 2008, Siarhei Siamashka wrote: [...]
That's a different topic (more related to ffmpeg), but ALT_BITSTREAM_READER is fine on ARM and other architectures. As you need to swap bytes in word on little endian architecture, just reading 4 bytes and doing 3 shifted AND operations is quite fast. With a small modification this code can be turned into 3 byte reads and two ANDs as we don't need all 32 bits and can make code smaller and faster.
A fix for typo. These are ADD (or OR) operations and not AND of course... -- Best regards, Siarhei Siamashka
On Thu, May 01, 2008 at 01:05:40PM +0300, Siarhei Siamashka wrote:
On Monday 28 April 2008, Siarhei Siamashka wrote:
Currently liba52 is broken for ARM processors which require strict alignment for accessing memory (ARM9, XScale, ...). AC3 audio playback just produces noise.
Interestingly enough, the problem was introduced in 2003 (SVN revision 10314), and it was supposedly a fix for ARM: http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/2003-June/019377.html
I wonder if it was a fix that turned compilation error into runtime failure ;)
Anyway, it should be mentioned that modern ARM cores (such as ARM11) support unaligned memory access natively. Older cores can emulate unaligned memory accesses by handling alignment faults in the kernel, but this is very slow.
PS. liba52 uses floating point math which is very slow on old ARM cores without hardware FPU, so using liba52 on such old cores is not very practical anyway.
Ping.
Can anybody consider reverting r10314 change? Or alternatively you can try to use the attached patch. Anyway, some kind of action needs to be taken to fix this breakage.
Patch applied, thanks. Diego
participants (4)
-
Diego Biurrun -
Ivan Kalvachev -
Siarhei Siamashka -
Uoti Urpala