[FFmpeg-devel] [RFC] rtpdec: Reordering RTP packets

[Martin Storsjö]

On Wed, 26 May 2010, Michael Niedermayer wrote:

> On Wed, May 26, 2010 at 12:19:55AM +0300, Martin Storsj? wrote:
> > On Tue, 25 May 2010, Michael Niedermayer wrote:
> > 
> > > very low delay is absolutely mandatory for some applications and refusing
> > > to return the correct packet if you have 2 packets is just insane.
> > 
> > Well, that's what the code currently does, too.
> > 
> > The code we've proposed up til now doesn't make the "minimum delay" case 
> > any different from what it does now, if the reordering is disabled.
> > 
> > > i dont object to a max_delay (in milli second or such)
> > > i object to the code returning the wrong packet of several
> > > that it has immedeately available if max_delay=0 or for example
> > > if there are 5 packets in the kernel buffer and max_delay=1 to then
> > > limit reordering to 1 packet (and drop 3 as they kernel buffer overflows
> > > till the next call of av_read_frame())
> > 
> > I tried implementing something like the thing I outlined above, and this 
> > should handle these cases where all the necessary packets are in kernel 
> > buffers but in incorrect orders.
> > 
> > That is, first we try to receive as many packets as we need to get the 
> > correct one using nonblocking reads. If the correct one isn't available at 
> > all, then we return the first one of the packets in the reorder queues 
> > when max_delay has expired.
> > 
> > The attached patches does this - they're untested, but try to show roughly 
> > how this could be implemented.
> [...]
> > @@ -1806,9 +1807,27 @@ static int rtsp_fetch_packet(AVFormatContext *s, AVPacket *pkt)
> >  #endif
> >      case RTSP_LOWER_TRANSPORT_UDP:
> >      case RTSP_LOWER_TRANSPORT_UDP_MULTICAST:
> > -        len = udp_read_packet(s, &rtsp_st, buf, sizeof(buf), 1);
> > +        len = udp_read_packet(s, &rtsp_st, buf, sizeof(buf), wait);
> >          if (len >=0 && rtsp_st->transport_priv && rt->transport == RTSP_TRANSPORT_RTP)
> >              rtp_check_and_send_back_rr(rtsp_st->transport_priv, len);
> > +        if (len == FF_NETERROR(ETIMEDOUT) && !wait) {
> > +            /* No more queued packets in the kernel buffers */
> > +            if (rt->transport == RTSP_TRANSPORT_RTP && s->max_delay <= 0) {
> > +                /* max_delay expired, try to flush one of the queued packets,
> 
> s->max_delay <= 0 and max_delay expired dont sound like they match
> or are you intending to decrease s->max_delay ?

The comment is a bit badly worded. I meant "max_delay has elapsed".

So if the correct packet wasn't found when reading nonblocking, wait for 
more packets up to s->max_delay, after that has elapsed (more precisely, 
after receiving the next packet after max_delay has elapsed, since I 
didn't write any code to abort waiting for packets after an exact time) 
just try to flush out the first packet from the queues, even if we haven't 
got the one we're looking for.

// Martin