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

[Diego Biurrun]

On Mon, Aug 13, 2007 at 09:12:22PM +0800, Ulion wrote:
> 2007/8/13, Reimar Döffinger <Reimar.Doeffinger at stud.uni-karlsruhe.de>:
> > 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.
> for machine has Apple Remote support it run return 0 and _apple_remote
> setted to yes.
> for machine not support Apple Remote, it run return non-zero,
> _apple_remote setted to no.
> this is exactly what the 'auto' mean, check whether current machine
> support Apple Remote.

There is a difference between a machine that can compile Apple Remote
support and another one that has Apple Remote support and can actually
use it.  What happens if you compile on a machine without a remote for
other Apple machines that do have remotes?

> --- input/input.c	(revision 24050)
> +++ input/input.c	(working copy)
> --- input/ar.c	(revision 0)
> +++ input/ar.c	(revision 0)
> @@ -0,0 +1,411 @@
> +
> +    // TODO: multi controls could be found, we only use the first usable one

multiple

> +        // TODO: cooperate correctly with other apps using Remote Control
> +        // Wrapper 2.0 above, after open exclusive mode failed, try to send
> +        // a request to other apps and wait for a moment someone release it.

Using a software called "Remote Control Wrapper" at version 2.0 or
above?

I think what you want to say in the second part (which should be a
separate sentence) is

  If opening the remote control in exclusive mode failed, try to send a
  request to other apps and wait a moment for it to be released.

Did I understand correctly?

> +    /* depth: maximum number of elements in queue before oldest elements
> +              in queue begin to be lost. */

Number of elements the queue can hold before the oldest elements get
dropped.

Is that really a queue?  A queue should drop the newest, not the oldest
elements.

> +    // FIXME: should we necessary to add these cookies into the queue,
> +    //        or just wait the queue give us new events?

I cannot parse this one.

> +    // not used any more.

anymore

> +    // not useful any more.

ditto

> +        // if mplayer running in slave mode, also check parent process

if MPlayer is running

> +        // skip 5 to get different sum cookie from each input from Apple Remote

Skip 5 cookies to get a cookie with a different sum from each input of
the Apple Remote.

Is that what you were trying to say?

> +    // current front, if closed, re-open the device to make sure
> +    // not affected by other apps or system use apple remote

I don't understand.

> +        // TODO: same as first time open the device, try cooperate with
> +        //       other apps to request exculsive open.

excLUsive

maybe better:

TODO: Try to cooperate with other apps to exclusively open the device,
just like on initial opening.


Thanks for bearing with us through all those review rounds.  Since
Reimar seemed reasonably happy with the code I will apply it after we
address the minor issues above.

Diego