diff mbox

[FFmpeg-devel] avcodec/utvideodec: use cached bitstream reader only on targets with fast 64bit ops

Message ID 20180901231835.1404-1-jamrial@gmail.com
State New
Headers show

Commit Message

James Almer Sept. 1, 2018, 11:18 p.m. UTC
Signed-off-by: James Almer <jamrial@gmail.com>
---
Unbenched, but if x86_32 is slow with it, then chances are so are arm
and similar 32bit architectures.

It may also be worth looking into just making HAVE_FAST_64BIT the
default in get_bits.h directly.

 libavcodec/utvideodec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Carl Eugen Hoyos Sept. 2, 2018, 8:30 a.m. UTC | #1
> Am 02.09.2018 um 01:18 schrieb James Almer <jamrial@gmail.com>:
> 
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
> Unbenched, but if x86_32 is slow with it, then chances are so are arm
> and similar 32bit architectures.

Please do not commit without benchmarks, I found no other architecture where the new reader is slower.

Thank you, Carl Eugen
James Almer Sept. 2, 2018, 5:46 p.m. UTC | #2
On 9/2/2018 5:30 AM, Carl Eugen Hoyos wrote:
> 
> 
>> Am 02.09.2018 um 01:18 schrieb James Almer <jamrial@gmail.com>:
>>
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>> Unbenched, but if x86_32 is slow with it, then chances are so are arm
>> and similar 32bit architectures.
> 
> Please do not commit without benchmarks, I found no other architecture where the new reader is slower.

I can't bench on anything but x86, which is why i sent this patch to get
other people's opinions.

You tried other 32 bit architectures like arm7 or PPC? fast_64 is a
configure time constant used in a lot of speed critical modules to
choose between using 32bit or 64bit ops, so seeing how this new
bitstream reader uses a 64bit cache I'd expect it to be slower in such
targets as well.
Carl Eugen Hoyos Sept. 2, 2018, 6:29 p.m. UTC | #3
> Am 02.09.2018 um 19:46 schrieb James Almer <jamrial@gmail.com>:
> 
>> On 9/2/2018 5:30 AM, Carl Eugen Hoyos wrote:
>> 
>> 
>>> Am 02.09.2018 um 01:18 schrieb James Almer <jamrial@gmail.com>:
>>> 
>>> Signed-off-by: James Almer <jamrial@gmail.com>
>>> ---
>>> Unbenched, but if x86_32 is slow with it, then chances are so are arm
>>> and similar 32bit architectures.
>> 
>> Please do not commit without benchmarks, I found no other architecture where the new reader is slower.
> 
> I can't bench on anything but x86, which is why i sent this patch to get
> other people's opinions.

Then please don’t commit.

> You tried other 32 bit architectures like arm7 or PPC?

Yes.

 Carl Eugen
Carl Eugen Hoyos Sept. 3, 2018, 4:04 p.m. UTC | #4
2018-09-02 19:46 GMT+02:00, James Almer <jamrial@gmail.com>:
> fast_64 is a
> configure time constant used in a lot of speed critical modules to
> choose between using 32bit or 64bit ops, so seeing how this new
> bitstream reader uses a 64bit cache I'd expect it to be slower in such
> targets as well.

Do you have a suggestion on how to test the existing usages
of fast_64?

Carl Eugen
diff mbox

Patch

diff --git a/libavcodec/utvideodec.c b/libavcodec/utvideodec.c
index 3891df3570..542b051e45 100644
--- a/libavcodec/utvideodec.c
+++ b/libavcodec/utvideodec.c
@@ -27,7 +27,7 @@ 
 #include <inttypes.h>
 #include <stdlib.h>
 
-#define CACHED_BITSTREAM_READER !ARCH_X86_32
+#define CACHED_BITSTREAM_READER HAVE_FAST_64BIT
 #define UNCHECKED_BITSTREAM_READER 1
 
 #include "libavutil/intreadwrite.h"