[FFmpeg-devel] [PATCH 1/6] avformat/rtpdec: update the previous with new seq

[lance.lmwang at gmail.com]

On Wed, Oct 14, 2020 at 10:56:33PM +0300, Martin Storsjö wrote:
> On Wed, 14 Oct 2020, lance.lmwang at gmail.com wrote:
> 
> > From: Limin Wang <lance.lmwang at gmail.com>
> > 
> > If the rtp input is discontinued, we'll drop all the new received packets now,
> > It's better to update the previous seq with the new seq so that the following
> > packets will not be dropped always.
> > 
> > Signed-off-by: Limin Wang <lance.lmwang at gmail.com>
> > ---
> > libavformat/rtpdec.c | 1 +
> > 1 file changed, 1 insertion(+)
> > 
> > diff --git a/libavformat/rtpdec.c b/libavformat/rtpdec.c
> > index 3d5b200..de79536 100644
> > --- a/libavformat/rtpdec.c
> > +++ b/libavformat/rtpdec.c
> > @@ -833,6 +833,7 @@ static int rtp_parse_one_packet(RTPDemuxContext *s, AVPacket *pkt,
> >             /* Packet older than the previously emitted one, drop */
> >             av_log(s->ic, AV_LOG_WARNING,
> >                    "RTP: dropping old packet received too late\n");
> > +            s->seq = seq;
> 
> I'm not sure if this is better, depending on the usecase.
> 
> The existing logic tries to make sure that packets that have been skipped
> really aren't output, as that should make for a less disruptive data stream.
> 
> Consider if you're receiving packets in this order:
> 
> 0 1 2 3 6 7 8 9 10 11 12 13 14 15 16 4 5 17 18 19 ...
> 
> When receiving 0-3 everything runs fine. Packets 6 and onwards are enqueued.
> If the queue size limit hits before packets 4-5 arrive (say with a queue
> size limit of 5 packets), we start outputting packets 6, 7 etc to the
> parser/decoder. When packets 4 and 5 arrive late, they're skipped
> completely. With your patch, packet 4 would be skipped but 5 would be
> included.
> 
> Thus, I think this sequence is better:
> 
> 0 1 2 3 6 7 8 9 10 11 ...
> 
> Than this:
> 
> 0 1 2 3 6 7 8 9 4 5 10 11 ..
> 
> The first sequence has got one discontinuity, the other one has three
> discontinuities.
> 
> I do see your point that there's an issue if the the sequence jumps to a
> different point (over 32k packets ahead from the current numbrer), and it
> would be good to handle that in some way, but the suggested patch does
> regress the cases the existing code tries to handle.

yes, my use case is the rtp input is restarted, so the next packets are
discontinued and dropped which isn't expected behavior. I'll consider more
about the case your raised. 


> 
> // Martin
> 

-- 
Thanks,
Limin Wang