Fix a bug in AbsCpatureTimeExtension

pion · Mar 15, 2024 · 410c582 · 410c582
1 parent 78da5a2
commit 410c582
Show file tree

Hide file tree

Showing 2 changed files with 89 additions and 33 deletions.
diff --git a/abscapturetimeextension.go b/abscapturetimeextension.go
@@ -66,7 +66,15 @@ func (t AbsCaptureTimeExtension) EstimatedCaptureClockOffsetDuration() *time.Dur
 		return nil
 	}
 	offset := *t.EstimatedCaptureClockOffset
+	negative := false
+	if offset < 0 {
+		offset = -offset
+		negative = true
+	}
 	duration := time.Duration(offset/(1<<32))*time.Second + time.Duration((offset&0xFFFFFFFF)*1e9/(1<<32))*time.Nanosecond
+	if negative {
+		duration = -duration
+	}
 	return &duration
 }
 
@@ -80,9 +88,17 @@ func NewAbsCaptureTimeExtension(captureTime time.Time) *AbsCaptureTimeExtension
 // NewAbsCaptureTimeExtensionWithCaptureClockOffset makes new AbsCaptureTimeExtension from time.Time and a clock offset.
 func NewAbsCaptureTimeExtensionWithCaptureClockOffset(captureTime time.Time, captureClockOffset time.Duration) *AbsCaptureTimeExtension {
 	ns := captureClockOffset.Nanoseconds()
+	negative := false
+	if ns < 0 {
+		ns = -ns
+		negative = true
+	}
 	lsb := (ns / 1e9) & 0xFFFFFFFF
 	msb := (((ns % 1e9) * (1 << 32)) / 1e9) & 0xFFFFFFFF
 	offset := (lsb << 32) | msb
+	if negative {
+		offset = -offset
+	}
 	return &AbsCaptureTimeExtension{
 		Timestamp:                   toNtpTime(captureTime),
 		EstimatedCaptureClockOffset: &offset,

diff --git a/abscapturetimeextension_test.go b/abscapturetimeextension_test.go
@@ -9,38 +9,78 @@ import (
 )
 
 func TestAbsCaptureTimeExtension_Roundtrip(t *testing.T) {
-	t0 := time.Now()
-	e1 := NewAbsCaptureTimeExtension(t0)
-	b1, err1 := e1.Marshal()
-	if err1 != nil {
-		t.Fatal(err1)
-	}
-	var o1 AbsCaptureTimeExtension
-	if err := o1.Unmarshal(b1); err != nil {
-		t.Fatal(err)
-	}
-	dt1 := o1.CaptureTime().Sub(t0).Seconds()
-	if dt1 < -0.001 || dt1 > 0.001 {
-		t.Fatalf("timestamp differs, want %v got %v (dt=%f)", t0, o1.CaptureTime(), dt1)
-	}
-	if o1.EstimatedCaptureClockOffsetDuration() != nil {
-		t.Fatalf("duration differs, want nil got %d", o1.EstimatedCaptureClockOffsetDuration())
-	}
+	t.Run("positive captureClockOffset", func(t *testing.T) {
+		t0 := time.Now()
+		e1 := NewAbsCaptureTimeExtension(t0)
+		b1, err1 := e1.Marshal()
+		if err1 != nil {
+			t.Fatal(err1)
+		}
+		var o1 AbsCaptureTimeExtension
+		if err := o1.Unmarshal(b1); err != nil {
+			t.Fatal(err)
+		}
+		dt1 := o1.CaptureTime().Sub(t0).Seconds()
+		if dt1 < -0.001 || dt1 > 0.001 {
+			t.Fatalf("timestamp differs, want %v got %v (dt=%f)", t0, o1.CaptureTime(), dt1)
+		}
+		if o1.EstimatedCaptureClockOffsetDuration() != nil {
+			t.Fatalf("duration differs, want nil got %d", o1.EstimatedCaptureClockOffsetDuration())
+		}
 
-	e2 := NewAbsCaptureTimeExtensionWithCaptureClockOffset(t0, 1250*time.Millisecond)
-	b2, err2 := e2.Marshal()
-	if err2 != nil {
-		t.Fatal(err2)
-	}
-	var o2 AbsCaptureTimeExtension
-	if err := o2.Unmarshal(b2); err != nil {
-		t.Fatal(err)
-	}
-	dt2 := o1.CaptureTime().Sub(t0).Seconds()
-	if dt2 < -0.001 || dt2 > 0.001 {
-		t.Fatalf("timestamp differs, want %v got %v (dt=%f)", t0, o2.CaptureTime(), dt2)
-	}
-	if *o2.EstimatedCaptureClockOffsetDuration() != 1250*time.Millisecond {
-		t.Fatalf("duration differs, want 250ms got %d", *o2.EstimatedCaptureClockOffsetDuration())
-	}
+		e2 := NewAbsCaptureTimeExtensionWithCaptureClockOffset(t0, 1250*time.Millisecond)
+		b2, err2 := e2.Marshal()
+		if err2 != nil {
+			t.Fatal(err2)
+		}
+		var o2 AbsCaptureTimeExtension
+		if err := o2.Unmarshal(b2); err != nil {
+			t.Fatal(err)
+		}
+		dt2 := o1.CaptureTime().Sub(t0).Seconds()
+		if dt2 < -0.001 || dt2 > 0.001 {
+			t.Fatalf("timestamp differs, want %v got %v (dt=%f)", t0, o2.CaptureTime(), dt2)
+		}
+		if *o2.EstimatedCaptureClockOffsetDuration() != 1250*time.Millisecond {
+			t.Fatalf("duration differs, want 250ms got %d", *o2.EstimatedCaptureClockOffsetDuration())
+		}
+	})
+
+	// This test can verify the for for the issue 247
+	t.Run("negative captureClockOffset", func(t *testing.T) {
+		t0 := time.Now()
+		e1 := NewAbsCaptureTimeExtension(t0)
+		b1, err1 := e1.Marshal()
+		if err1 != nil {
+			t.Fatal(err1)
+		}
+		var o1 AbsCaptureTimeExtension
+		if err := o1.Unmarshal(b1); err != nil {
+			t.Fatal(err)
+		}
+		dt1 := o1.CaptureTime().Sub(t0).Seconds()
+		if dt1 < -0.001 || dt1 > 0.001 {
+			t.Fatalf("timestamp differs, want %v got %v (dt=%f)", t0, o1.CaptureTime(), dt1)
+		}
+		if o1.EstimatedCaptureClockOffsetDuration() != nil {
+			t.Fatalf("duration differs, want nil got %d", o1.EstimatedCaptureClockOffsetDuration())
+		}
+
+		e2 := NewAbsCaptureTimeExtensionWithCaptureClockOffset(t0, -250*time.Millisecond)
+		b2, err2 := e2.Marshal()
+		if err2 != nil {
+			t.Fatal(err2)
+		}
+		var o2 AbsCaptureTimeExtension
+		if err := o2.Unmarshal(b2); err != nil {
+			t.Fatal(err)
+		}
+		dt2 := o1.CaptureTime().Sub(t0).Seconds()
+		if dt2 < -0.001 || dt2 > 0.001 {
+			t.Fatalf("timestamp differs, want %v got %v (dt=%f)", t0, o2.CaptureTime(), dt2)
+		}
+		if *o2.EstimatedCaptureClockOffsetDuration() != -250*time.Millisecond {
+			t.Fatalf("duration differs, want -250ms got %v", *o2.EstimatedCaptureClockOffsetDuration())
+		}
+	})
 }