[MPlayer-dev-eng] [PATCH] Support for QNX: QSA audio and Photon GUI.

[Reimar Döffinger]

On Sun, Feb 03, 2013 at 10:23:11PM +0200, Mike Gorchak wrote:
> Hi,
> 
> Here is a patch (made against latest repository snapshot) to enable
> native QNX support in the mplayer. This
> patch contains QNX Sound Architecture (QSA) audio driver and Photon GUI
> YUV/RGB renderer.
> 
> I've posted previous patch a week ago, but it seems ignored. This
> patch has many improvements comparing to previous.

One problem is that the patch is mangled, there are linebreaks
that shouldn't be there.
That makes it impossible to apply.
Also "svn diff" results in somewhat nicer looking output,
instead of comparing "svn export" trees.
It would also be a good bit easier to review if the ao and vo
was separate, but much more importantly the configure changes
that don't have anything to do with ao and vo definitely should be
separate.
For example this one has nothing to do with ao or vo support:

> +elif qnx ; then
> +  default_cdrom_device="/dev/cd0"
> @@ -3986,6 +3990,19 @@
>  fi #if sunos


> +if qnx; then
> +echocheck "dcmd_cam.h (QNX)"
> +_qnx_cam=no
> +header_check sys/dcmd_cam.h && _qnx_cam=yes
> +if test "$_qnx_cam" = yes ; then
> +  def_qnx_cam='#define QNX_DCMD_CAM_H 1'
> +else
> +  def_qnx_cam='#undef QNX_DCMD_CAM_H'
> +fi
> +echores "$_qnx_cam"
> +fi #if qnx

These defines seem unused.

> +echocheck "QSA audio"
> +if test "$_qsa" = auto ; then
> +  _qsa=no
> +  header_check sys/asoundlib.h -lasound $ld_dl $ld_pthread && _qsa=yes

This check is 100% identical to the Linux ALSA check.
This can't be working correctly.
In fact, your ao_qsa seems to simply reimplement ao_alsa?!?!

> -if win32 || os2 || dragonfly || freebsd || openbsd || sunos || amigaos ; then
> +if win32 || os2 || dragonfly || freebsd || openbsd || sunos ||
> amigaos || qnx ; then
>    default_dvd_device=$default_cdrom_device
>  elif darwin ; then
>    default_dvd_device="/dev/rdiskN"
> @@ -5924,9 +5976,9 @@
>  if test "$_dvdread_internal" = auto ; then
>    _dvdread_internal=no
>    _dvdread=no
> -  if (linux || freebsd || netbsd || openbsd || dragonfly || sunos || hpux) &&
> +  if (linux || freebsd || netbsd || openbsd || dragonfly || sunos ||
> hpux || qnx) &&
>       (test "$_dvd" = yes || test "$_cdrom" = yes || test "$_cdio" = yes ||
> -      test "$_dvdio" = yes || test "$_bsdi_dvd" = yes) ||
> +      test "$_dvdio" = yes || test "$_bsdi_dvd" = yes || test
> "$_qnx_cam" = yes) ||
>       darwin || win32 || os2; then
>      _dvdread_internal=yes

These seem unrelated to the ao/vo, and what does _qnx_cam have
to do with dvdread?

> --- mplayer-export-2013-02-03-orig/cpudetect.c	2013-01-31
> 07:24:14.000000000 +0200
> +++ mplayer-export-2013-02-03/cpudetect.c	2013-01-24 14:04:50.000000000 +0200
> @@ -46,6 +46,9 @@
>  #include <os2.h>
>  #elif defined(__AMIGAOS4__)
>  #include <proto/exec.h>
> +#elif defined(__QNXNTO__) && defined(__X86__)
> +#include <x86/cpuinline.h>
> +#include <sys/syspage.h>
>  #endif
> 
>  /* Thanks to the FreeBSD project for some of this cpuid code, and
> @@ -216,6 +219,19 @@
>      mp_msg(MSGT_CPUDETECT,MSGL_WARN, "Cannot test OS support for SSE,
> disabling to be safe.\n" );
>      gCpuCaps.hasSSE=0;
>  #endif /* _POSIX_SOURCE */
> +#elif defined(__QNXNTO__) && defined(__X86__)
> +    mp_msg(MSGT_CPUDETECT,MSGL_V, "Testing OS support for SSE... " );
> +    if ((__cpu_flags & X86_CPU_SIMD)==X86_CPU_SIMD)
> +    {
> +        gCpuCaps.hasSSE=1;
> +    }
> +    mp_msg(MSGT_CPUDETECT,MSGL_V, gCpuCaps.hasSSE ? "yes.\n" : "no!\n" );
> +    mp_msg(MSGT_CPUDETECT,MSGL_V, "Testing OS support for SSE2... " );
> +    if ((__cpu_flags & X86_CPU_SSE2)==X86_CPU_SSE2)
> +    {
> +        gCpuCaps.hasSSE2=1;
> +    }
> +    mp_msg(MSGT_CPUDETECT,MSGL_V, gCpuCaps.hasSSE2 ? "yes.\n" : "no!\n" );

Also unrelated to ao/vo support.

> diff -u -r -N mplayer-export-2013-02-03-orig/libdvdread4/bswap.h
> mplayer-export-2013-02-03/libdvdread4/bswap.h
> --- mplayer-export-2013-02-03-orig/libdvdread4/bswap.h	2013-01-31
> 07:24:56.000000000 +0200
> +++ mplayer-export-2013-02-03/libdvdread4/bswap.h	2013-02-03
> 15:03:30.000000000 +0200
> @@ -92,6 +92,12 @@
>        (((x) & 0x000000000000ff00ULL) << 40) | \
>        (((x) & 0x00000000000000ffULL) << 56))
> 
> +#elif defined(__QNXNTO__)
> +#include <gulliver.h>
> +#define B2N_16(x) x = ENDIAN_SWAP16(x)
> +#define B2N_32(x) x = ENDIAN_SWAP32(x)
> +#define B2N_64(x) x = ENDIAN_SWAP64(x)

libdvdread is a separate project, you should send the patch to them.
Though IMHO it would be better to just always use the fallback code,
and maybe the gcc bultins.
I don't think using the OS-specific stuff has much value.

> 
> +#if !defined(__QNXNTO__)
>  typedef short int16_t;
> +#endif /* __QNXNTO__ */

That line is nonsense, removed it in SVN, thanks for noticing.

> +    switch (photon_mplayer_format)
> +    {
> +        case IMGFMT_YUY2:
> +        case IMGFMT_YVYU:

I think you should be able to avoid this switch and reduce the code
size a lot by using vo_get_draw_alpha.
I'll probably have a lot more comments on the vo, but I don't
have time to review in detail right now.