opus - Jun 2015 - [PATCH 1/1] Updated opus_types.h to correctly support 8 and 64 bit types.

- Replaced blanket #define of 8 & 64 bit types with typedefs for each
  platform to match 16 & 32 bit types.

- Updated existing typedefs for each platform to fix odd values and
  improve consistency.
---
 include/opus_types.h | 125 ++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 100 insertions(+), 25 deletions(-)

diff --git a/include/opus_types.h b/include/opus_types.h
index b28e03a..209c17c 100644
--- a/include/opus_types.h
+++ b/include/opus_types.h
@@ -1,5 +1,6 @@
 /* (C) COPYRIGHT 1994-2002 Xiph.Org Foundation */
 /* Modified by Jean-Marc Valin */
+/* Modified by Simon Jackson (sizeak at gmail.com) 03/Jun/2015 */
 /*
    Redistribution and use in source and binary forms, with or without
    modification, are permitted provided that the following conditions
@@ -35,125 +36,199 @@
 
 /* Use the real stdint.h if it's there (taken from Paul Hsieh's
pstdint.h) */
 #if (defined(__STDC__) && __STDC__ && __STDC_VERSION__ >=
199901L) || (defined(__GNUC__) && (defined(_STDINT_H) ||
defined(_STDINT_H_)) || defined (HAVE_STDINT_H))
-#include <stdint.h>
 
+#  include <stdint.h>
+   typedef int8_t opus_int8;
+   typedef uint8_t opus_uint8;
    typedef int16_t opus_int16;
    typedef uint16_t opus_uint16;
    typedef int32_t opus_int32;
    typedef uint32_t opus_uint32;
+   typedef int64_t opus_int64;
+   typedef uint64_t opus_uint64;
+   
 #elif defined(_WIN32)
 
 #  if defined(__CYGWIN__)
-#    include <_G_config.h>
-     typedef _G_int32_t opus_int32;
-     typedef _G_uint32_t opus_uint32;
-     typedef _G_int16 opus_int16;
-     typedef _G_uint16 opus_uint16;
+#     include <stdint.h>
+      typedef int8_t opus_int8;
+      typedef uint8_t opus_uint8;
+      typedef int16_t opus_int16;
+      typedef uint16_t opus_uint16;
+      typedef int32_t opus_int32;
+      typedef uint32_t opus_uint32;
+      typedef int64_t opus_int64;
+      typedef uint64_t opus_uint64;
 #  elif defined(__MINGW32__)
+#    include <sys/types.h>
+     typedef char opus_int8;
+     typedef unsigned char opus_uint8; 
      typedef short opus_int16;
      typedef unsigned short opus_uint16;
      typedef int opus_int32;
      typedef unsigned int opus_uint32;
+     typedef long long opus_int64;
+     typedef unsigned long long opus_uint64;
 #  elif defined(__MWERKS__)
-     typedef int opus_int32;
-     typedef unsigned int opus_uint32;
+     typedef char opus_int8;
+     typedef unsigned char opus_uint8; 
      typedef short opus_int16;
      typedef unsigned short opus_uint16;
+     typedef int opus_int32;
+     typedef unsigned int opus_uint32;
+     typedef long long opus_int64;
+     typedef unsigned long long opus_uint64;
 #  else
      /* MSVC/Borland */
-     typedef __int32 opus_int32;
-     typedef unsigned __int32 opus_uint32;
+     typedef __int8 opus_int8;
+     typedef unsigned __int8 opus_uint8;
      typedef __int16 opus_int16;
      typedef unsigned __int16 opus_uint16;
+     typedef __int32 opus_int32;
+     typedef unsigned __int32 opus_uint32;
+     typedef __int64 opus_int64;
+     typedef unsigned __int64 opus_uint64;     
 #  endif
 
 #elif defined(__MACOS__)
 
 #  include <sys/types.h>
+   typedef SInt8 opus_int8;
+   typedef UInt8 opus_uint8;
    typedef SInt16 opus_int16;
    typedef UInt16 opus_uint16;
    typedef SInt32 opus_int32;
    typedef UInt32 opus_uint32;
+   typedef SInt64 opus_int64;
+   typedef UInt64 opus_uint64;
 
 #elif (defined(__APPLE__) && defined(__MACH__)) /* MacOS X Framework
build */
 
-#  include <sys/types.h>
+#  include <inttypes.h>
+   typedef int8_t opus_int8;
+   typedef uint8_t opus_uint8;
    typedef int16_t opus_int16;
-   typedef u_int16_t opus_uint16;
+   typedef uint16_t opus_uint16;
    typedef int32_t opus_int32;
-   typedef u_int32_t opus_uint32;
+   typedef uint32_t opus_uint32;
+   typedef int64_t opus_int64;
+   typedef uint64_t opus_uint64;
+
+#elif defined(__HAIKU__)
 
+  /* Haiku */
+#  include <sys/types.h>
+   typedef char opus_int8;
+   typedef unsigned char opus_uint8;
+   typedef short opus_int16;
+   typedef unsigned short opus_uint16;
+   typedef int opus_int32;
+   typedef unsigned int opus_uint32;
+   typedef long long opus_int64;
+   typedef unsigned long long opus_uint64;
+   
 #elif defined(__BEOS__)
 
    /* Be */
 #  include <inttypes.h>
-   typedef int16 opus_int16;
-   typedef u_int16 opus_uint16;
+   /* 
+    * BeOS inttypes seem to define these as c99 types,
+    * however I don't have access to an actual BeOS machine 
+    */
+   typedef int8_t opus_int8;
+   typedef uint8_t opus_uint8;
+   typedef int16_t opus_int16;
+   typedef uint16_t opus_uint16;
    typedef int32_t opus_int32;
-   typedef u_int32_t opus_uint32;
+   typedef uint32_t opus_uint32;
+   typedef int64_t opus_int64;
+   typedef uint64_t opus_uint64;
 
 #elif defined (__EMX__)
 
    /* OS/2 GCC */
+   typedef char opus_int8;
+   typedef unsigned char opus_uint8;
    typedef short opus_int16;
    typedef unsigned short opus_uint16;
    typedef int opus_int32;
    typedef unsigned int opus_uint32;
+   typedef long long opus_int64;
+	typedef unsigned long long opus_uint64;
 
 #elif defined (DJGPP)
 
    /* DJGPP */
+   typedef char opus_int8;
+   typedef unsigned char opus_uint8;
    typedef short opus_int16;
    typedef unsigned short opus_uint16;
    typedef int opus_int32;
    typedef unsigned int opus_uint32;
+   typedef long long opus_int64;
+	typedef unsigned long long opus_uint64;
 
 #elif defined(R5900)
 
    /* PS2 EE */
-   typedef int opus_int32;
-   typedef unsigned opus_uint32;
+   typedef char opus_int8;
+   typedef unsigned char opus_uint8;
    typedef short opus_int16;
    typedef unsigned short opus_uint16;
+   typedef int opus_int32;
+   typedef unsigned int opus_uint32;
+   typedef long long opus_int64;
+	typedef unsigned long long opus_uint64;
 
 #elif defined(__SYMBIAN32__)
 
    /* Symbian GCC */
+   typedef char signed opus_int8;
+   typedef unsigned char opus_uint8;
    typedef signed short opus_int16;
    typedef unsigned short opus_uint16;
    typedef signed int opus_int32;
    typedef unsigned int opus_uint32;
+   typedef signed long long opus_int64;
+	typedef unsigned long long opus_uint64;
 
 #elif defined(CONFIG_TI_C54X) || defined (CONFIG_TI_C55X)
 
+   typedef char opus_int8;
+   typedef unsigned char opus_uint8;
    typedef short opus_int16;
    typedef unsigned short opus_uint16;
    typedef long opus_int32;
    typedef unsigned long opus_uint32;
+   typedef long long opus_int64;
+	typedef unsigned long long opus_uint64;
 
 #elif defined(CONFIG_TI_C6X)
 
+   typedef char opus_int8;
+   typedef unsigned char opus_uint8;
    typedef short opus_int16;
    typedef unsigned short opus_uint16;
    typedef int opus_int32;
    typedef unsigned int opus_uint32;
+   typedef long long opus_int64;
+   typedef unsigned long long opus_uint64;
 
 #else
 
    /* Give up, take a reasonable guess */
+   typedef char opus_int8;
+   typedef unsigned char opus_uint8;
    typedef short opus_int16;
    typedef unsigned short opus_uint16;
    typedef int opus_int32;
    typedef unsigned int opus_uint32;
+   typedef long long opus_int64;
+   typedef unsigned long long opus_uint64;
 
 #endif
 
-#define opus_int         int                     /* used for counters etc; at
least 16 bits */
-#define opus_int64       long long
-#define opus_int8        signed char
-
-#define opus_uint        unsigned int            /* used for counters etc; at
least 16 bits */
-#define opus_uint64      unsigned long long
-#define opus_uint8       unsigned char
+typedef int opus_int;                   /* used for counters etc; at least 16
bits */
+typedef unsigned int opus_uint;           /* used for counters etc; at least 16
bits */
 
 #endif  /* OPUS_TYPES_H */
-- 
2.4.1

Simon Jackson wrote:
> +   typedef char opus_int8;
ISO C allows the "char" type to be either signed or unsigned.  It is
even a compiler switch on some compilers.  Changing this from "signed
char" to plain "char" breaks when "char" is unsigned.

Several of the changed lines introduce inconsistent indentation or
spaces at the end of the line, which is somewhat unfitting for a patch
whose purpose is to improve consistency.

Have each of the affected #elif sections been tested on the platforms
where that section is compiled?

 - Mark

Indentation looked ok to me, its probably due to my editor. I will fix it in vim
and re-submit it.

Didn?t know that, will fix. 

No they haven?t all been tested, however I consulted documentation available for
each of the platforms on the internet and copied the ogg headers when in doubt.
> On 3 Jun 2015, at 17:43, Mark Harris <mark.hsj at gmail.com> wrote:
> 
> Simon Jackson wrote:
>> +   typedef char opus_int8;
> 
> ISO C allows the "char" type to be either signed or unsigned.  It
is
> even a compiler switch on some compilers.  Changing this from "signed
> char" to plain "char" breaks when "char" is
unsigned.
> 
> Several of the changed lines introduce inconsistent indentation or
> spaces at the end of the line, which is somewhat unfitting for a patch
> whose purpose is to improve consistency.
> 
> Have each of the affected #elif sections been tested on the platforms
> where that section is compiled?
> 
> - Mark

A more important issue here is that not all platforms we target are even
8-bit addressable (e.g. the TI C5x family). That is why I've preferred
using "unsigned char" (or in some places signed char) when the idea is
just to use the smallest size possible on the platform. As for 64 bits
types, it's been purposefully defined as "long long" because on
some
platforms (e.g. C55x again) long long is just 40 bits, which is just
enough for what we need.

Simon, is your patch designed to fix a particular issue with types on a
certain platform or is it more of a "general cleanup" type?

Cheers,

	Jean-Marc

On 03/06/15 12:43 PM, Mark Harris wrote:
> Simon Jackson wrote:
>> +   typedef char opus_int8;
> 
> ISO C allows the "char" type to be either signed or unsigned.  It
is
> even a compiler switch on some compilers.  Changing this from "signed
> char" to plain "char" breaks when "char" is
unsigned.
> 
> Several of the changed lines introduce inconsistent indentation or
> spaces at the end of the line, which is somewhat unfitting for a patch
> whose purpose is to improve consistency.
> 
> Have each of the affected #elif sections been tested on the platforms
> where that section is compiled?
> 
>  - Mark
> _______________________________________________
> opus mailing list
> opus at xiph.org
> http://lists.xiph.org/mailman/listinfo/opus
>

On 2015-06-03 6:21 AM, Simon Jackson wrote:
> - Replaced blanket #define of 8 & 64 bit types with typedefs for each
>   platform to match 16 & 32 bit types.
You didn't say why you need these additional types?

 -r

Jean-Marc,

Clearly I haven't thought these changes through properly and was only
really considering common desktop systems. I was originally fixing an issue
when compiling my code as 64bit where my use of int64_t (defined as long on
my platform) was conflicting with the opusfile stream callbacks (e.g.
op_seek_func) which were using the opus_int64 type which was defined as
long long. When I looked at the header I thought it could do with some
work, so instead of putting casts in my code, I modified the header and
sent a patch. I didn't even consider building for architectures which could
have a 40bit long etc. and am obviously a bit out of my depth in this case.

Sorry for the bad patch, please just disregard it. It might be worth
updating the header based on the ogg one though. Even without the breaking
changes I made, it seems to be much more update in some areas.

Simon

On 3 June 2015 at 17:59, Ralph Giles <giles at thaumas.net> wrote:
> On 2015-06-03 6:21 AM, Simon Jackson wrote:
>
> > - Replaced blanket #define of 8 & 64 bit types with typedefs for
each
> >   platform to match 16 & 32 bit types.
>
> You didn't say why you need these additional types?
>
>  -r
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
http://lists.xiph.org/pipermail/opus/attachments/20150603/21dbe28d/attachment-0001.htm