From dfca9046d3082281ed295d0098321334178491e1 Mon Sep 17 00:00:00 2001 From: Michal Adamczak Date: Wed, 14 Sep 2022 14:25:36 +0200 Subject: [PATCH 1/3] Scaling bug fix (issue 353) It turned out special logic that makes sure Nvidia encoder limits are respected can sometimes cause problems. More specifically: - Resolution cropping logic may generate odd resolutions for the video stream, which is not supported by h.264 and usually shows up as a "green bar" at the bottom of the video (because encoder adds a line of zero YUV values, which translated to certain shade of green) - Portrait resolutions get needlessly enlarged, ie value that was requested as a height of the picture gets used as a width, and as aspect ratio is preserved, height is rescaled and much larger Two changes were made: 1) all scaled values are now rounded to the nearest even number and 2) the whole mechanism is not applied if the resolution stays within the encoder limits --- ffmpeg/ffmpeg.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/ffmpeg/ffmpeg.go b/ffmpeg/ffmpeg.go index e5c03a6685..65892b3e01 100755 --- a/ffmpeg/ffmpeg.go +++ b/ffmpeg/ffmpeg.go @@ -248,12 +248,15 @@ type MediaFormatInfo struct { Width, Height int } + +// h.264 only allows even resolutions, so both functions round up the results +// to the nearest even number func (f *MediaFormatInfo) ScaledHeight(width int) int { - return int(float32(width) * float32(f.Height) / float32(f.Width)) + return int(float32(width) * float32(f.Height) / float32(f.Width) + 1) & (^1) } func (f *MediaFormatInfo) ScaledWidth(height int) int { - return int(float32(height) * float32(f.Width) / float32(f.Height)) + return int(float32(height) * float32(f.Width) / float32(f.Height) + 1) & (^1) } func GetCodecInfo(fname string) (CodecStatus, MediaFormatInfo, error) { @@ -540,6 +543,11 @@ func (l *CodingSizeLimit) Clamp(p *VideoProfile, format MediaFormatInfo) error { if err != nil { return err } + // check if we _should_ clamp at all + if (l.WidthMin <= w) && (w <= l.WidthMax) && (l.HeightMin <= h) && (h <= l.HeightMax) { + // given video resolution is within encoder limits, no need for intervention + return nil + } // detect correct rotation outputAr := float32(w) / float32(h) inputAr := float32(format.Width) / float32(format.Height) From a86206e25aea3d8d6ae80e2dfb2e77157239d1b6 Mon Sep 17 00:00:00 2001 From: Michal Adamczak Date: Thu, 15 Sep 2022 08:22:00 +0200 Subject: [PATCH 2/3] Test_ResolutionClamp fixed Previous commit changed the resolution clamping approach: instead of changing every resolution given, change only resolutions that are not acceptable to the encoder. To make tests pass properly, one thing needed to change. Previously, when 600x300 resolution was given to the encoder with 700x500 limits, it got changed to the 250x500. While that was an expected behaviour, I don't think it is a proper one - why change something that encoder is fully willing to accept? So the test was modified to be OK, with original 600x300 --- ffmpeg/ffmpeg_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ffmpeg/ffmpeg_test.go b/ffmpeg/ffmpeg_test.go index 098e81d15a..507f68a4e7 100644 --- a/ffmpeg/ffmpeg_test.go +++ b/ffmpeg/ffmpeg_test.go @@ -1855,7 +1855,7 @@ func TestResolution_Clamp(t *testing.T) { test(l, Size{1000, 500}, landscape, Size{700, 350}) test(l, Size{1000, 500}, portrait, Size{250, 500}) test(l, Size{500, 1000}, landscape, Size{700, 350}) - test(l, Size{600, 300}, portrait, Size{250, 500}) + test(l, Size{600, 300}, portrait, Size{600, 300}) test(l, Size{300, 600}, portrait, Size{250, 500}) // Test impossible limits for aspect ratio == 2 From 1019972bcdc376dba038d8f851c10212e952f13b Mon Sep 17 00:00:00 2001 From: Michal Adamczak Date: Fri, 16 Sep 2022 12:37:32 +0200 Subject: [PATCH 3/3] Fix for TestTranscoder_Portrait Previous changes caused limits from the array in ffmpeg.go to be strictly applied, i.e. if the resolution was fitting the limits, no change was done. This uncovered the fact that the limits themselves were bad - on my machine NVenc refused to work with the message that the frame dimensions is too low. Changing the minimum resolution to 146x145 fixed the problem. Note that is is quite likely that the limits should be different for h265 as well, but on my machine all works. Proper approach for this problem would be something along the line of dynamic discovery of these limits (via Nvidia APIs) and then applying true local HW limits, not hardcoded ones. --- ffmpeg/ffmpeg.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ffmpeg/ffmpeg.go b/ffmpeg/ffmpeg.go index 65892b3e01..2ab2912a0a 100755 --- a/ffmpeg/ffmpeg.go +++ b/ffmpeg/ffmpeg.go @@ -577,7 +577,7 @@ func (l *CodingSizeLimit) Clamp(p *VideoProfile, format MediaFormatInfo) error { // 7th Gen NVENC limits: var nvidiaCodecSizeLimts = map[VideoCodec]CodingSizeLimit{ - H264: {146, 50, 4096, 4096}, + H264: {146, 145, 4096, 4096}, H265: {132, 40, 8192, 8192}, }