[FFmpeg-devel] [PATCH] Replace select with poll

[Luca Barbato]

On 01/24/2011 11:26 AM, Martin Storsj? wrote:
> 
> I'm not particularly fond of VLA's, and others have worked hard on getting 
> rid of them...

I'm thinking make something like

udp_read_packet(args) {

p= malloc()

ret = real_udp_read_packet(args, p)

free(p)
return ret;
}

or we could allocate those once we know the number of the streams, I'll
check later.

> 
>> +            //FIXME this loop should be simplified
> 
> Is this comment still valid? During the last review, you simplified it a 
> bit, making it not worse than before at least.

it's a stray, thanks for pointing

> 
>>              for (i = 0; i < rt->nb_rtsp_streams; i++) {
>>                  rtsp_st = rt->rtsp_streams[i];
>>                  if (rtsp_st->rtp_handle) {
>> -                    fd = url_get_file_handle(rtsp_st->rtp_handle);
>> -                    fd_rtcp = rtp_get_rtcp_file_handle(rtsp_st->rtp_handle);
>> -                    if (FD_ISSET(fd_rtcp, &rfds) || FD_ISSET(fd, &rfds)) {
>> +                    if (p[j].revents & POLLIN) {
>>                          ret = url_read(rtsp_st->rtp_handle, buf, buf_size);
>>                          if (ret > 0) {
>>                              *prtsp_st = rtsp_st;
>>                              return ret;
>>                          }
>>                      }
>> +                    j+=2;
>>                  }
>>              }
> 
> This only checks if POLLIN was set on the RTP fd, not on the RTCP one. I 
> guess a simple if (p[j].revents & POLLIN || p[j + 1].revents & POLLIN) 
> would be sufficient?

Should

>> @@ -73,6 +71,7 @@ static int tcp_open(URLContext *h, const char *uri, int flags)
>>   redo:
>>      ret = connect(fd, cur_ai->ai_addr, cur_ai->ai_addrlen);
>>      if (ret < 0) {
>> +        struct pollfd p = {fd, POLLOUT, 0};
>>          if (ff_neterrno() == FF_NETERROR(EINTR)) {
>>              if (url_interrupt_cb())
>>                  goto fail1;
>> @@ -88,15 +87,8 @@ static int tcp_open(URLContext *h, const char *uri, int flags
>>                  ret = AVERROR(EINTR);
>>                  goto fail1;
>>              }
>> -            fd_max = fd;
>> -            FD_ZERO(&wfds);
>> -            FD_ZERO(&efds);
>> -            FD_SET(fd, &wfds);
>> -            FD_SET(fd, &efds);
>> -            tv.tv_sec = 0;
>> -            tv.tv_usec = 100 * 1000;
>> -            ret = select(fd_max + 1, NULL, &wfds, &efds, &tv);
>> -            if (ret > 0 && (FD_ISSET(fd, &wfds) || FD_ISSET(fd, 
>> &efds)))
>> +            ret = poll(&p, 1, 100);
>> +            if (ret > 0)
>>                  break;
>>          }
>  
> Previously, this select checked the fd in both wfds and efds, should se 
> set POLLERR in the pollfd above to keep the same behaviour? (I'm not 
> off-hand sure if there's a concrete need for it.)

Not sure as well, still better safe than sorry ^^;

lu

-- 

Luca Barbato
Gentoo/linux
http://dev.gentoo.org/~lu_zero