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

[Martin Storsjö]

On Tue, 18 May 2010, Ronald S. Bultje wrote:

> On May 18, 2010, at 8:17 PM, Michael Niedermayer <michaelni at gmx.at>  
> wrote:
> > On Tue, May 18, 2010 at 11:37:57PM +0300, Martin Storsj? wrote:
> >> On Tue, 18 May 2010, Luca Barbato wrote:
> >>
> >>>>> Default reorder queue size - the idea was to move the decision  
> >>>>> on the
> >>>>> queue size out of rtpdec, so that it could be dynamically  
> >>>>> choosable.
> >>>>> For now there's no such code, but the decision on the queue size  
> >>>>> is
> >>>>> outside of rtpdec at least.
> >>>>>
> >>>>> This perhaps still could be a define (where should it be in that  
> >>>>> case,
> >>>>> rtpdec.h?) even if it isn't hardcoded as a static array in  
> >>>>> RTPDemuxContext
> >>>>> as in the previous attempt.
> >>>>
> >>>> Changed to a define now.
> >>>
> >>> I'd consider it as rtp proto param, but I know will be annoying
> >>> forwarding it from rtsp://path/to/resource?buffer=10&tcp to it but  
> >>> might
> >>> be something nice for downstream users. Otherwise use the  
> >>> AVOption...
> >>
> >> Yes, Ronald suggested something such, too. Although, I see that as a
> >> separate feature that can be added later, after the general  
> >> reordering
> >> support within rtpdec...
> >
> > iam scared
> >
> > the user cares about 1 thing and that is delay, not packet number  
> > which is
> > just a meaningless number, unfit to be constant by default
> 
> Or size in bytes (more from a security/resource/admin point of view),  

Usually, the RTP packets have a fixed max size, and as long as the max 
queue size is capped by a fixed number of packets (in addition to a max 
delay given in time), I don't see this as such a big threat from a 
resource usage point of view. If the source sends huge packets, that will 
force the whole pipeline to allocate more resources. The queue just uses a 
bit more resources, scaling linearly with the increase in packet size.

> but yes I think I agree with Michael here. We should inplement both  
> and nr of packets isn't terribly useful then...

Yes, a max delay would be useful. To implement it properly, it needs code 
in two places, though:

Within rtpdec, which checks the timestamp range of packets in the queue, 
before choosing whether to forcibly output the first one in the queue, 
regardless of if we're waiting for a possibly dropped packet.

Within the rtsp demuxer. Say we receive a packet once a second, but have a 
max reordering delay of 50 ms. If we miss one packet and receive the next 
one, it will be kept within the reorder queue until the next received 
packet (one second later), since the rtpdec code doesn't have a chance to 
flush it from the queue until it gets called next time, when the next 
packet is received. This would require additional code within 
udp_read_packet, to call rtp_parse_packet with a NULL buffer to forcibly 
get the first one from the queue if there is one, if we've been waiting 
longer than (max reordering delay) for the next packet.

It gets even worse if we're receiving two streams, one which gets packets 
very often, and another one that gets very seldom. Then we have to keep 
track that we haven't gotten any packets on one stream for a while, trying 
to flush packets from its reordering queue.


Does this sound necessary, or overly complicated? If this really is a 
problem, the user at least should be able to disable the reordering, 
either as a number of packets or as a delay in ms.

// Martin