Repository: bluenviron/gortsplib

Issue:
ntpTimeGoToRTCP is not compliant with RFC3550 (RTP)
If you look at RFC3550 section `6.4.1 SR: Sender Report RTCP Packet` and section `4 Byte Order, Alignment, and Time Format`, the NTP format is `The full NTP timestamp is a 64-bit unsigned fixed-point number with integer part in the first 32 bits and the fractional part in the 32 bits.`.

The function `ntpTimeGoToRTCP` is ok with the seconds part, but the fraction is bad.

`s % 1000000000` gives the number of nanoseconds, but that's not what the RFC says, NTP fractional seconds are in units of 1/2^32 seconds, not nanoseconds.

Because of that mistake, I cannot use `gst_rtcp_ntp_to_unix()` C function in my GStreamer pipeline (that take a RTSP stream from MediaMTX), I had to implement my own function to mitigate this error.

I'm not a Go developer, so with my current knowledge of Go and help of AI, it could be something like:

```
func ntpTimeGoToRTCP(v time.Time) uint64 {
    ntpTime := v.Unix() + 2208988800
    seconds := uint64(ntpTime)
    
    // Get fractional seconds from nanoseconds
    nanos := uint64(v.Nanosecond())
    
    // Use 64-bit arithmetic to avoid precision loss
    fractional := uint32((nanos * (1 << 32)) / 1000000000)
    
    return (seconds << 32) | uint64(fractional)
}
```

The division in `uint32((nanos * (1 << 32)) / 1000000000)` could eventually be rounded before casting to uint32.

Discussion:
@aler9: Thank you very much for reporting this issue, this is fixed by #870@tvidaldev: Thanks for the fix!
May I know when do you plan to release a new version of MediaMTX including this fix?

Solution patch:
diff --git a/pkg/ntp/ntp.go b/pkg/ntp/ntp.go
new file mode 100644
index 00000000..e226c117
--- /dev/null
+++ b/pkg/ntp/ntp.go
@@ -0,0 +1,24 @@
+// Package ntp contains functions to encode and decode timestamps to/from NTP format.
+package ntp
+
+import (
+	"math"
+	"time"
+)
+
+// Encode encodes a timestamp in NTP format.
+// Specification: RFC3550, section 4
+func Encode(t time.Time) uint64 {
+	ntp := uint64(t.UnixNano()) + 2208988800*1000000000
+	secs := ntp / 1000000000
+	fractional := uint64(math.Round(float64((ntp%1000000000)*(1<<32)) / 1000000000))
+	return secs<<32 | fractional
+}
+
+// Decode decodes a timestamp from NTP format.
+// Specification: RFC3550, section 4
+func Decode(v uint64) time.Time {
+	secs := int64((v >> 32) - 2208988800)
+	nanos := int64(math.Round(float64(((v & 0xFFFFFFFF) * 1000000000) / (1 << 32))))
+	return time.Unix(secs, nanos)
+}
diff --git a/pkg/rtcpreceiver/rtcpreceiver.go b/pkg/rtcpreceiver/rtcpreceiver.go
index b00b994c..9c9ec15f 100644
--- a/pkg/rtcpreceiver/rtcpreceiver.go
+++ b/pkg/rtcpreceiver/rtcpreceiver.go
@@ -7,17 +7,11 @@ import (
 	"sync"
 	"time"
 
+	"github.com/bluenviron/gortsplib/v4/pkg/ntp"
 	"github.com/pion/rtcp"
 	"github.com/pion/rtp"
 )
 
-// seconds since 1st January 1900
-// higher 32 bits are the integer part, lower 32 bits are the fractional part
-func ntpTimeRTCPToGo(v uint64) time.Time {
-	nano := int64((v>>32)*1000000000+(v&0xFFFFFFFF)) - 2208988800*1000000000
-	return time.Unix(0, nano)
-}
-
 func randUint32() (uint32, error) {
 	var b [4]byte
 	_, err := rand.Read(b[:])
@@ -418,7 +412,7 @@ func (rr *RTCPReceiver) packetNTPUnsafe(ts uint32) (time.Time, bool) {
 	timeDiff := int32(ts - rr.lastSenderReportTimeRTP)
 	timeDiffGo := (time.Duration(timeDiff) * time.Second) / time.Duration(rr.ClockRate)
 
-	return ntpTimeRTCPToGo(rr.lastSenderReportTimeNTP).Add(timeDiffGo), true
+	return ntp.Decode(rr.lastSenderReportTimeNTP).Add(timeDiffGo), true
 }
 
 // PacketNTP returns the NTP (absolute timestamp) of the packet.
diff --git a/pkg/rtcpsender/rtcpsender.go b/pkg/rtcpsender/rtcpsender.go
index 867b8c07..4f785f3d 100644
--- a/pkg/rtcpsender/rtcpsender.go
+++ b/pkg/rtcpsender/rtcpsender.go
@@ -5,17 +5,11 @@ import (
 	"sync"
 	"time"
 
+	"github.com/bluenviron/gortsplib/v4/pkg/ntp"
 	"github.com/pion/rtcp"
 	"github.com/pion/rtp"
 )
 
-// seconds since 1st January 1900
-// higher 32 bits are the integer part, lower 32 bits are the fractional part
-func ntpTimeGoToRTCP(v time.Time) uint64 {
-	s := uint64(v.UnixNano()) + 2208988800*1000000000
-	return (s/1000000000)<<32 | (s % 1000000000)
-}
-
 // RTCPSender is a utility to send RTP packets.
 // It is in charge of generating RTCP sender reports.
 type RTCPSender struct {
@@ -112,7 +106,7 @@ func (rs *RTCPSender) report() rtcp.Packet {
 
 	return &rtcp.SenderReport{
 		SSRC:        rs.localSSRC,
-		NTPTime:     ntpTimeGoToRTCP(ntpTime),
+		NTPTime:     ntp.Encode(ntpTime),
 		RTPTime:     rtpTime,
 		PacketCount: rs.packetCount,
 		OctetCount:  rs.octetCount,
diff --git a/server_conn.go b/server_conn.go
index a200f1d2..cab14c3a 100644
--- a/server_conn.go
+++ b/server_conn.go
@@ -19,6 +19,7 @@ import (
 	"github.com/bluenviron/gortsplib/v4/pkg/headers"
 	"github.com/bluenviron/gortsplib/v4/pkg/liberrors"
 	"github.com/bluenviron/gortsplib/v4/pkg/mikey"
+	"github.com/bluenviron/gortsplib/v4/pkg/ntp"
 )
 
 func getSessionID(header base.Header) string {
@@ -87,7 +88,7 @@ func mikeyGenerate(ctx *wrappedSRTPContext) (*mikey.Message, error) {
 	msg.Payloads = []mikey.Payload{
 		&mikey.PayloadT{
 			TSType:  0,
-			TSValue: mikeyEncodeTime(time.Now()),
+			TSValue: ntp.Encode(time.Now()),
 		},
 		&mikey.PayloadRAND{
 			Data: randData,
diff --git a/server_session.go b/server_session.go
index 7be6490a..8f192308 100644
--- a/server_session.go
+++ b/server_session.go
@@ -22,6 +22,7 @@ import (
 	"github.com/bluenviron/gortsplib/v4/pkg/headers"
 	"github.com/bluenviron/gortsplib/v4/pkg/liberrors"
 	"github.com/bluenviron/gortsplib/v4/pkg/mikey"
+	"github.com/bluenviron/gortsplib/v4/pkg/ntp"
 	"github.com/bluenviron/gortsplib/v4/pkg/rtcpreceiver"
 	"github.com/bluenviron/gortsplib/v4/pkg/rtcpsender"
 	"github.com/bluenviron/gortsplib/v4/pkg/rtptime"
@@ -201,21 +202,6 @@ func pickFirstSupportedTransport(s *Server, tsh headers.Transports) *headers.Tra
 	return nil
 }
 
-func mikeyDecodeTime(t uint64) time.Time {
-	sec := t >> 32
-	dec := t & 0xFFFFFFFF
-	sec -= 2208988800
-	return time.Unix(int64(sec), int64(dec))
-}
-
-func mikeyEncodeTime(n time.Time) uint64 {
-	nano := uint64(n.UnixNano())
-	sec := nano / 1000000000
-	dec := nano % 1000000000
-	sec += 2208988800
-	return sec<<32 | dec
-}
-
 func mikeyGetPayload[T mikey.Payload](mikeyMsg *mikey.Message) (T, bool) {
 	var zero T
 	for _, wrapped := range mikeyMsg.Payloads {
@@ -241,7 +227,7 @@ func mikeyToContext(mikeyMsg *mikey.Message) (*wrappedSRTPContext, error) {
 		return nil, fmt.Errorf("time payload not present")
 	}
 
-	ts := mikeyDecodeTime(timePayload.TSValue)
+	ts := ntp.Decode(timePayload.TSValue)
 	diff := time.Since(ts)
 	if diff < -time.Hour || diff > time.Hour {
 		return nil, fmt.Errorf("NTP difference is too high")



Test patch:
diff --git a/client_play_test.go b/client_play_test.go
index 1d37631b..78655669 100644
--- a/client_play_test.go
+++ b/client_play_test.go
@@ -24,6 +24,7 @@ import (
 	"github.com/bluenviron/gortsplib/v4/pkg/format"
 	"github.com/bluenviron/gortsplib/v4/pkg/headers"
 	"github.com/bluenviron/gortsplib/v4/pkg/mikey"
+	"github.com/bluenviron/gortsplib/v4/pkg/ntp"
 	"github.com/bluenviron/mediacommon/v2/pkg/codecs/mpeg4audio"
 )
 
@@ -2570,7 +2571,7 @@ func TestClientPlayRTCPReport(t *testing.T) {
 
 		_, err2 = l2.WriteTo(mustMarshalPacketRTCP(&rtcp.SenderReport{
 			SSRC:        753621,
-			NTPTime:     ntpTimeGoToRTCP(time.Date(2017, 8, 12, 15, 30, 0, 0, time.UTC)),
+			NTPTime:     ntp.Encode(time.Date(2017, 8, 12, 15, 30, 0, 0, time.UTC)),
 			RTPTime:     54352,
 			PacketCount: 1,
 			OctetCount:  4,
@@ -3581,7 +3582,7 @@ func TestClientPlayPacketNTP(t *testing.T) {
 
 		_, err2 = l2.WriteTo(mustMarshalPacketRTCP(&rtcp.SenderReport{
 			SSRC:        753621,
-			NTPTime:     ntpTimeGoToRTCP(time.Date(2017, 8, 12, 15, 30, 0, 0, time.UTC)),
+			NTPTime:     ntp.Encode(time.Date(2017, 8, 12, 15, 30, 0, 0, time.UTC)),
 			RTPTime:     54352,
 			PacketCount: 1,
 			OctetCount:  4,
diff --git a/client_record_test.go b/client_record_test.go
index d93ea9d1..cd64dc1c 100644
--- a/client_record_test.go
+++ b/client_record_test.go
@@ -21,6 +21,7 @@ import (
 	"github.com/bluenviron/gortsplib/v4/pkg/format"
 	"github.com/bluenviron/gortsplib/v4/pkg/headers"
 	"github.com/bluenviron/gortsplib/v4/pkg/mikey"
+	"github.com/bluenviron/gortsplib/v4/pkg/ntp"
 	"github.com/bluenviron/gortsplib/v4/pkg/sdp"
 )
 
@@ -69,11 +70,6 @@ var testRTCPPacket = rtcp.SourceDescription{
 
 var testRTCPPacketMarshaled = mustMarshalPacketRTCP(&testRTCPPacket)
 
-func ntpTimeGoToRTCP(v time.Time) uint64 {
-	s := uint64(v.UnixNano()) + 2208988800*1000000000
-	return (s/1000000000)<<32 | (s % 1000000000)
-}
-
 func record(c *Client, ur string, medias []*description.Media, cb func(*description.Media, rtcp.Packet)) error {
 	u, err := base.ParseURL(ur)
 	if err != nil {
@@ -1371,7 +1367,7 @@ func TestClientRecordRTCPReport(t *testing.T) {
 				require.Equal(t, []rtcp.Packet{
 					&rtcp.SenderReport{
 						SSRC:        packets[0].(*rtcp.SenderReport).SSRC,
-						NTPTime:     ntpTimeGoToRTCP(time.Date(1996, 2, 13, 14, 33, 5, 0, time.UTC)),
+						NTPTime:     ntp.Encode(time.Date(1996, 2, 13, 14, 33, 5, 0, time.UTC)),
 						RTPTime:     1300000 + 60*90000,
 						PacketCount: 1,
 						OctetCount:  1,
diff --git a/pkg/ntp/ntp_test.go b/pkg/ntp/ntp_test.go
new file mode 100644
index 00000000..cbabf3ce
--- /dev/null
+++ b/pkg/ntp/ntp_test.go
@@ -0,0 +1,43 @@
+package ntp
+
+import (
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+)
+
+var cases = []struct {
+	name string
+	dec  time.Time
+	enc  uint64
+}{
+	{
+		"a",
+		time.Date(2013, 4, 15, 11, 15, 17, 958404853, time.UTC).Local(),
+		15354565283395798332,
+	},
+	{
+		"b",
+		time.Date(2013, 4, 15, 11, 15, 18, 0, time.UTC).Local(),
+		15354565283574448128,
+	},
+}
+
+func TestEncode(t *testing.T) {
+	for _, ca := range cases {
+		t.Run(ca.name, func(t *testing.T) {
+			v := Encode(ca.dec)
+			require.Equal(t, ca.enc, v)
+		})
+	}
+}
+
+func TestDecode(t *testing.T) {
+	for _, ca := range cases {
+		t.Run(ca.name, func(t *testing.T) {
+			v := Decode(ca.enc)
+			require.Equal(t, ca.dec, v)
+		})
+	}
+}
diff --git a/server_play_test.go b/server_play_test.go
index 44ef4257..20a5412d 100644
--- a/server_play_test.go
+++ b/server_play_test.go
@@ -23,6 +23,7 @@ import (
 	"github.com/bluenviron/gortsplib/v4/pkg/format"
 	"github.com/bluenviron/gortsplib/v4/pkg/headers"
 	"github.com/bluenviron/gortsplib/v4/pkg/mikey"
+	"github.com/bluenviron/gortsplib/v4/pkg/ntp"
 	"github.com/bluenviron/gortsplib/v4/pkg/sdp"
 )
 
@@ -1526,7 +1527,7 @@ func TestServerPlayRTCPReport(t *testing.T) {
 			require.Equal(t, []rtcp.Packet{
 				&rtcp.SenderReport{
 					SSRC:        packets[0].(*rtcp.SenderReport).SSRC,
-					NTPTime:     ntpTimeGoToRTCP(time.Date(2017, 8, 10, 12, 22, 30, 0, time.UTC)),
+					NTPTime:     ntp.Encode(time.Date(2017, 8, 10, 12, 22, 30, 0, time.UTC)),
 					RTPTime:     240000 + 90000*30,
 					PacketCount: 1,
 					OctetCount:  1,
diff --git a/server_record_test.go b/server_record_test.go
index 6107dff3..af8a603b 100644
--- a/server_record_test.go
+++ b/server_record_test.go
@@ -20,6 +20,7 @@ import (
 	"github.com/bluenviron/gortsplib/v4/pkg/format"
 	"github.com/bluenviron/gortsplib/v4/pkg/headers"
 	"github.com/bluenviron/gortsplib/v4/pkg/mikey"
+	"github.com/bluenviron/gortsplib/v4/pkg/ntp"
 	"github.com/bluenviron/gortsplib/v4/pkg/sdp"
 )
 
@@ -1079,7 +1080,7 @@ func TestServerRecordRTCPReport(t *testing.T) {
 
 	_, err = l2.WriteTo(mustMarshalPacketRTCP(&rtcp.SenderReport{
 		SSRC:        753621,
-		NTPTime:     ntpTimeGoToRTCP(time.Date(2018, 2, 20, 19, 0, 0, 0, time.UTC)),
+		NTPTime:     ntp.Encode(time.Date(2018, 2, 20, 19, 0, 0, 0, time.UTC)),
 		RTPTime:     54352,
 		PacketCount: 1,
 		OctetCount:  4,
@@ -1682,7 +1683,7 @@ func TestServerRecordPacketNTP(t *testing.T) {
 
 	_, err = l2.WriteTo(mustMarshalPacketRTCP(&rtcp.SenderReport{
 		SSRC:        753621,
-		NTPTime:     ntpTimeGoToRTCP(time.Date(2018, 2, 20, 19, 0, 0, 0, time.UTC)),
+		NTPTime:     ntp.Encode(time.Date(2018, 2, 20, 19, 0, 0, 0, time.UTC)),
 		RTPTime:     54352,
 		PacketCount: 1,
 		OctetCount:  4,