[MPlayer-dev-eng] [PATCH] Apple Remote support

[Reimar Döffinger]

Hello,
On Mon, Aug 13, 2007 at 03:38:41PM +0800, Ulion wrote:
> 2007/8/13, Reimar Döffinger <Reimar.Doeffinger at stud.uni-karlsruhe.de>:
> >
> > > +  cc_check -framework IOKit && tmp_run && _apple_remote=yes
> >
> > configure checks should not execute anything unless absolutely
> > necessary.
> I think this run check is necessary when apple-remote support is setted 'auto'.

I do not really like it because I think it causes problems for everyone
compiling for a different machine.
But configure is Diego's domain and if he is fine with it I won't
complain.
But the function you use to test is a copy-and-paste of the FindHIDDevices
function you updated, so it would be good to update this as well so they
match again.

> > > +#ifdef HAVE_APPLE_REMOTE
> > > +  { { AR_PLAY, 0}, "pause" },
> > > ...
> > > +  { { AR_VDOWN, 0 }, "volume -1"},
> > > +#endif
> >
> > I think the #ifdef is not needed.
> in the context, other codes all have this '#ifdef' such as joystick's,
> so we follow the style.

Actually only joystick is comparable, the other ifdefs are not for input
devices but for MPlayer features, so their bindings make no sense (and
actually print an error) when the feature is not available.
But I admit this is a separate issue.

> > > +static IOHIDDeviceInterface **hidDeviceInterface = NULL;
> > > +static int inited = 0;
> > > +static int hidDeviceIsOpen = 0;
> > > +static io_iterator_t hidObjectIterator = (io_iterator_t)NULL;
> >
> > Useless initializations.
> hidObjectIterator moved into init function.
> other init maybe useless, but for make sure.

I don't really mind, but the reason I call them useless is because the C
standard explicitly says that this kind of variables (not allocated dynamically on
the stack) will be initialized to zero.
Btw. I think you forgot to set inited = 0 in mp_input_ar_close.
While I probably as always could find more nits to pick, I don't have
much time for reviewing and the code seems good enough for me.

Greetings,
Reimar Döffinger