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

[Reimar Döffinger]

On Mon, Feb 04, 2013 at 09:16:24PM +0200, Mike Gorchak wrote:
> >> +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.
> 
> No, not identical. ALSA headers are located in the alsa/ subdirectory,
> while QSA headers are always in sys/ subdirectory.

While I was wrong, so are you unfortunately, on a Linux system:
$ cat /usr/include/sys/asoundlib.h 
#warning This header is deprecated, use <alsa/asoundlib.h> instead.
#include <alsa/asoundlib.h>

This means that QSA code will be enable in a Linux system.

> > In fact, your ao_qsa seems to simply reimplement ao_alsa?!?!
> 
> No, QSA is based on ALSA 0.5 API, but not the ALSA source (FYI when
> mplayer had ao_alsa5 module - it does not work well in QSA
> environment). But further improvements of QSA made it uncompatible
> with ALSA 0.5.

If it's not compatible they really would have done better
to come up with their own header.

> >> +    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.
> 
> Yes, but vo_get_draw_alpha does not support V422 format and some other
> which will be added later.

You can still use it for those format where it works,
this kind of 100s of duplicated lines that really does some
very simple stuff is a maintenance horror.
While we never got to the point of having unit tests, any code
where the thought of writing tests to achieve 100% line coverage
sends shivers down your spine should be avoided.
Even more so when you don't have unit tests and need to test everything
manually.