[FFmpeg-devel] [PATCH] Some ra144.c simplifications

[Siarhei Siamashka]

On Sunday 22 June 2008, Stefan Gehrer wrote:
> Michael Niedermayer wrote:
> > On Sat, Jun 21, 2008 at 07:53:09AM +0200, Vitor Sessak wrote:
> >>     memcpy(work, statbuf,20);
> >
> > 10*sizeof(int16_t)
>
> I am curious about this. I would think we can guarantee that
> sizeof(int16_t) is always two. So the change would only have
> the advantage of better readability ("number of elements *
> size of elements" instead of total size).
> Am I right, or is there another advantage I don't see?

Well, as I'm currently investigating if it is possible to 
turn C55x DSP core into a video accelerator for FFmpeg on 
Nokia 770/N800/N810, I have to deal with this stuff:

"TMS320C55x Optimizing C/C++ Compiler User's Guide (Rev. F)":
http://focus.ti.com/lit/ug/spru281f/spru281f.pdf

You can check section "5.3 data types" of this manual. This 
compiler uses 16-bit char type, but sizeof(char) is equal to 1.
Consequently, operator sizeof returns the following for this 
set of data types:
[51517.806030] sizeof(int16_t)=1
[51517.806091] sizeof(int32_t)=2
[51517.806152] sizeof(int40_t)=4
[51517.806182] sizeof(void *)=2

int8_t and int64_t types are not supported on C55x at all (they are not
supported natively by hardware and the toolchain does not emulate them)

Strictly speaking, FFmpeg is not written in portable C, but that
does not make any problems for the vast majority of architectures.
And the rare problematic platforms will hopefully not survive long :)

But if real portability is the goal, the code should also check CHAR_BIT 
define from <limits.h>

Practically speaking, it probably makes sense to add something like this guard
code to FFmpeg:

/****/
#include <limits.h>

#if CHAR_BIT != 8
#error You are in a big trouble, your platform is not supported by FFmpeg
#endif
/****/

After that, the assumption that "sizeof(int16_t) is always two" would be
correct.

-- 
Best regards,
Siarhei Siamashka