[MPlayer-dev-eng] [PATCH] RTP Reorder - fix out-of-sequence packets in rtp streaming

[Nico Sabbi]

ErniTron wrote:

>
> Thanks for your review. Reply in short:
>
> 1) Yes, indentation is broken. But not that broken if you use ts=8 in 
> vim. No problem to fix it in next version.
>
> 2) When packet is in sequence behaviour of patched code is exactly the 
> same of old code in terms of buffer copy. Look at the old code:
>
> int read_rtp_from_server(int fd, char *buffer, ...)
> ...
>         getrtp2(fd, &rh, &data, &len);
> ...
>         memcpy(buffer, data, len);
>
> in fact getrtp2 has it's own static buf[1600] and rtp_read_from_server 
> makes a copy of every packets to the caller buffer. My code does 
> exactly the same when packets are in order. I agree with you that the 
> copy can be saved, but I didn't dare to touch getrtp2 since it's a 
> very old function that belongs to the original Dave Chapman dvbstream 
> code and you can find it identical in other applications (rtpdump, 
> etc...). I just stick to the new functionality.
>
> 3) For the sake of readability I wouldn't substitute repeated code 
> with block and goto. But that's probably a matter of personal taste.
>
> 4) Why -3? It's a sort of heuristic threshold. I prefer to drop the 
> very first packet and keep 4 more free slots in order to fill with new 
> packets in sequence instead of dropping them while waiting for old 
> packet to come.
>
> Let me know if you agree with my considerations and I will proceed 
> with a new revised patch.
>
> Thanks again.
>
> Erni
>

this one contains your code reformatted with tabs (as most of teh rest 
of rtp.c) and with
a small simplification in rtp_cache(); overall it's identical to your 
first patch.
You were right about getrtp2(): it's scary (it even calls exit()) and it 
should not be modified
by this patch; rather as a separate commit.

The patch looks ok to me; I'm going to commit it in the next few days if 
no one objects.

    Nico
-------------- next part --------------
A non-text attachment was scrubbed...
Name: rtpreorder2.diff
Type: text/x-patch
Size: 5885 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20060617/1763153b/attachment.bin>