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

[Martin Storsjö]

On Mon, 15 Nov 2010, Luca Barbato wrote:

> On 11/15/2010 03:18 PM, Martin Storsj? wrote:
> > 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.
> 
> The vla was the quickest way to get the feature w/out adding more lines,
> I wouldn't keep it at all.

Ok, good. :-)

> >> 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.
> 
> 0 is always for the tcp fd if present.

Yes, but that's not what I meant. In patch #1, you do this:

int j = 1 - (tcp_fd == -1);
[...] (code not touching j)
for (j = 1; j < max_p; j++)

In that case, the complicated initialization of j is just confusing, since 
the initial value isn't used at all. In patch #2, that is of course 
totally correct, but there, you also move the line. That is, if applied 
separately, this shouldn't be in patch #1. If both of them are applied at 
once, this point is moot.

> >>                      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?
> 
> Yes please

Ok, will do.

// Martin