[FFmpeg-devel] [PATCH] MMX for Win64

[Måns Rullgård]

Diego Biurrun <diego at biurrun.de> writes:

> On Wed, May 07, 2008 at 02:03:50AM +0100, Ramiro Polla wrote:
>> 
>>   wrote:
>> > Aurelien Jacobs <aurel at gnuage.org> writes:
>> > 
>> >> M?ns Rullg?rd wrote:
>> >>
>> >>> Ramiro Polla <ramiro at lisha.ufsc.br> writes:
>> >>>
>> >>>>> *_t is reserved by POSIX.
>> >>>> Well, then I guess you should take care of the rest of the code
>> >>>> that typedefs _t stuff:
>> >>>> ramiro at found:/usrc/ffmpeg/src$ grep -r typedef * | grep "_t;"
>> >>>> libavcodec/vp56.h:typedef struct vp56_context vp56_context_t;
>> >>>> libavcodec/vp56.h:typedef struct vp56_mv vp56_mv_t;
>> >>>> libavcodec/i386/dsputil_mmx.h:typedef struct { uint64_t a, b; }
>> >>>> xmm_t; libavformat/iff.c:typedef enum {COMP_NONE, COMP_FIB,
>> >>>> COMP_EXP} svx8_compression_t;
>> >>>> libavformat/avio.h:typedef int64_t offset_t;
>> >>>> libavformat/os_support.h:typedef int socklen_t;
>> >>>> libavformat/os_support.h:typedef unsigned long nfds_t;
>> >>>> libpostproc/postprocess.h:typedef void pp_context_t;
>> >>>> libpostproc/postprocess.h:typedef void pp_mode_t;
>> >>>> tests/videogen.c:typedef unsigned char uint8_t;
>> >>> Patches welcome ;-)
>> 
>> Using Michael's words [0] with diverging text edited in []:
>> You misunderstand things a little
>> you want it fixed -> you fix it
>> you dont want it fixed -> you dont fix it
>> you want someone else to fix it -> you pay someone else to fix it
>> 
>> If you expect us to work for you for free then you have a serious 
>> problem with reality, we are volunteers and we work on what we consider 
>> fun. [Full POSIX compliance] work is not fun.
>> 
>> Our [POSIX compliance] does not [comply to] everything, no [program] 
>> does, []. If you want some specific feature supported its up to you to 
>> do the work. We will certainly help, awnser questions and review patches.
>
> It certainly would not be the first time somebody steps up and does
> janitorial work...
>
>> >>>> And what about being constructive from time to time?
>>                      ^^^^^^^^^^^^^^^^^^
>> [...]
>> > Regardless of the precise semantics intended, I think it's a bad idea
>> > to invade this namespace.
>> 
>> You forgot the "being constructive" part.
>
> OK guys, how hard can it be?  Just replace _t with _type or with
> _struct.
>
>   ramiro at found:/usrc/ffmpeg/src$ grep -r typedef * | grep "_t;"
>   libavcodec/vp56.h:typedef struct vp56_context vp56_context_t;
>   libavcodec/vp56.h:typedef struct vp56_mv vp56_mv_t;
>
> Maybe just write 'struct vp56_mv' everywhere..

There are good reasons not to use typedefs for structs (or pointers).
I avoid doing it in new code.

>   libavcodec/i386/dsputil_mmx.h:typedef struct { uint64_t a, b; } xmm_t;
>   libavformat/iff.c:typedef enum {COMP_NONE, COMP_FIB, COMP_EXP} svx8_compression_t;
>   libpostproc/postprocess.h:typedef void pp_context_t;
>   libpostproc/postprocess.h:typedef void pp_mode_t;
>
> These would have to be renamed, see my above suggestion.

Those typedefs to void look really stupid to me.

>   libavformat/avio.h:typedef int64_t offset_t;
>
> That one looks suspicious.  Isn't offset_t defined in some system
> header?

You're thinking of off_t.  And yes, it's confusing.

>   libavformat/os_support.h:typedef int socklen_t;
>   libavformat/os_support.h:typedef unsigned long nfds_t;
>
> These can be ignored, the definitions are basically under an
> #ifndef NON_POSIX_SYSTEM.
>
>   tests/videogen.c:typedef unsigned char uint8_t;
>
> This ugliness can be removed, patch sent.

Apply it.

-- 
M?ns Rullg?rd
mans at mansr.com