On Sun, Jan 25, 2009 at 07:46:52PM +0100, Reimar Dцffinger wrote:
> On Sat, Jan 24, 2009 at 03:56:08AM +1300, Dennis Vshivkov
> wrote:
>> attached is a patch I've been using for a few months in my
>> personal MPlayer Debian package.
> I have seen it, but I did and still do not like it, not being
> able to use select also means that proper "sleep until input
> or next frame" behaviour will not be possible.
Sure thing. The select(2), in a sense, is already being not
used, but rather abused by input/lirc.c.
> Unfortunately there seems to be no other way to fix it, so
> basically this is ok.
Yeah, the `right' way seems:
[1] Make lirc_sock non-blocking.
[2] Rework current LIRC handling so that it's not polled
constantly but made a proper part of top-level select(2).
[3] When top-level select(2) indicates data on lirc_sock, poll
lirc_nextcode() until no code is returned.
Keeping solutions of separate problems separate seems favoured
on the ML, which is good. The [2], of course, solves a problem
that is rather orthogonal to what made me write the patch, which
basically implemented what [1] and [3] were in absence of [2].
The [2] can be added later, provided the polling, despite having
shrunk from repeated select(2) to repeated read(2), keeps
bothering whoever's knowledgeable enough to do it.
>> @@ -21,7 +22,8 @@
>>
>> int
>> mp_input_lirc_init(void) {
>> - int lirc_sock;
>> + int lirc_sock,
>> + mode;
>>
>> mp_msg(MSGT_LIRC,MSGL_V,MSGTR_SettingUpLIRC);
>> if((lirc_sock=lirc_init("mplayer",1))==-1){
> The lirc_sock line should not be modified.
Fixed.
>> @@ -36,12 +38,18 @@
>> return -1;
>> }
>>
>> + if ((mode = fcntl(lirc_sock, F_GETFL)) < 0 ||
>> + fcntl(lirc_sock, F_SETFL, mode | O_NONBLOCK) < 0) {
>> + mp_msg(MSGT_LIRC, MSGL_ERR, "setting non-blocking mode failed: %s\n",
>> + strerror(errno));
>> + lirc_deinit();
>> + return -1;
>> + }
> This is not very readable, IMO it should be
>> mode = fcntl(lirc_sock, F_GETFL);
>> if (mode < 0 || fcntl(lirc_sock, F_SETFL, mode | O_NONBLOCK) < 0) {
>> ...
Fixed.
>> - fd_set fds;
>> - struct timeval tv;
>> int r,cl = 0;
>> char *code = NULL,*c = NULL;
>>
>> @@ -59,21 +67,7 @@
>> return w;
>> }
>>
>> - // Nothing in the buffer, pool the lirc fd
>> - FD_ZERO(&fds);
>> - FD_SET(fd,&fds);
>> - memset(&tv,0,sizeof(tv));
>> - while((r = select(fd+1,&fds,NULL,NULL,&tv)) <= 0) {
>> - if(r < 0) {
>> - if(errno == EINTR)
>> - continue;
>> - mp_msg(MSGT_INPUT,MSGL_ERR,"Select error : %s\n",strerror(errno));
>> - return MP_INPUT_ERROR;
>> - } else
>> - return MP_INPUT_NOTHING;
>> - }
>> -
>> - // There's something to read
>> + // Nothing in the buffer, poll the lirc fd
> I think this means at least the
>> #include <sys/types.h>
>> #include <sys/time.h>
> can go,
Seems so (compiles on me). Fixed.
> also the pool -> poll fix should not be part of this patch
> (and I just fixed that in SVN).
Fixed.
The v2 of the patch is attached.
--
/Dennis Vshivkov <jaimor at orcon.net.nz>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: lirc-input-fully-consume.diff
Type: text/x-diff
Size: 3362 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20090127/6342b81a/attachment.diff>