Opened 22 months ago

Closed 22 months ago

Last modified 21 months ago

#149 closed defect (fixed)

ntripcaster-2.0.37 UDP-based mountpoint time-out after RTP Sequence number overflow

Reported by: emiravalls Owned by: stoecker
Priority: major Component: Professional Caster
Version: Keywords:



We are using the BKG Ntrip Caster in the following setup:

Corrections Data Source [IPv4 / UDP / RTP / RTCM] -> BKG Caster [TCP / NTRIP] <-> Web Proxy [IPv4 / TCP / TLS / NTRIP] <-> BNC

What we found is that the following check in store_udp_data() at main.c:997:

if((int)(seq-con->udpbuffers->seq) > 0)

Is always false once the Corrections Data Source has sent more than 65535 messages, which is the maximum number of messages that can be sent without repeating sequence numbers in RTP (the sequence numbers wrap around 0). After some seconds, the mountpoint expires.

We believe the Caster should be able to support wrap around of sequence number for very long-lived mountpoints. Is that limit imposed in some standard?

The check could be changed to something like

choose a value between 1 and MAX_RTP_SEQNUM

if ((seq != con->udpbuffers->seq) && (((seq - con->udpbuffers->seq) & MAX_RTP_SEQNUM) <= MAX_PACKETS_IN_FLIGHT))

so that it would allow jumps of upto MAX_PACKETS_IN_FLIGHT RTP messages. We have found that this proposed check is similar to the ones performed in rtp_recieve_datagram_buffered() in rtp.c.

Given that the old check allowed jumps from 0 to 65535, MAX_PACKETS_IN_FLIGHT could be set upto to MAX_RTP_SEQNUM, but this would allow accepting packets potentially from the past if there is a very high rate of packets per second, which it didn't with the original check. Thus a threshold less than half the range seems reasonable (and it would be very similar to the checks in rtp.c).

We would like some feedback on this issue and the proposed solution.

Thank you,
Kind regards.

Attachments (0)

Change History (4)

comment:1 by stoecker, 22 months ago

Can you test following patch:

  • src/main.c

    997997static int store_udp_data(connection_t *con, unsigned char *buffer, int len, unsigned int seq)
    999999//printf("Store %d seq %d last %d ssrc %lu\n", len, seq, con->udpbuffers->seq, con->rtp->datagram->ssrc);
    1000   if((int)(seq-con->udpbuffers->seq) > 0)
     1000  /* handle 16 bit overflows, needs an upper limit in the following if clause */
     1001  unsigned int diff = seq < con->udpbuffers->seq ? 65536+seq-con->udpbuffers->seq : seq-con->udpbuffers->seq;
     1002  /* when 1000 blocks got missing essentially consider the stream broken or
     1003   * the overflow detection handling was wrong */
     1004  if(diff > 0 && diff < 1000)
    10011005  {
    10021006    con->udpbuffers->seq = seq;
    10031007    if(len)

It's essentially what you propose only with a little less magic code :-)

comment:2 by stoecker, 22 months ago

Resolution: fixed
Status: newclosed

In 9813/ntrip:

fix UDP isses, fix #149

comment:3 by emiravalls, 21 months ago

Agreed. We are testing your solution and we will confirm if it works for us, thank you!

One comment though, since the issue is the same, perhaps I would suggest to have the exact same computation implementation of "diff" in rtp.c:220 (either both using the modulo or the ternary operator) to avoid having two different implementations in the code. Sorry I didn't though of that from the beginning...

comment:4 by stoecker, 21 months ago

In principle I'd agree, but the code in rtp.c seems to cache out-of-order datagrams and reorder them which the plain UDP code doesn't try at all. So when I do changes there this would need intensive testing which I think isn't worth only for theoretical considerations. Especially as nearly nobody uses RTSP or UDP modes of Ntrip ;-)

Modify Ticket

Change Properties
as closed The owner will remain stoecker.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment

E-mail address and name can be saved in the Preferences .
Note: See TracTickets for help on using tickets.