From 1b6d34f89c3ff81bfb325e636d086c4da099afa6 Mon Sep 17 00:00:00 2001
From: Matjaz Debelak <matjaz.debelak@bcc.no>
Date: Tue, 20 Aug 2024 13:25:50 +0200
Subject: [PATCH] Added onne more thing to attempt prevention of the %2 error
 for H264

Also added test
---
 services/transcode/h264.go      | 11 +++++---
 services/transcode/h264_test.go | 45 +++++++++++++++++++++++++++++++++
 utils/testutils/video.go        | 37 +++++++++++++++++++++++++++
 3 files changed, 90 insertions(+), 3 deletions(-)
 create mode 100644 services/transcode/h264_test.go
 create mode 100644 utils/testutils/video.go

diff --git a/services/transcode/h264.go b/services/transcode/h264.go
index a3084650..993cf4a3 100644
--- a/services/transcode/h264.go
+++ b/services/transcode/h264.go
@@ -1,12 +1,13 @@
 package transcode
 
 import (
-	"github.com/bcc-code/bcc-media-flows/utils"
 	"os"
 	"path/filepath"
 	"strconv"
 	"strings"
 
+	"github.com/bcc-code/bcc-media-flows/utils"
+
 	"github.com/bcc-code/bcc-media-flows/paths"
 	"github.com/bcc-code/bcc-media-flows/services/ffmpeg"
 	"github.com/samber/lo"
@@ -69,12 +70,18 @@ func H264(input H264EncodeInput, progressCallback ffmpeg.ProgressCallback) (*Enc
 		)
 	}
 
+	var videoFilters []string
+
 	if input.Resolution != nil {
 		input.Resolution.EnsureEven()
 		params = append(
 			params,
 			"-s", input.Resolution.FFMpegString(),
 		)
+
+		// This ensures that the scaled image is evenly divisible by 2,
+		// because the `-s` keeps the aspect ratio and thus can't be forced to be even
+		videoFilters = append(videoFilters, "scale=trunc(iw/2)*2:trunc(ih/2)*2")
 	}
 
 	if input.FrameRate != 0 {
@@ -84,8 +91,6 @@ func H264(input H264EncodeInput, progressCallback ffmpeg.ProgressCallback) (*Enc
 		)
 	}
 
-	var videoFilters []string
-
 	if input.Interlace {
 		params = append(
 			params,
diff --git a/services/transcode/h264_test.go b/services/transcode/h264_test.go
new file mode 100644
index 00000000..fd5f73fe
--- /dev/null
+++ b/services/transcode/h264_test.go
@@ -0,0 +1,45 @@
+package transcode_test
+
+import (
+	"testing"
+
+	"github.com/bcc-code/bcc-media-flows/paths"
+	"github.com/bcc-code/bcc-media-flows/services/ffmpeg"
+	"github.com/bcc-code/bcc-media-flows/services/transcode"
+	"github.com/bcc-code/bcc-media-flows/utils"
+	"github.com/bcc-code/bcc-media-flows/utils/testutils"
+	"github.com/davecgh/go-spew/spew"
+	"github.com/stretchr/testify/assert"
+)
+
+func Test_H264_WeirdResolutions(t *testing.T) {
+	testFile := paths.MustParse("./testdata/generated/h264_weird_resolutions.mov")
+
+	testutils.GenerateVideoFile(testFile, testutils.VideoGeneratorParams{
+		DAR:       "16/9",
+		SAR:       "608/405",
+		Width:     720,
+		Height:    608,
+		Duration:  5,
+		FrameRate: 25,
+	})
+
+	progressCallback := func(i ffmpeg.Progress) {
+		spew.Dump(i)
+	}
+
+	r, err := transcode.H264(transcode.H264EncodeInput{
+		Bitrate: "320k",
+		Resolution: &utils.Resolution{
+			Width:  320,
+			Height: 180,
+		},
+		FrameRate: 0,
+		FilePath:  testFile.Local(),
+		OutputDir: testFile.Dir().Local(),
+	}, progressCallback)
+
+	assert.NoError(t, err)
+	assert.NotNil(t, r)
+	spew.Dump(r)
+}
diff --git a/utils/testutils/video.go b/utils/testutils/video.go
new file mode 100644
index 00000000..79b4ba3a
--- /dev/null
+++ b/utils/testutils/video.go
@@ -0,0 +1,37 @@
+package testutils
+
+import (
+	"fmt"
+	"os/exec"
+
+	"github.com/bcc-code/bcc-media-flows/paths"
+)
+
+type VideoGeneratorParams struct {
+	Duration  float64
+	FrameRate int
+	Width     int
+	Height    int
+	SAR       string
+	DAR       string
+}
+
+func GenerateVideoFile(outFile paths.Path, videoParams VideoGeneratorParams) paths.Path {
+	args := []string{
+		"-f", "lavfi",
+		"-i", fmt.Sprintf("color=c=blue:s=720x608:d=%f", videoParams.Duration),
+		"-vf", fmt.Sprintf("setsar=%s, setdar=%s", videoParams.SAR, videoParams.DAR),
+		"-r", fmt.Sprintf("%d", videoParams.FrameRate),
+		"-c:v", "prores_ks",
+		"-profile:v", "3",
+		"-y", outFile.Local(),
+	}
+
+	cmd := exec.Command("ffmpeg", args...)
+	err := cmd.Run()
+	if err != nil {
+		panic(err)
+	}
+
+	return outFile
+}