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

[Martin Storsjö]

On Wed, 11 Aug 2010, Ronald S. Bultje wrote:

> On Tue, May 25, 2010 at 6:33 PM, Martin Storsj? <martin at martin.st> wrote:
> > 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.
> 
> That sounds OK to me. Is this still being worked on?

I haven't worked much on this in favour of other things lately, but I'll 
try to look at this again in a while.

// Martin