[FFmpeg-devel] [RFC] switch to poll()

[Martin Storsjö]

On Sun, 14 Nov 2010, Luca Barbato wrote:

> On 11/14/2010 07:00 PM, Luca Barbato wrote:
> > On 11/14/2010 06:35 PM, Ronald S. Bultje wrote:
> >> Hi,
> 
> The vla is still there for now, your patch is there.
> 
> Moving the array allocation up (on ff_sdp_parse I guess) is feasible,
> only that we use that poller just in the udp case. So I'd like to think
> a bit more regarding that.

I don't think mallocing a small array for the poll structs is a problem 
even if we wouldn't use it - it's a much smaller issue in practice than 
having VLA's in the code, I'd say.

In general it looks quite good to me, especially after getting patch #2 to 
clean up the rtsp stuff. For that patch, perhaps it would be good with a 
comment saying that care must be taken to keep j in sync with the poll 
structs.


> diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
> index 1f55016..77d2873 100644
> --- a/libavformat/rtsp.c
> +++ b/libavformat/rtsp.c
>              for (i = 0; i < rt->nb_rtsp_streams; i++) {
>                  rtsp_st = rt->rtsp_streams[i];
>                  if (rtsp_st->rtp_handle) {
> +                    int j = 1 - (tcp_fd == -1);
>                      fd = url_get_file_handle(rtsp_st->rtp_handle);

The initialization of j here, when it will be reinitialized in the for loop below,
feels strange. I do see what you do with it in patch #2, though, but it isn't
necessary here yet.

>                      fd_rtcp = rtp_get_rtcp_file_handle(rtsp_st->rtp_handle);
> -                    if (FD_ISSET(fd_rtcp, &rfds) || FD_ISSET(fd, &rfds)) {
> +                    for(j = 1; j<max_p; j++) {
> +                    if ((p[j].revents & POLLIN) &&
> +                        (p[j].fd == fd_rtcp || p[j].fd == fd)) {
>                          ret = url_read(rtsp_st->rtp_handle, buf, buf_size);
>                          if (ret > 0) {
>                              *prtsp_st = rtsp_st;
>                              return ret;
>                          }
>                      }
> +                    }
>                  }
>              }


Do you want me to test run some of the modified codepaths that you haven't 
tested yourself, e.g. SAP?

// Martin