[Ffmpeg-devel] FFMPEG RTSP for Windows

[Rich Felker]

On Wed, Mar 29, 2006 at 03:52:20PM -0600, Michael A. Kohn wrote:
> 
> >> I don't know anything about Winsock, but go ahead and send patches for
> >> any improvements you make. Send unified diffs: 'diff -Naur' or 'cvs
> diff
> >> -u'. If they're good changes then the patches ought to be considered.
> 
> One important thing is kinda missing from this.  The Winsock library has
> to be intialized before it can be used.  At the top of my test program I
> have something like this:
> 
> WSADATA wsaData;
> WORD wVersionRequested=MAKEWORD(1,1);
> int Win32isStupid;
> 
>   Win32isStupid=WSAStartup(wVersionRequested, &wsaData);
>   if (Win32isStupid) return -1;

This code does not give me much confidence in your knowledge of the C
language. The above can be done without useless temp vars:

WSADATA dummy;
if (WSAStartup(MAKEWORD(1,1), &dummy)) return 1;

and I assume MAKEWORD(1,1) is just 0x0101..

BTW your variable name is misleading. It suggests that the function
should always return -1... :)

> +
> +#ifndef CONFIG_WIN32
>  #include <sys/socket.h>
>  #include <netinet/in.h>
> +#endif

This is wrong. It will break cygwin.

> +#ifndef CONFIG_WIN32
>          sleep(1);
> +#else
> +        Sleep(1000);
> +#endif
>          return 0;
>      }

This is wrong. I doubt win32 is missing sleep, but if it is, you need
to make an emulation of sleep() using Sleep in a separate
win32-dependent file.

> +/* I'm sure the next two lines are evil in some way, but I'm not sure
> +   how else to do that right now */
> +#undef rand
> +#define random rand
> +#endif

random is not allowed in libav*. If it's used this is a bug and must
be corrected.

> +#ifdef CONFIG_WIN32
> +        sdp_ip.s_addr=inet_addr(buf1);
> +        if (sdp_ip.s_addr == INADDR_NONE)
> +            return;
> +#else
>          if (inet_aton(buf1, &sdp_ip) == 0)
>              return;
> +#endif

Way too many invasive changes for broken win32 stuff. Instead you
should isolate everything in a single place and emulate the correct
socket semantics.

> +#ifdef CONFIG_WIN32
> +        getsockopt (fd, SOL_SOCKET, SO_ERROR, (char FAR *)&ret, (int FAR
> *)&optlen);

The use of the FAR pointer qualifier is ABSOLUTELY WRONG!
This is win16 code!!

Moreover making such a cast is incorrect anyway, even with win16. The
promotion of a pointer to a far pointer on win16 would be implicit.

> -#ifdef __BEOS__
> +#if defined(__BEOS__) || defined(CONFIG_WIN32)
>              len = recv(s->fd, buf, size, 0);
>  #else
>              len = read(s->fd, buf, size);

IMO it's safe to use recv always.

> -#ifdef __BEOS__
> +#if defined(__BEOS__) || defined(CONFIG_WIN32)
>              len = send(s->fd, buf, size, 0);
>  #else
>              len = write(s->fd, buf, size);

Same for send.

> +#ifndef CONFIG_WIN32
>      close(s->udp_fd);
>  #else
>      closesocket(s->udp_fd);
>  #endif
> +#else
> +    closesocket(s->udp_fd);
> +#endif

Using shutdown() instead of close() or closesocket() should be safe on
both unix and windows.

Rich