Tolerate video frame rate variation in properties detector (pion#261)

Changes:
* Add argument to tolerate some FPS variations.
   * Update wrapper function.
   * Update tests.
* Add test about frame rate change tolerance.
   * Verify the onChange function is not called when the frame rate change
is within the the specified tolerance.
* Update test about frame rate variation detection
   * Create dedicated throttle transform function to slow down after a specific amount of time.
* Remove unnecessary code.

meta.go

@@ -14,7 +14,7 @@ func detectCurrentVideoProp(broadcaster *video.Broadcaster) (prop.Media, error)
 	// buffered frame or a new frame from the source. This also implies that no frame will be lost
 	// in any case.
 	metaReader := broadcaster.NewReader(false)
-	metaReader = video.DetectChanges(0, func(p prop.Media) { currentProp = p })(metaReader)
+	metaReader = video.DetectChanges(0, 0, func(p prop.Media) { currentProp = p })(metaReader)
 	_, _, err := metaReader.Read()
 
 	return currentProp, err

pkg/io/video/detect.go

@@ -2,14 +2,15 @@ package video
 
 import (
 	"image"
+	"math"
 	"time"
 
 	"github.com/pion/mediadevices/pkg/prop"
 )
 
 // DetectChanges will detect frame and video property changes. For video property detection,
 // since it's time related, interval will be used to determine the sample rate.
-func DetectChanges(interval time.Duration, onChange func(prop.Media)) TransformFunc {
+func DetectChanges(interval time.Duration, fpsDiffTolerance float64, onChange func(prop.Media)) TransformFunc {
 	return func(r Reader) Reader {
 		var currentProp prop.Media
 		var lastTaken time.Time
@@ -40,11 +41,12 @@ func DetectChanges(interval time.Duration, onChange func(prop.Media)) TransformF
 			elapsed := now.Sub(lastTaken)
 			if elapsed >= interval {
 				fps := float32(float64(frames) / elapsed.Seconds())
-				// TODO: maybe add some epsilon so that small changes will not mark as dirty
-				currentProp.FrameRate = fps
 				frames = 0
 				lastTaken = now
-				dirty = true
+				if math.Abs(float64(currentProp.FrameRate-fps)) > fpsDiffTolerance {
+					currentProp.FrameRate = fps
+					dirty = true
+				}
 			}
 
 			if dirty {

pkg/io/video/detect_test.go

@@ -28,7 +28,7 @@ func BenchmarkDetectChanges(b *testing.B) {
 		src := src
 		b.Run(fmt.Sprintf("WithDetectChanges%d", n), func(b *testing.B) {
 			for i := 0; i < n; i++ {
-				src = DetectChanges(time.Microsecond, func(p prop.Media) {})(src)
+				src = DetectChanges(time.Microsecond, 0, func(p prop.Media) {})(src)
 			}
 
 			for i := 0; i < b.N; i++ {
@@ -74,14 +74,35 @@ func TestDetectChanges(t *testing.T) {
 		}
 	}
 
+	SlowDownAfterThrottle := func(rate float32, factor float64, after time.Duration) TransformFunc {
+		return func(r Reader) Reader {
+			sleep := float64(time.Second) / float64(rate)
+			start := time.Now()
+			f := 1.0
+			return ReaderFunc(func() (image.Image, func(), error) {
+				for {
+					img, _, err := r.Read()
+					if err != nil {
+						return nil, func() {}, err
+					}
+					if time.Since(start) > after {
+						f = factor
+					}
+					time.Sleep(time.Duration(sleep * f))
+					return img, func() {}, nil
+				}
+			})
+		}
+	}
+
 	t.Run("OnChangeCalledBeforeFirstFrame", func(t *testing.T) {
 		var detectBeforeFirstFrame bool
 		var expected prop.Media
 		var actual prop.Media
 		expected.Width = 1920
 		expected.Height = 1080
 		src, _ := buildSource(expected)
-		src = DetectChanges(time.Second, func(p prop.Media) {
+		src = DetectChanges(time.Second, 0, func(p prop.Media) {
 			actual = p
 			detectBeforeFirstFrame = true
 		})(src)
@@ -104,7 +125,7 @@ func TestDetectChanges(t *testing.T) {
 		expected.Width = 1920
 		expected.Height = 1080
 		src, update := buildSource(expected)
-		src = DetectChanges(time.Second, func(p prop.Media) {
+		src = DetectChanges(time.Second, 0, func(p prop.Media) {
 			actual = p
 		})(src)
 
@@ -137,7 +158,7 @@ func TestDetectChanges(t *testing.T) {
 		expected.FrameRate = 30
 		src, _ := buildSource(expected)
 		src = Throttle(expected.FrameRate)(src)
-		src = DetectChanges(time.Second*5, func(p prop.Media) {
+		src = DetectChanges(time.Second*5, 0, func(p prop.Media) {
 			actual = p
 			count++
 		})(src)
@@ -155,4 +176,31 @@ func TestDetectChanges(t *testing.T) {
 			assertEq(t, actual, expected, frame, checkFrameRate)
 		}
 	})
+
+	t.Run("OnChangeNotCalledForToleratedFrameRateVariation", func(t *testing.T) {
+		// https://github.com/pion/mediadevices/issues/198
+		if runtime.GOOS == "darwin" {
+			t.Skip("Skipping because Darwin CI is not reliable for timing related tests.")
+		}
+
+		var expected prop.Media
+		var count int
+		expected.Width = 1920
+		expected.Height = 1080
+		expected.FrameRate = 30
+		src, _ := buildSource(expected)
+		src = SlowDownAfterThrottle(expected.FrameRate, 1.1, time.Second)(src)
+		src = DetectChanges(time.Second, 5, func(p prop.Media) {
+			count++
+		})(src)
+		for start := time.Now(); time.Since(start) < 3*time.Second; {
+			src.Read()
+		}
+		// onChange is called once before first frame: prop.FrameRate still 0.
+		// onChange is called again after receiving frames during the specified interval: prop.FrameRate is properly calculated
+		// So if the frame rate only changes within the specified tolerance, onChange should no longer be called.
+		if count > 2 {
+			t.Fatalf("onChange was called more than twice.")
+		}
+	})
 }