From 29df4b34930832456e917401a9c303a75016c376 Mon Sep 17 00:00:00 2001
From: Josh Allmann <joshua.allmann@gmail.com>
Date: Sat, 14 Mar 2020 23:49:32 +0000
Subject: [PATCH 1/9] ffmpeg: Add tests for first and last PTS values.

This helps ensure that changes to sensitive areas that munge pts
such as the filtergraph continue to produce the correct results.
---
 ffmpeg/nvidia_test.go | 130 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 129 insertions(+), 1 deletion(-)

diff --git a/ffmpeg/nvidia_test.go b/ffmpeg/nvidia_test.go
index b1a91c430e..f736f5716a 100644
--- a/ffmpeg/nvidia_test.go
+++ b/ffmpeg/nvidia_test.go
@@ -510,6 +510,7 @@ func TestNvidia_CountEncodedFrames(t *testing.T) {
 	run(cmd)
 
 	tc := NewTranscoder()
+	defer tc.StopTranscoder()
 
 	// Test decoding
 	for i := 0; i < 4; i++ {
@@ -550,7 +551,134 @@ func TestNvidia_CountEncodedFrames(t *testing.T) {
 			t.Error(in.Fname, " Mismatched frame count: expected 240 got ", res.Encoded[2].Frames)
 		}
 	}
-	tc.StopTranscoder()
+
+	// Check timestamps of the results we just got.
+	// A bit brute force but another layer of checking
+	// First output the expected PTS - first and last 3 frames, per segment
+	//  (hardcoded below)
+	// Second calculate the same set of PTS for transcoded results
+	// Then take the diff of the two. Should match.
+
+	// Write expected PTS to a file for diff'ing
+	//  (Calculated by running the same routine below)
+	cmd = `
+  cat << EOF > expected_pts.out
+==> out_120fps_0.ts.pts <==
+pkt_pts=129000
+pkt_pts=129750
+pkt_pts=130500
+pkt_pts=306750
+pkt_pts=307500
+pkt_pts=308250
+
+==> out_120fps_1.ts.pts <==
+pkt_pts=309000
+pkt_pts=309750
+pkt_pts=310500
+pkt_pts=486750
+pkt_pts=487500
+pkt_pts=488250
+
+==> out_120fps_2.ts.pts <==
+pkt_pts=489000
+pkt_pts=489750
+pkt_pts=490500
+pkt_pts=666750
+pkt_pts=667500
+pkt_pts=668250
+
+==> out_120fps_3.ts.pts <==
+pkt_pts=669000
+pkt_pts=669750
+pkt_pts=670500
+pkt_pts=846750
+pkt_pts=847500
+pkt_pts=848250
+
+==> out_30fps_0.ts.pts <==
+pkt_pts=129000
+pkt_pts=132000
+pkt_pts=135000
+pkt_pts=300000
+pkt_pts=303000
+pkt_pts=306000
+
+==> out_30fps_1.ts.pts <==
+pkt_pts=309000
+pkt_pts=312000
+pkt_pts=315000
+pkt_pts=480000
+pkt_pts=483000
+pkt_pts=486000
+
+==> out_30fps_2.ts.pts <==
+pkt_pts=489000
+pkt_pts=492000
+pkt_pts=495000
+pkt_pts=660000
+pkt_pts=663000
+pkt_pts=666000
+
+==> out_30fps_3.ts.pts <==
+pkt_pts=669000
+pkt_pts=672000
+pkt_pts=675000
+pkt_pts=840000
+pkt_pts=843000
+pkt_pts=846000
+
+==> out_60fps_0.ts.pts <==
+pkt_pts=129000
+pkt_pts=130500
+pkt_pts=132000
+pkt_pts=304500
+pkt_pts=306000
+pkt_pts=307500
+
+==> out_60fps_1.ts.pts <==
+pkt_pts=309000
+pkt_pts=310500
+pkt_pts=312000
+pkt_pts=484500
+pkt_pts=486000
+pkt_pts=487500
+
+==> out_60fps_2.ts.pts <==
+pkt_pts=489000
+pkt_pts=490500
+pkt_pts=492000
+pkt_pts=664500
+pkt_pts=666000
+pkt_pts=667500
+
+==> out_60fps_3.ts.pts <==
+pkt_pts=669000
+pkt_pts=670500
+pkt_pts=672000
+pkt_pts=844500
+pkt_pts=846000
+pkt_pts=847500
+EOF
+  `
+	run(cmd)
+
+	// Calculate first and last 3 frame PTS for each output, and compare
+	cmd = `
+    # First and last 3 frame PTS for each output
+    FILES=out_*.ts
+
+    for f in  $FILES
+    do
+      ffprobe -loglevel warning -select_streams v -show_frames $f | grep pkt_pts= | head -3 > $f.pts
+      ffprobe -loglevel warning -select_streams v -show_frames $f | grep pkt_pts= | tail -3 >> $f.pts
+    done
+    tail -n +1 out_*.ts.pts > transcoded_pts.out
+
+    # Do the comparison!
+    diff -u expected_pts.out transcoded_pts.out
+  `
+	run(cmd)
+
 }
 
 func TestNvidia_RepeatedSpecialOpts(t *testing.T) {

From 899eb77da8f5f1040ef9a2cf8a2d2ecc1666bd77 Mon Sep 17 00:00:00 2001
From: Josh Allmann <joshua.allmann@gmail.com>
Date: Sun, 15 Mar 2020 01:06:36 +0000
Subject: [PATCH 2/9] ffmpeg: Check both software and Nvidia frame counts.

More sanity checking.
---
 ffmpeg/nvidia_test.go | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/ffmpeg/nvidia_test.go b/ffmpeg/nvidia_test.go
index f736f5716a..a34ed20af5 100644
--- a/ffmpeg/nvidia_test.go
+++ b/ffmpeg/nvidia_test.go
@@ -495,7 +495,12 @@ func TestNvidia_CountFrames(t *testing.T) {
 	tc.StopTranscoder()
 }
 
-func TestNvidia_CountEncodedFrames(t *testing.T) {
+func TestNvidia_API_CountEncodedFrames(t *testing.T) {
+	countEncodedFrames(t, Software)
+	countEncodedFrames(t, Nvidia)
+}
+
+func countEncodedFrames(t *testing.T, accel Acceleration) {
 	run, dir := setupTest(t)
 	defer os.RemoveAll(dir)
 
@@ -512,12 +517,12 @@ func TestNvidia_CountEncodedFrames(t *testing.T) {
 	tc := NewTranscoder()
 	defer tc.StopTranscoder()
 
-	// Test decoding
+	// Test encoding
 	for i := 0; i < 4; i++ {
 		in := &TranscodeOptionsIn{
 			Fname:  fmt.Sprintf("%s/test%d.ts", dir, i),
-			Accel:  Nvidia,
-			Device: "3",
+			Accel:  accel,
+			Device: "0",
 		}
 		p60fps := P144p30fps16x9
 		p60fps.Framerate = 60
@@ -526,15 +531,15 @@ func TestNvidia_CountEncodedFrames(t *testing.T) {
 		out := []TranscodeOptions{{
 			Oname:   fmt.Sprintf("%s/out_30fps_%d.ts", dir, i),
 			Profile: P144p30fps16x9,
-			Accel:   Nvidia,
+			Accel:   accel,
 		}, {
 			Oname:   fmt.Sprintf("%s/out_60fps_%d.ts", dir, i),
 			Profile: p60fps,
-			Accel:   Nvidia,
+			Accel:   accel,
 		}, {
 			Oname:   fmt.Sprintf("%s/out_120fps_%d.ts", dir, i),
 			Profile: p120fps,
-			Accel:   Nvidia,
+			Accel:   accel,
 		}}
 
 		res, err := tc.Transcode(in, out)

From f134b9ebf4e08d83dde41fa77f9548a483302b90 Mon Sep 17 00:00:00 2001
From: Josh Allmann <joshua.allmann@gmail.com>
Date: Mon, 30 Mar 2020 21:43:51 +0000
Subject: [PATCH 3/9] ffmpeg: Move encoded frame count into API tests for sw.

This is a useful test and we should always run it for software.
---
 ffmpeg/api_test.go    | 192 ++++++++++++++++++++++++++++++++++++++++++
 ffmpeg/nvidia_test.go | 189 +----------------------------------------
 2 files changed, 193 insertions(+), 188 deletions(-)

diff --git a/ffmpeg/api_test.go b/ffmpeg/api_test.go
index c4a8a197fd..463f79841e 100644
--- a/ffmpeg/api_test.go
+++ b/ffmpeg/api_test.go
@@ -1,6 +1,8 @@
 package ffmpeg
 
 import (
+	"fmt"
+	"os"
 	"testing"
 )
 
@@ -88,3 +90,193 @@ func TestTranscoderAPI_TooManyOutputs(t *testing.T) {
 		t.Error("Expected 'Too many outputs', got ", err)
 	}
 }
+
+func countEncodedFrames(t *testing.T, accel Acceleration) {
+	run, dir := setupTest(t)
+	defer os.RemoveAll(dir)
+
+	cmd := `
+    # run segmenter and sanity check frame counts . Hardcode for now.
+    ffmpeg -loglevel warning -i "$1"/../transcoder/test.ts -c:a copy -c:v copy -f hls test.m3u8
+    ffprobe -loglevel warning -select_streams v -count_frames -show_streams test0.ts | grep nb_read_frames=120
+    ffprobe -loglevel warning -select_streams v -count_frames -show_streams test1.ts | grep nb_read_frames=120
+    ffprobe -loglevel warning -select_streams v -count_frames -show_streams test2.ts | grep nb_read_frames=120
+    ffprobe -loglevel warning -select_streams v -count_frames -show_streams test3.ts | grep nb_read_frames=120
+  `
+	run(cmd)
+
+	tc := NewTranscoder()
+	defer tc.StopTranscoder()
+
+	// Test encoding
+	for i := 0; i < 4; i++ {
+		in := &TranscodeOptionsIn{
+			Fname:  fmt.Sprintf("%s/test%d.ts", dir, i),
+			Accel:  accel,
+			Device: "0",
+		}
+		p60fps := P144p30fps16x9
+		p60fps.Framerate = 60
+		p120fps := P144p30fps16x9
+		p120fps.Framerate = 120
+		out := []TranscodeOptions{{
+			Oname:   fmt.Sprintf("%s/out_30fps_%d.ts", dir, i),
+			Profile: P144p30fps16x9,
+			Accel:   accel,
+		}, {
+			Oname:   fmt.Sprintf("%s/out_60fps_%d.ts", dir, i),
+			Profile: p60fps,
+			Accel:   accel,
+		}, {
+			Oname:   fmt.Sprintf("%s/out_120fps_%d.ts", dir, i),
+			Profile: p120fps,
+			Accel:   accel,
+		}}
+
+		res, err := tc.Transcode(in, out)
+		if err != nil {
+			t.Error(err)
+		}
+		if res.Encoded[0].Frames != 60 {
+			t.Error(in.Fname, " Mismatched frame count: expected 60 got ", res.Encoded[0].Frames)
+		}
+		if res.Encoded[1].Frames != 120 {
+			t.Error(in.Fname, " Mismatched frame count: expected 120 got ", res.Encoded[1].Frames)
+		}
+		if res.Encoded[2].Frames != 240 {
+			t.Error(in.Fname, " Mismatched frame count: expected 240 got ", res.Encoded[2].Frames)
+		}
+	}
+
+	// Check timestamps of the results we just got.
+	// A bit brute force but another layer of checking
+	// First output the expected PTS - first and last 3 frames, per segment
+	//  (hardcoded below)
+	// Second calculate the same set of PTS for transcoded results
+	// Then take the diff of the two. Should match.
+
+	// Write expected PTS to a file for diff'ing
+	//  (Calculated by running the same routine below)
+	cmd = `
+  cat << EOF > expected_pts.out
+==> out_120fps_0.ts.pts <==
+pkt_pts=129000
+pkt_pts=129750
+pkt_pts=130500
+pkt_pts=306750
+pkt_pts=307500
+pkt_pts=308250
+
+==> out_120fps_1.ts.pts <==
+pkt_pts=309000
+pkt_pts=309750
+pkt_pts=310500
+pkt_pts=486750
+pkt_pts=487500
+pkt_pts=488250
+
+==> out_120fps_2.ts.pts <==
+pkt_pts=489000
+pkt_pts=489750
+pkt_pts=490500
+pkt_pts=666750
+pkt_pts=667500
+pkt_pts=668250
+
+==> out_120fps_3.ts.pts <==
+pkt_pts=669000
+pkt_pts=669750
+pkt_pts=670500
+pkt_pts=846750
+pkt_pts=847500
+pkt_pts=848250
+
+==> out_30fps_0.ts.pts <==
+pkt_pts=129000
+pkt_pts=132000
+pkt_pts=135000
+pkt_pts=300000
+pkt_pts=303000
+pkt_pts=306000
+
+==> out_30fps_1.ts.pts <==
+pkt_pts=309000
+pkt_pts=312000
+pkt_pts=315000
+pkt_pts=480000
+pkt_pts=483000
+pkt_pts=486000
+
+==> out_30fps_2.ts.pts <==
+pkt_pts=489000
+pkt_pts=492000
+pkt_pts=495000
+pkt_pts=660000
+pkt_pts=663000
+pkt_pts=666000
+
+==> out_30fps_3.ts.pts <==
+pkt_pts=669000
+pkt_pts=672000
+pkt_pts=675000
+pkt_pts=840000
+pkt_pts=843000
+pkt_pts=846000
+
+==> out_60fps_0.ts.pts <==
+pkt_pts=129000
+pkt_pts=130500
+pkt_pts=132000
+pkt_pts=304500
+pkt_pts=306000
+pkt_pts=307500
+
+==> out_60fps_1.ts.pts <==
+pkt_pts=309000
+pkt_pts=310500
+pkt_pts=312000
+pkt_pts=484500
+pkt_pts=486000
+pkt_pts=487500
+
+==> out_60fps_2.ts.pts <==
+pkt_pts=489000
+pkt_pts=490500
+pkt_pts=492000
+pkt_pts=664500
+pkt_pts=666000
+pkt_pts=667500
+
+==> out_60fps_3.ts.pts <==
+pkt_pts=669000
+pkt_pts=670500
+pkt_pts=672000
+pkt_pts=844500
+pkt_pts=846000
+pkt_pts=847500
+EOF
+  `
+	run(cmd)
+
+	// Calculate first and last 3 frame PTS for each output, and compare
+	cmd = `
+    # First and last 3 frame PTS for each output
+    FILES=out_*.ts
+
+    for f in  $FILES
+    do
+      ffprobe -loglevel warning -select_streams v -show_frames $f | grep pkt_pts= | head -3 > $f.pts
+      ffprobe -loglevel warning -select_streams v -show_frames $f | grep pkt_pts= | tail -3 >> $f.pts
+    done
+    tail -n +1 out_*.ts.pts > transcoded_pts.out
+
+    # Do the comparison!
+    diff -u expected_pts.out transcoded_pts.out
+  `
+	run(cmd)
+
+}
+
+func TestTranscoderAPI_CountEncodedFrames(t *testing.T) {
+	countEncodedFrames(t, Software)
+}
diff --git a/ffmpeg/nvidia_test.go b/ffmpeg/nvidia_test.go
index a34ed20af5..28423309ec 100644
--- a/ffmpeg/nvidia_test.go
+++ b/ffmpeg/nvidia_test.go
@@ -495,197 +495,10 @@ func TestNvidia_CountFrames(t *testing.T) {
 	tc.StopTranscoder()
 }
 
-func TestNvidia_API_CountEncodedFrames(t *testing.T) {
-	countEncodedFrames(t, Software)
+func TestNvidia_CountEncodedFrames(t *testing.T) {
 	countEncodedFrames(t, Nvidia)
 }
 
-func countEncodedFrames(t *testing.T, accel Acceleration) {
-	run, dir := setupTest(t)
-	defer os.RemoveAll(dir)
-
-	cmd := `
-    # run segmenter and sanity check frame counts . Hardcode for now.
-    ffmpeg -loglevel warning -i "$1"/../transcoder/test.ts -c:a copy -c:v copy -f hls test.m3u8
-    ffprobe -loglevel warning -select_streams v -count_frames -show_streams test0.ts | grep nb_read_frames=120
-    ffprobe -loglevel warning -select_streams v -count_frames -show_streams test1.ts | grep nb_read_frames=120
-    ffprobe -loglevel warning -select_streams v -count_frames -show_streams test2.ts | grep nb_read_frames=120
-    ffprobe -loglevel warning -select_streams v -count_frames -show_streams test3.ts | grep nb_read_frames=120
-  `
-	run(cmd)
-
-	tc := NewTranscoder()
-	defer tc.StopTranscoder()
-
-	// Test encoding
-	for i := 0; i < 4; i++ {
-		in := &TranscodeOptionsIn{
-			Fname:  fmt.Sprintf("%s/test%d.ts", dir, i),
-			Accel:  accel,
-			Device: "0",
-		}
-		p60fps := P144p30fps16x9
-		p60fps.Framerate = 60
-		p120fps := P144p30fps16x9
-		p120fps.Framerate = 120
-		out := []TranscodeOptions{{
-			Oname:   fmt.Sprintf("%s/out_30fps_%d.ts", dir, i),
-			Profile: P144p30fps16x9,
-			Accel:   accel,
-		}, {
-			Oname:   fmt.Sprintf("%s/out_60fps_%d.ts", dir, i),
-			Profile: p60fps,
-			Accel:   accel,
-		}, {
-			Oname:   fmt.Sprintf("%s/out_120fps_%d.ts", dir, i),
-			Profile: p120fps,
-			Accel:   accel,
-		}}
-
-		res, err := tc.Transcode(in, out)
-		if err != nil {
-			t.Error(err)
-		}
-		if res.Encoded[0].Frames != 60 {
-			t.Error(in.Fname, " Mismatched frame count: expected 60 got ", res.Encoded[0].Frames)
-		}
-		if res.Encoded[1].Frames != 120 {
-			t.Error(in.Fname, " Mismatched frame count: expected 120 got ", res.Encoded[1].Frames)
-		}
-		if res.Encoded[2].Frames != 240 {
-			t.Error(in.Fname, " Mismatched frame count: expected 240 got ", res.Encoded[2].Frames)
-		}
-	}
-
-	// Check timestamps of the results we just got.
-	// A bit brute force but another layer of checking
-	// First output the expected PTS - first and last 3 frames, per segment
-	//  (hardcoded below)
-	// Second calculate the same set of PTS for transcoded results
-	// Then take the diff of the two. Should match.
-
-	// Write expected PTS to a file for diff'ing
-	//  (Calculated by running the same routine below)
-	cmd = `
-  cat << EOF > expected_pts.out
-==> out_120fps_0.ts.pts <==
-pkt_pts=129000
-pkt_pts=129750
-pkt_pts=130500
-pkt_pts=306750
-pkt_pts=307500
-pkt_pts=308250
-
-==> out_120fps_1.ts.pts <==
-pkt_pts=309000
-pkt_pts=309750
-pkt_pts=310500
-pkt_pts=486750
-pkt_pts=487500
-pkt_pts=488250
-
-==> out_120fps_2.ts.pts <==
-pkt_pts=489000
-pkt_pts=489750
-pkt_pts=490500
-pkt_pts=666750
-pkt_pts=667500
-pkt_pts=668250
-
-==> out_120fps_3.ts.pts <==
-pkt_pts=669000
-pkt_pts=669750
-pkt_pts=670500
-pkt_pts=846750
-pkt_pts=847500
-pkt_pts=848250
-
-==> out_30fps_0.ts.pts <==
-pkt_pts=129000
-pkt_pts=132000
-pkt_pts=135000
-pkt_pts=300000
-pkt_pts=303000
-pkt_pts=306000
-
-==> out_30fps_1.ts.pts <==
-pkt_pts=309000
-pkt_pts=312000
-pkt_pts=315000
-pkt_pts=480000
-pkt_pts=483000
-pkt_pts=486000
-
-==> out_30fps_2.ts.pts <==
-pkt_pts=489000
-pkt_pts=492000
-pkt_pts=495000
-pkt_pts=660000
-pkt_pts=663000
-pkt_pts=666000
-
-==> out_30fps_3.ts.pts <==
-pkt_pts=669000
-pkt_pts=672000
-pkt_pts=675000
-pkt_pts=840000
-pkt_pts=843000
-pkt_pts=846000
-
-==> out_60fps_0.ts.pts <==
-pkt_pts=129000
-pkt_pts=130500
-pkt_pts=132000
-pkt_pts=304500
-pkt_pts=306000
-pkt_pts=307500
-
-==> out_60fps_1.ts.pts <==
-pkt_pts=309000
-pkt_pts=310500
-pkt_pts=312000
-pkt_pts=484500
-pkt_pts=486000
-pkt_pts=487500
-
-==> out_60fps_2.ts.pts <==
-pkt_pts=489000
-pkt_pts=490500
-pkt_pts=492000
-pkt_pts=664500
-pkt_pts=666000
-pkt_pts=667500
-
-==> out_60fps_3.ts.pts <==
-pkt_pts=669000
-pkt_pts=670500
-pkt_pts=672000
-pkt_pts=844500
-pkt_pts=846000
-pkt_pts=847500
-EOF
-  `
-	run(cmd)
-
-	// Calculate first and last 3 frame PTS for each output, and compare
-	cmd = `
-    # First and last 3 frame PTS for each output
-    FILES=out_*.ts
-
-    for f in  $FILES
-    do
-      ffprobe -loglevel warning -select_streams v -show_frames $f | grep pkt_pts= | head -3 > $f.pts
-      ffprobe -loglevel warning -select_streams v -show_frames $f | grep pkt_pts= | tail -3 >> $f.pts
-    done
-    tail -n +1 out_*.ts.pts > transcoded_pts.out
-
-    # Do the comparison!
-    diff -u expected_pts.out transcoded_pts.out
-  `
-	run(cmd)
-
-}
-
 func TestNvidia_RepeatedSpecialOpts(t *testing.T) {
 
 	_, dir := setupTest(t)

From 6d66e5b293d98b2b56aa02a368758588db114103 Mon Sep 17 00:00:00 2001
From: Josh Allmann <joshua.allmann@gmail.com>
Date: Wed, 1 Apr 2020 23:40:45 +0000
Subject: [PATCH 4/9] ffmpeg: Add pts checks for passthrough.

---
 ffmpeg/api_test.go | 41 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/ffmpeg/api_test.go b/ffmpeg/api_test.go
index 463f79841e..833458c1f4 100644
--- a/ffmpeg/api_test.go
+++ b/ffmpeg/api_test.go
@@ -119,6 +119,8 @@ func countEncodedFrames(t *testing.T, accel Acceleration) {
 		p60fps.Framerate = 60
 		p120fps := P144p30fps16x9
 		p120fps.Framerate = 120
+		passthru := P144p30fps16x9
+		passthru.Framerate = 0
 		out := []TranscodeOptions{{
 			Oname:   fmt.Sprintf("%s/out_30fps_%d.ts", dir, i),
 			Profile: P144p30fps16x9,
@@ -131,6 +133,10 @@ func countEncodedFrames(t *testing.T, accel Acceleration) {
 			Oname:   fmt.Sprintf("%s/out_120fps_%d.ts", dir, i),
 			Profile: p120fps,
 			Accel:   accel,
+		}, {
+			Oname:   fmt.Sprintf("%s/out_passthru_%d.ts", dir, i),
+			Profile: passthru,
+			Accel:   accel,
 		}}
 
 		res, err := tc.Transcode(in, out)
@@ -146,6 +152,9 @@ func countEncodedFrames(t *testing.T, accel Acceleration) {
 		if res.Encoded[2].Frames != 240 {
 			t.Error(in.Fname, " Mismatched frame count: expected 240 got ", res.Encoded[2].Frames)
 		}
+		if res.Encoded[3].Frames != 120 {
+			t.Error(in.Fname, " Mismatched frame count: expected 120 got ", res.Encoded[3].Frames)
+		}
 	}
 
 	// Check timestamps of the results we just got.
@@ -254,6 +263,38 @@ pkt_pts=672000
 pkt_pts=844500
 pkt_pts=846000
 pkt_pts=847500
+
+==> out_passthru_0.ts.pts <==
+pkt_pts=128970
+pkt_pts=130500
+pkt_pts=131940
+pkt_pts=304470
+pkt_pts=305910
+pkt_pts=307440
+
+==> out_passthru_1.ts.pts <==
+pkt_pts=308970
+pkt_pts=310500
+pkt_pts=311940
+pkt_pts=484470
+pkt_pts=485910
+pkt_pts=487440
+
+==> out_passthru_2.ts.pts <==
+pkt_pts=488970
+pkt_pts=490410
+pkt_pts=491940
+pkt_pts=664470
+pkt_pts=665910
+pkt_pts=667440
+
+==> out_passthru_3.ts.pts <==
+pkt_pts=668970
+pkt_pts=670500
+pkt_pts=671940
+pkt_pts=844470
+pkt_pts=845910
+pkt_pts=847440
 EOF
   `
 	run(cmd)

From e00624ca4b3499cc9b92adf0afce38194b0c7d73 Mon Sep 17 00:00:00 2001
From: Josh Allmann <joshua.allmann@gmail.com>
Date: Thu, 5 Mar 2020 23:30:56 +0000
Subject: [PATCH 5/9] ffmpeg: Persist filtergraph in between transcodes.

Re-initializing the filtergraph turns out to be expensive,
especially if GPUs are in the picture. We get around 30% more
performance by persisting the filtergraph.

In order to flush the filter, we cache the most recent audio/video
frame and feed those into the filter repeatedly with a sentinel
value set (`AVFrame.opque`). Once we receive a frame from the
filtergraph with the sentinel set, we know the filter has been
completely flushed.
---
 ffmpeg/lpms_ffmpeg.c | 86 +++++++++++++++++++++++++++++++++-----------
 1 file changed, 65 insertions(+), 21 deletions(-)

diff --git a/ffmpeg/lpms_ffmpeg.c b/ffmpeg/lpms_ffmpeg.c
index 686f001191..6bb39ad909 100644
--- a/ffmpeg/lpms_ffmpeg.c
+++ b/ffmpeg/lpms_ffmpeg.c
@@ -9,8 +9,6 @@
 #include <libavutil/opt.h>
 #include <libavutil/pixdesc.h>
 
-#include <pthread.h>
-
 // Not great to appropriate internal API like this...
 const int lpms_ERR_INPUT_PIXFMT = FFERRTAG('I','N','P','X');
 const int lpms_ERR_FILTERS = FFERRTAG('F','L','T','R');
@@ -32,11 +30,12 @@ struct input_ctx {
   enum AVHWDeviceType hw_type;
   char *device;
 
-  int64_t next_pts_a, next_pts_v;
-
   // Decoder flush
   AVPacket *first_pkt;
   int flushed;
+
+  // Filter flush
+  AVFrame *last_frame_v, *last_frame_a;
 };
 
 struct filter_ctx {
@@ -47,6 +46,8 @@ struct filter_ctx {
   AVFilterContext *src_ctx;
 
   uint8_t *hwframes; // GPU frame pool data
+  int64_t flush_offset;
+  int flushed, flush_count;
 };
 
 struct output_ctx {
@@ -215,13 +216,14 @@ static void close_output(struct output_ctx *octx)
   }
   if (octx->vc && AV_HWDEVICE_TYPE_NONE == octx->hw_type) avcodec_free_context(&octx->vc);
   if (octx->ac) avcodec_free_context(&octx->ac);
-  free_filter(&octx->vf);
-  free_filter(&octx->af);
+  octx->af.flushed = octx->vf.flushed = 0;
 }
 
 static void free_output(struct output_ctx *octx) {
   close_output(octx);
   if (octx->vc) avcodec_free_context(&octx->vc);
+  free_filter(&octx->vf);
+  free_filter(&octx->af);
 }
 
 
@@ -748,6 +750,8 @@ static void free_input(struct input_ctx *inctx)
   }
   if (inctx->ac) avcodec_free_context(&inctx->ac);
   if (inctx->hw_device_ctx) av_buffer_unref(&inctx->hw_device_ctx);
+  if (inctx->last_frame_v) av_frame_free(&inctx->last_frame_v);
+  if (inctx->last_frame_a) av_frame_free(&inctx->last_frame_a);
 }
 
 static int open_video_decoder(input_params *params, struct input_ctx *ctx)
@@ -867,6 +871,10 @@ static int open_input(input_params *params, struct input_ctx *ctx)
   if (ret < 0) dd_err("Unable to open video decoder\n")
   ret = open_audio_decoder(params, ctx);
   if (ret < 0) dd_err("Unable to open audio decoder\n")
+  ctx->last_frame_v = av_frame_alloc();
+  if (!ctx->last_frame_v) dd_err("Unable to alloc last_frame_v");
+  ctx->last_frame_a = av_frame_alloc();
+  if (!ctx->last_frame_a) dd_err("Unable to alloc last_frame_a");
 
   return 0;
 
@@ -1037,6 +1045,7 @@ int process_out(struct input_ctx *ictx, struct output_ctx *octx, AVCodecContext
   goto proc_cleanup; \
 }
   int ret = 0;
+  int is_flushing = 0;
 
   if (!encoder) proc_err("Trying to transmux; not supported")
 
@@ -1055,15 +1064,24 @@ int process_out(struct input_ctx *ictx, struct output_ctx *octx, AVCodecContext
     ret = init_video_filters(ictx, octx);
     if (ret < 0) return lpms_ERR_FILTERS;
   }
+  // Start filter flushing process if necessary
+  if (!inf && !filter->flushed) {
+    // Set input frame to the last frame
+    // And increment pts offset by pkt_duration
+    // TODO It may make sense to use the expected output packet duration instead
+    int is_video = AVMEDIA_TYPE_VIDEO == ost->codecpar->codec_type;
+    AVFrame *frame = is_video ? ictx->last_frame_v : ictx->last_frame_a;
+    filter->flush_offset += frame->pkt_duration;
+    inf = frame;
+    inf->opaque = (void*)inf->pts; // value doesn't matter; just needs to be set
+    is_flushing = 1;
+  }
   if (inf) {
+    // Apply the offset from filter flushing, then reset for the next output
+    inf->pts += filter->flush_offset;
     ret = av_buffersrc_write_frame(filter->src_ctx, inf);
+    inf->pts -= filter->flush_offset;
     if (ret < 0) proc_err("Error feeding the filtergraph");
-  } else {
-    // We need to set the pts at EOF to the *end* of the last packet
-    // in order to avoid discarding any queued packets
-    int64_t next_pts = AVMEDIA_TYPE_VIDEO == ost->codecpar->codec_type ?
-      ictx->next_pts_v : ictx->next_pts_a;
-    av_buffersrc_close(filter->src_ctx, next_pts, AV_BUFFERSRC_FLAG_PUSH);
   }
 
   while (1) {
@@ -1078,6 +1096,28 @@ int process_out(struct input_ctx *ictx, struct output_ctx *octx, AVCodecContext
       if (inf) return ret;
       frame = NULL;
     } else if (ret < 0) proc_err("Error consuming the filtergraph\n");
+    if (frame && frame->opaque) {
+      // opaque being set means it's a flush packet
+
+      // When using the fps filter, the timebase of the filtergraph is
+      // 1/framerate, so each frame output by the filter has its pts
+      // incremented by one. However, if the fps filter is *not* used
+      // (eg, in frame passthrough mode) then the pts is passed through
+      // the filtergraph untouched. In this case, the pts will be offset
+      // by the accumulated duration of any flush packets (and the input
+      // timebase *should* match the output timebase, so we can add
+      // without any rescaling).
+      // If any of these no longer hold (eg, due to future changes) then some
+      // unit tests should fail, and this approach needs to be re-evaluated.
+      if (octx->fps.den) filter->flush_count += 1;
+      else filter->flush_count += frame->pkt_duration;
+
+      // don't set flushed flag in case this is a flush from a previous segment
+      if (is_flushing) filter->flushed = 1;
+      frame->opaque = NULL; // reset just to be sure
+      continue;
+    }
+    if (frame) frame->pts -= filter->flush_count;
     ret = encode(encoder, frame, octx, ost);
     av_frame_unref(frame);
     // For HW we keep the encoder open so will only get EAGAIN.
@@ -1179,8 +1219,6 @@ int transcode(struct transcode_thread *h,
       if (octx->vc) {
         ret = add_video_stream(octx, ictx);
         if (ret < 0) main_err("Unable to re-add video stream\n");
-        ret = init_video_filters(ictx, octx);
-        if (ret < 0) main_err("Unable to re-open video filter\n")
       } else fprintf(stderr, "no video stream\n");
 
       // re-attach audio encoder
@@ -1202,6 +1240,7 @@ int transcode(struct transcode_thread *h,
   while (1) {
     int has_frame = 0;
     AVStream *ist = NULL;
+    AVFrame *last_frame = NULL;
     av_frame_unref(dframe);
     ret = process_in(ictx, dframe, &ipkt);
     if (ret == AVERROR_EOF) break;
@@ -1216,20 +1255,25 @@ int transcode(struct transcode_thread *h,
       // width / height will be zero for pure streamcopy (no decoding)
       decoded_results->frames += dframe->width && dframe->height;
       decoded_results->pixels += dframe->width * dframe->height;
+      has_frame = has_frame && dframe->width && dframe->height;
+      if (has_frame) last_frame = ictx->last_frame_v;
+    } else if (AVMEDIA_TYPE_AUDIO == ist->codecpar->codec_type) {
+      has_frame = has_frame && dframe->nb_samples;
+      if (has_frame) last_frame = ictx->last_frame_a;
+    }
       if (has_frame) {
         int64_t dur = 0;
         if (dframe->pkt_duration) dur = dframe->pkt_duration;
-        else if (ist->avg_frame_rate.den) {
-          dur = av_rescale_q(1, av_inv_q(ist->avg_frame_rate), ist->time_base);
+        else if (ist->r_frame_rate.den) {
+          dur = av_rescale_q(1, av_inv_q(ist->r_frame_rate), ist->time_base);
         } else {
           // TODO use better heuristics for this; look at how ffmpeg does it
-          //fprintf(stderr, "Could not determine next pts; filter might drop\n");
+          fprintf(stderr, "Could not determine next pts; filter might drop\n");
         }
-        ictx->next_pts_v = dframe->pts + dur;
+        dframe->pkt_duration = dur;
+        av_frame_unref(last_frame);
+        av_frame_ref(last_frame, dframe);
       }
-    } else if (AVMEDIA_TYPE_AUDIO == ist->codecpar->codec_type) {
-      if (has_frame) ictx->next_pts_a = dframe->pts + dframe->pkt_duration;
-    }
 
     for (i = 0; i < nb_outputs; i++) {
       struct output_ctx *octx = &outputs[i];

From bd99819c6871fee109648ecefd14860f34b9ebc2 Mon Sep 17 00:00:00 2001
From: Josh Allmann <joshua.allmann@gmail.com>
Date: Sun, 22 Mar 2020 22:09:48 +0000
Subject: [PATCH 6/9] ffmpeg: Disallow out-of-order segments.

This is a consequence of persisting the fps filter which expects
pts timestamps to be monotonic. Non-monotonic frames are dropped.

Accommodating out-of-order segments would involve totally
rewriting output timestamps based on the input value from the
source frame, while forging the pts that the filter receives.
---
 ffmpeg/api_test.go      | 82 +++++++++++++++++++++++++++++++++++++++++
 ffmpeg/ffmpeg_errors.go |  1 +
 ffmpeg/lpms_ffmpeg.c    | 33 ++++++++++++++++-
 ffmpeg/lpms_ffmpeg.h    |  1 +
 ffmpeg/nvidia_test.go   | 14 +++++--
 5 files changed, 126 insertions(+), 5 deletions(-)

diff --git a/ffmpeg/api_test.go b/ffmpeg/api_test.go
index 833458c1f4..b654aa46c4 100644
--- a/ffmpeg/api_test.go
+++ b/ffmpeg/api_test.go
@@ -321,3 +321,85 @@ EOF
 func TestTranscoderAPI_CountEncodedFrames(t *testing.T) {
 	countEncodedFrames(t, Software)
 }
+
+func TestTranscoder_API_AlternatingTimestamps(t *testing.T) {
+	// Really should refactor this test to increase commonality with other
+	// tests that also check things like SSIM, MD5 hashes, etc...
+	// See TestNvidia_API_MixedOutput / TestTranscoder_EncoderOpts / TestTranscoder_StreamCopy / TestNvidia_API_AlternatingTimestamps
+	run, dir := setupTest(t)
+	err := RTMPToHLS("../transcoder/test.ts", dir+"/out.m3u8", dir+"/out_%d.ts", "2", 0)
+	if err != nil {
+		t.Error(err)
+	}
+
+	profile := P144p30fps16x9
+	profile.Framerate = 123
+	tc := NewTranscoder()
+	defer tc.StopTranscoder()
+	idx := []int{1, 0, 3, 2}
+	for _, i := range idx {
+		in := &TranscodeOptionsIn{Fname: fmt.Sprintf("%s/out_%d.ts", dir, i)}
+		out := []TranscodeOptions{{
+			Oname:        fmt.Sprintf("%s/%d.md5", dir, i),
+			AudioEncoder: ComponentOptions{Name: "drop"},
+			VideoEncoder: ComponentOptions{Name: "copy"},
+			Muxer:        ComponentOptions{Name: "md5"},
+		}, {
+			Oname:        fmt.Sprintf("%s/sw_%d.ts", dir, i),
+			Profile:      profile,
+			AudioEncoder: ComponentOptions{Name: "copy"},
+		}, {
+			Oname:   fmt.Sprintf("%s/sw_audio_encode_%d.ts", dir, i),
+			Profile: profile,
+		}}
+		res, err := tc.Transcode(in, out)
+		if (i == 1 || i == 3) && err != nil {
+			t.Error(err)
+		}
+		if i == 0 || i == 2 {
+			if err == nil || err.Error() != "Segment out of order" {
+				t.Error(err)
+			}
+			// Maybe one day we'll be able to run the rest of this test
+			continue
+		}
+		if res.Decoded.Frames != 120 {
+			t.Error("Did not get decoded frames", res.Decoded.Frames)
+		}
+		if res.Encoded[1].Frames != res.Encoded[2].Frames {
+			t.Error("Mismatched frame count for hw/nv")
+		}
+	}
+	cmd := `
+    function check {
+
+      # Check md5sum for stream copy / drop
+      ffmpeg -loglevel warning -i out_$1.ts -an -c:v copy -f md5 ffmpeg_$1.md5
+      diff -u $1.md5 ffmpeg_$1.md5
+
+      ffmpeg -loglevel warning -i out_$1.ts -c:a aac -ar 44100 -ac 2 \
+        -vf fps=123,scale=w=256:h=144 -c:v libx264 ffmpeg_sw_$1.ts
+
+      # sanity check ffmpeg frame count against ours
+      ffprobe -count_frames -show_streams -select_streams v ffmpeg_sw_$1.ts | grep nb_read_frames=246
+      ffprobe -count_frames -show_streams -select_streams v sw_$1.ts | grep nb_read_frames=246
+      ffprobe -count_frames -show_streams -select_streams v sw_audio_encode_$1.ts | grep nb_read_frames=246
+
+    # check image quality
+    ffmpeg -loglevel warning -i sw_$1.ts -i ffmpeg_sw_$1.ts \
+      -lavfi "[0:v][1:v]ssim=sw_stats_$1.log" -f null -
+    grep -Po 'All:\K\d+.\d+' sw_stats_$1.log | \
+      awk '{ if ($1 < 0.95) count=count+1 } END{ exit count > 5 }'
+
+    # Really should check relevant audio as well...
+    }
+
+
+    # re-enable for seg 0 and 1 when alternating timestamps can be handled
+    # check 0
+    check 1
+    # check 2
+    check 3
+  `
+	run(cmd)
+}
diff --git a/ffmpeg/ffmpeg_errors.go b/ffmpeg/ffmpeg_errors.go
index a3a61e74d6..f592a65035 100644
--- a/ffmpeg/ffmpeg_errors.go
+++ b/ffmpeg/ffmpeg_errors.go
@@ -36,6 +36,7 @@ func error_map() map[int]error {
 		{code: C.lpms_ERR_INPUT_PIXFMT, desc: "Unsupported input pixel format"},
 		{code: C.lpms_ERR_FILTERS, desc: "Error initializing filtergraph"},
 		{code: C.lpms_ERR_OUTPUTS, desc: "Too many outputs"},
+		{code: C.lpms_ERR_DTS, desc: "Segment out of order"},
 	}
 	for _, v := range lpmsErrors {
 		m[int(v.code)] = errors.New(v.desc)
diff --git a/ffmpeg/lpms_ffmpeg.c b/ffmpeg/lpms_ffmpeg.c
index 6bb39ad909..dff0b91a9a 100644
--- a/ffmpeg/lpms_ffmpeg.c
+++ b/ffmpeg/lpms_ffmpeg.c
@@ -14,6 +14,7 @@ const int lpms_ERR_INPUT_PIXFMT = FFERRTAG('I','N','P','X');
 const int lpms_ERR_FILTERS = FFERRTAG('F','L','T','R');
 const int lpms_ERR_PACKET_ONLY = FFERRTAG('P','K','O','N');
 const int lpms_ERR_OUTPUTS = FFERRTAG('O','U','T','P');
+const int lpms_ERR_DTS = FFERRTAG('-','D','T','S');
 
 //
 // Internal transcoder data structures
@@ -226,6 +227,15 @@ static void free_output(struct output_ctx *octx) {
   free_filter(&octx->af);
 }
 
+static void flush_input(struct input_ctx *ictx) {
+  AVPacket pkt = {0};
+  av_init_packet(&pkt);
+  while (ictx->ic->pb) {
+    int ret = av_read_frame(ictx->ic, &pkt);
+    av_packet_unref(&pkt);
+    if (ret) break;
+  }
+}
 
 static int is_copy(char *encoder) {
   return encoder && !strcmp("copy", encoder);
@@ -912,9 +922,22 @@ int process_in(struct input_ctx *ictx, AVFrame *frame, AVPacket *pkt)
     else if (pkt->stream_index == ictx->vi || pkt->stream_index == ictx->ai) break;
     else dec_err("Could not find decoder or stream\n");
 
-    if (!ictx->first_pkt && pkt->flags & AV_PKT_FLAG_KEY && decoder == ictx->vc) {
+    if (!ictx->first_pkt && pkt->flags & AV_PKT_FLAG_KEY) {
+      // This would compare dts for every packet in an audio-only stream.
+      // Probably okay for now, since DTS really shouldn't go backwards anyway.
+      AVFrame *last_frame = NULL;
+      if (decoder == ictx->vc) {
       ictx->first_pkt = av_packet_clone(pkt);
       ictx->first_pkt->pts = -1;
+        last_frame = ictx->last_frame_v;
+      } else {
+        last_frame = ictx->last_frame_a;
+      }
+      if (AV_NOPTS_VALUE != last_frame->pkt_dts &&
+          pkt->dts <= last_frame->pkt_dts) {
+        ret = lpms_ERR_DTS;
+        dec_err("decoder: Segment out of order\n");
+      }
     }
 
     ret = avcodec_send_packet(decoder, pkt);
@@ -1168,7 +1191,7 @@ int transcode(struct transcode_thread *h,
   struct input_ctx *ictx = &h->ictx;
   struct output_ctx *outputs = h->outputs;
   int nb_outputs = h->nb_outputs;
-  AVPacket ipkt;
+  AVPacket ipkt = {0};
   AVFrame *dframe = NULL;
 
   if (!inp) main_err("transcoder: Missing input params\n")
@@ -1327,9 +1350,15 @@ int transcode(struct transcode_thread *h,
   }
 
 transcode_cleanup:
+  // Flush input then close input. Reads entire input, pretty inefficient.
+  // Necessary because the input context has data buffered that would get
+  // read out on the next segment if we have to exit early, eg seg out of order.
+  // TODO Is short-circuiting possible? Currently, closing + flushing segfaults
+  if (!ictx->flushed) flush_input(ictx);
   avio_closep(&ictx->ic->pb);
   if (dframe) av_frame_free(&dframe);
   ictx->flushed = 0;
+  av_packet_unref(&ipkt);  // needed for early exits
   if (ictx->first_pkt) av_packet_free(&ictx->first_pkt);
   if (ictx->ac) avcodec_free_context(&ictx->ac);
   if (ictx->vc && AV_HWDEVICE_TYPE_NONE == ictx->hw_type) avcodec_free_context(&ictx->vc);
diff --git a/ffmpeg/lpms_ffmpeg.h b/ffmpeg/lpms_ffmpeg.h
index 350eaeefee..fc0bd069d7 100644
--- a/ffmpeg/lpms_ffmpeg.h
+++ b/ffmpeg/lpms_ffmpeg.h
@@ -8,6 +8,7 @@
 extern const int lpms_ERR_INPUT_PIXFMT;
 extern const int lpms_ERR_FILTERS;
 extern const int lpms_ERR_OUTPUTS;
+extern const int lpms_ERR_DTS;
 
 struct transcode_thread;
 
diff --git a/ffmpeg/nvidia_test.go b/ffmpeg/nvidia_test.go
index 28423309ec..a15e17eafe 100644
--- a/ffmpeg/nvidia_test.go
+++ b/ffmpeg/nvidia_test.go
@@ -648,9 +648,16 @@ func TestNvidia_API_AlternatingTimestamps(t *testing.T) {
 			Profile: profile,
 		}}
 		res, err := tc.Transcode(in, out)
-		if err != nil {
+		if (i == 1 || i == 3) && err != nil {
 			t.Error(err)
 		}
+		if i == 0 || i == 2 {
+			if err == nil || err.Error() != "Segment out of order" {
+				t.Error(err)
+			}
+			// Maybe one day we'll be able to run the rest of this test
+			continue
+		}
 		if res.Decoded.Frames != 120 {
 			t.Error("Did not get decoded frames", res.Decoded.Frames)
 		}
@@ -690,9 +697,10 @@ func TestNvidia_API_AlternatingTimestamps(t *testing.T) {
     }
 
 
-    check 0
+    # re-enable for seg 0 and 1 when alternating timestamps can be handled
+    # check 0
     check 1
-    check 2
+    # check 2
     check 3
   `
 	run(cmd)

From e53a49826f1390d333d9a69923b4a1f84f68cbd3 Mon Sep 17 00:00:00 2001
From: Josh Allmann <joshua.allmann@gmail.com>
Date: Sun, 22 Mar 2020 23:12:10 +0000
Subject: [PATCH 7/9] ffmpeg: Log improvements and cosmetics.

---
 ffmpeg/lpms_ffmpeg.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/ffmpeg/lpms_ffmpeg.c b/ffmpeg/lpms_ffmpeg.c
index dff0b91a9a..8b411b65c8 100644
--- a/ffmpeg/lpms_ffmpeg.c
+++ b/ffmpeg/lpms_ffmpeg.c
@@ -901,7 +901,7 @@ int process_in(struct input_ctx *ictx, AVFrame *frame, AVPacket *pkt)
 {
 #define dec_err(msg) { \
   if (!ret) ret = -1; \
-  fprintf(stderr, msg); \
+  fprintf(stderr, "dec_cleanup: "msg); \
   goto dec_cleanup; \
 }
   int ret = 0;
@@ -927,8 +927,8 @@ int process_in(struct input_ctx *ictx, AVFrame *frame, AVPacket *pkt)
       // Probably okay for now, since DTS really shouldn't go backwards anyway.
       AVFrame *last_frame = NULL;
       if (decoder == ictx->vc) {
-      ictx->first_pkt = av_packet_clone(pkt);
-      ictx->first_pkt->pts = -1;
+        ictx->first_pkt = av_packet_clone(pkt);
+        ictx->first_pkt->pts = -1;
         last_frame = ictx->last_frame_v;
       } else {
         last_frame = ictx->last_frame_a;
@@ -1284,19 +1284,19 @@ int transcode(struct transcode_thread *h,
       has_frame = has_frame && dframe->nb_samples;
       if (has_frame) last_frame = ictx->last_frame_a;
     }
-      if (has_frame) {
-        int64_t dur = 0;
-        if (dframe->pkt_duration) dur = dframe->pkt_duration;
-        else if (ist->r_frame_rate.den) {
-          dur = av_rescale_q(1, av_inv_q(ist->r_frame_rate), ist->time_base);
-        } else {
-          // TODO use better heuristics for this; look at how ffmpeg does it
-          fprintf(stderr, "Could not determine next pts; filter might drop\n");
-        }
-        dframe->pkt_duration = dur;
-        av_frame_unref(last_frame);
-        av_frame_ref(last_frame, dframe);
+    if (has_frame) {
+      int64_t dur = 0;
+      if (dframe->pkt_duration) dur = dframe->pkt_duration;
+      else if (ist->r_frame_rate.den) {
+        dur = av_rescale_q(1, av_inv_q(ist->r_frame_rate), ist->time_base);
+      } else {
+        // TODO use better heuristics for this; look at how ffmpeg does it
+        fprintf(stderr, "Could not determine next pts; filter might drop\n");
       }
+      dframe->pkt_duration = dur;
+      av_frame_unref(last_frame);
+      av_frame_ref(last_frame, dframe);
+    }
 
     for (i = 0; i < nb_outputs; i++) {
       struct output_ctx *octx = &outputs[i];

From 316e214d947110abf3dc26354da2a7852872ae87 Mon Sep 17 00:00:00 2001
From: Josh Allmann <joshua.allmann@gmail.com>
Date: Sun, 22 Mar 2020 23:12:33 +0000
Subject: [PATCH 8/9] ffmpeg: Documentation updates.

---
 ffmpeg/lpms_ffmpeg.c | 55 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/ffmpeg/lpms_ffmpeg.c b/ffmpeg/lpms_ffmpeg.c
index 8b411b65c8..182a9d94e1 100644
--- a/ffmpeg/lpms_ffmpeg.c
+++ b/ffmpeg/lpms_ffmpeg.c
@@ -16,6 +16,46 @@ const int lpms_ERR_PACKET_ONLY = FFERRTAG('P','K','O','N');
 const int lpms_ERR_OUTPUTS = FFERRTAG('O','U','T','P');
 const int lpms_ERR_DTS = FFERRTAG('-','D','T','S');
 
+//
+//  Notes on transcoder internals:
+//
+//  Transcoding follows the typical process of the FFmpeg API:
+//    read/demux/decode/filter/encode/mux/write
+//
+//  This is done over discrete segments. However, decode/filter/encoder are
+//  expensive to re-initialize for every segment. We work around this by
+//  persisting these components across segments.
+//
+//  The challenge with persistence is there is often internal data that is
+//  buffered, and there isn't an explicit API to flush or drain that data
+//  short of re-initializing the component. This is addressed for each component
+//  as follows:
+//
+//  Decoder: For audio, we pay the price of closing and re-opening the decoder.
+//           For video, we cache the first packet we read (input_ctx.first_pkt).
+//           The pts is set to a sentinel value and fed to the decoder. Once we
+//           receive a frame from the decoder with the pts set to the sentinel
+//           value, then we know the decoder has been fully flushed. Ignore any
+//           subsequent frames we receive with the sentiel pts.
+//
+//  Filter:  The challenge here is around fps filter adding and dropping frames.
+//           The fps filter expects a strictly monotonic input pts: frames with
+//           earlier timestamps get dropped, and frames with too-late timestamps
+//           will see a bunch of duplicated frames be generated to catch up with
+//           the timestamp that was just inserted. So we cache the last seen
+//           frame, rewrite the PTS based on the expected duration, and set a
+//           sentinel field (AVFrame.opaque). Then do a lot of rewriting to
+//           accommodate changes. See the notes in the filter_ctx struct and the
+//           process_out function. This is done for both audio and video.
+//
+//           One consequence of this behavior is that we currently cannot
+//           process segments out of order, due to the monotonicity requirement.
+//
+// Encoder:  For software encoding, we close the encoder and re-open.
+//           For Nvidia encoding, there is luckily an API available via
+//           avcodec_flush_buffers to flush the encoder.
+//
+
 //
 // Internal transcoder data structures
 //
@@ -47,6 +87,18 @@ struct filter_ctx {
   AVFilterContext *src_ctx;
 
   uint8_t *hwframes; // GPU frame pool data
+
+  // When draining the filtergraph, we inject fake frames.
+  // These frames need monotonically increasing timestamps at
+  // approximately the same interval as a normal stream of frames.
+  // In order to continue using the filter after flushing, we need
+  // to do two things:
+  // Adjust input timestamps forward to match the expected cumulative offset
+  //      ( otherwise filter will drop frames <= current pts )
+  // Re-adjust output timestamps backwards to compensate for the offset
+  //      ( pts received from filter will be wrong by however many flushed
+  //      frames received )
+  // the cumulative offset effect of flushing
   int64_t flush_offset;
   int flushed, flush_count;
 };
@@ -1140,6 +1192,9 @@ int process_out(struct input_ctx *ictx, struct output_ctx *octx, AVCodecContext
       frame->opaque = NULL; // reset just to be sure
       continue;
     }
+    // For now, the time base of the filter output is 1/framerate, so each
+    // frame gets its pts incremented by one. This may not always hold later on!
+    // TODO rescale flush_count towards filter output timebase as appropriate.
     if (frame) frame->pts -= filter->flush_count;
     ret = encode(encoder, frame, octx, ost);
     av_frame_unref(frame);

From a15864fad1d432191ab732d0fa811a7b1d3d06be Mon Sep 17 00:00:00 2001
From: Josh Allmann <joshua.allmann@gmail.com>
Date: Tue, 31 Mar 2020 22:02:51 +0000
Subject: [PATCH 9/9] ffmpeg: Rearrange fps/scale filters.

Also add a test for skipping segments.
---
 ffmpeg/api_test.go | 75 ++++++++++++++++++++++++++++++++++++++++++++++
 ffmpeg/ffmpeg.go   | 17 ++++++++---
 2 files changed, 88 insertions(+), 4 deletions(-)

diff --git a/ffmpeg/api_test.go b/ffmpeg/api_test.go
index b654aa46c4..d3b477f36a 100644
--- a/ffmpeg/api_test.go
+++ b/ffmpeg/api_test.go
@@ -6,6 +6,81 @@ import (
 	"testing"
 )
 
+func TestAPI_SkippedSegment(t *testing.T) {
+	// Ensure that things are still OK even if there are gaps in between segments
+	// We should see a perf slowdown if the filters are in the wrong order
+	// (fps before scale).
+	// Would be nice to measure this somehow, eg counting number of flushed frames.
+
+	// Really should refactor this test to increase commonality with other
+	// tests that also check things like SSIM, MD5 hashes, etc...
+	// See TestNvidia_API_MixedOutput / TestTranscoder_EncoderOpts / TestTranscoder_StreamCopy
+	run, dir := setupTest(t)
+	err := RTMPToHLS("../transcoder/test.ts", dir+"/out.m3u8", dir+"/out_%d.ts", "2", 0)
+	if err != nil {
+		t.Error(err)
+	}
+
+	profile := P144p30fps16x9
+	profile.Framerate = 123
+	idx := []int{0, 3}
+	tc := NewTranscoder()
+	defer tc.StopTranscoder()
+	for _, i := range idx {
+		in := &TranscodeOptionsIn{Fname: fmt.Sprintf("%s/out_%d.ts", dir, i)}
+		out := []TranscodeOptions{{
+			Oname:        fmt.Sprintf("%s/%d.md5", dir, i),
+			AudioEncoder: ComponentOptions{Name: "drop"},
+			VideoEncoder: ComponentOptions{Name: "copy"},
+			Muxer:        ComponentOptions{Name: "md5"},
+		}, {
+			Oname:        fmt.Sprintf("%s/sw_%d.ts", dir, i),
+			Profile:      profile,
+			AudioEncoder: ComponentOptions{Name: "copy"},
+		}}
+		res, err := tc.Transcode(in, out)
+		if err != nil {
+			t.Error(err)
+		}
+		if res.Decoded.Frames != 120 {
+			t.Error("Did not get decoded frames", res.Decoded.Frames)
+		}
+		if res.Encoded[1].Frames != 246 {
+			t.Error("Did not get encoded frames ", res.Encoded[1].Frames)
+		}
+	}
+	cmd := `
+    function check {
+
+      # Check md5sum for stream copy / drop
+      ffmpeg -loglevel warning -i out_$1.ts -an -c:v copy -f md5 ffmpeg_$1.md5
+      diff -u $1.md5 ffmpeg_$1.md5
+
+      # muxdelay removes the 1.4 sec mpegts offset, copyts passes ts through
+      ffmpeg -loglevel warning -i out_$1.ts -c:a aac -ar 44100 -ac 2 \
+        -vf fps=123,scale=w=256:h=144 -c:v libx264 -muxdelay 0 -copyts ffmpeg_sw_$1.ts
+
+      # sanity check ffmpeg frame count against ours
+      ffprobe -count_frames -show_streams -select_streams v ffmpeg_sw_$1.ts | grep nb_read_frames=246
+      ffprobe -count_frames -show_streams -select_streams v sw_$1.ts | grep nb_read_frames=246
+
+   # check image quality
+    ffmpeg -loglevel warning -i sw_$1.ts -i ffmpeg_sw_$1.ts \
+      -lavfi "[0:v][1:v]ssim=sw_stats_$1.log" -f null -
+    grep -Po 'All:\K\d+.\d+' sw_stats_$1.log | \
+      awk '{ if ($1 < 0.95) count=count+1 } END{ exit count > 5 }'
+
+    # Really should check relevant audio as well...
+    }
+
+
+    check 0
+    check 3
+  `
+	run(cmd)
+
+}
+
 func TestTranscoderAPI_InvalidFile(t *testing.T) {
 	// Test the following file open results on input: fail, success, fail, success
 
diff --git a/ffmpeg/ffmpeg.go b/ffmpeg/ffmpeg.go
index 18ceb88e50..6be6a884b8 100644
--- a/ffmpeg/ffmpeg.go
+++ b/ffmpeg/ffmpeg.go
@@ -211,14 +211,23 @@ func (t *Transcoder) Transcode(input *TranscodeOptionsIn, ps []TranscodeOptions)
 		}
 		// preserve aspect ratio along the larger dimension when rescaling
 		var filters string
-		if param.Framerate > 0 {
-			filters = fmt.Sprintf("fps=%d/1,", param.Framerate)
-		}
-		filters += fmt.Sprintf("%s='w=if(gte(iw,ih),%d,-2):h=if(lt(iw,ih),%d,-2)'", scale_filter, w, h)
+		filters = fmt.Sprintf("%s='w=if(gte(iw,ih),%d,-2):h=if(lt(iw,ih),%d,-2)'", scale_filter, w, h)
 		if input.Accel != Software && p.Accel == Software {
 			// needed for hw dec -> hw rescale -> sw enc
 			filters = filters + ",hwdownload,format=nv12"
 		}
+		// Add fps filter *after* scale filter because otherwise we could
+		// be scaling duplicate frames unnecessarily. This becomes a DoS vector
+		// when a user submits two frames that are "far apart" in pts and
+		// the fps filter duplicates frames to fill out the difference to maintain
+		// a consistent frame rate.
+		// Once we allow for alternating segments, this issue should be mitigated
+		// and the fps filter can come *before* the scale filter to minimize work
+		// when going from high fps to low fps (much more common when transcoding
+		// than going from low fps to high fps)
+		if param.Framerate > 0 {
+			filters += fmt.Sprintf(",fps=%d/1", param.Framerate)
+		}
 		muxOpts := C.component_opts{
 			opts: newAVOpts(p.Muxer.Opts), // don't free this bc of avformat_write_header API
 		}