Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/eslint-plugin/src/rules/even-dimensions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ type Options = [];
type MessageIds = 'EvenDimensions';

const EvenDimensions =
"Videos rendered in H264/H265 codec do not support dimensions that are not divisible by 2. Make the number even to resolve this warning. Ignore this warning if you don't plan on rendering this video with a H264 or H265 codec.";
"Videos rendered in H264/H265 codec do not support dimensions that are not divisible by 2. Remotion will round down your dimensions to satisfy this constraint. Ignore this warning if you don't plan on rendering this video with a H264 or H265 codec.";

export default createRule<Options, MessageIds>({
name: 'even-dimensions',
Expand Down
38 changes: 12 additions & 26 deletions packages/renderer/src/render-media.ts
Original file line number Diff line number Diff line change
Expand Up @@ -443,33 +443,19 @@ const internalRenderMediaRaw = ({
onCtrlCExit(`Delete ${workingDir}`, () => deleteDirectory(workingDir));
}

const {
actualWidth: widthEvenDimensionsUndivided,
actualHeight: heightEvenDimensionsUndivided,
} = validateEvenDimensionsWithCodec({
codec,
height: compositionWithPossibleUnevenDimensions.height,
scale,
width: compositionWithPossibleUnevenDimensions.width,
wantsImageSequence: false,
indent,
logLevel,
});

const heightEvenDimensions = Math.round(
heightEvenDimensionsUndivided / scale,
);
const widthEvenDimensions = Math.round(widthEvenDimensionsUndivided / scale);
const {actualWidth: widthEvenDimensions, actualHeight: heightEvenDimensions} =
validateEvenDimensionsWithCodec({
codec,
height: compositionWithPossibleUnevenDimensions.height,
scale,
width: compositionWithPossibleUnevenDimensions.width,
wantsImageSequence: false,
indent,
logLevel,
});
const actualWidth = widthEvenDimensions * scale;
const actualHeight = heightEvenDimensions * scale;

const {actualWidth, actualHeight} = validateEvenDimensionsWithCodec({
codec,
height: compositionWithPossibleUnevenDimensions.height,
scale,
width: compositionWithPossibleUnevenDimensions.width,
wantsImageSequence: false,
indent,
logLevel,
});
const composition = {
...compositionWithPossibleUnevenDimensions,
height: heightEvenDimensions,
Expand Down
36 changes: 36 additions & 0 deletions packages/renderer/src/test/even-dimensions.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import {expect, test} from 'bun:test';
import {validateEvenDimensionsWithCodec} from '../validate-even-dimensions-with-codec';

test('should eventually result in even dimensions', () => {
const scale = 2.3275862069;
const {actualWidth, actualHeight} = validateEvenDimensionsWithCodec({
codec: 'h264',
width: 464,
height: 832,
indent: false,
logLevel: 'info',
scale,
wantsImageSequence: false,
});
expect(actualHeight).toBe(831);
expect(actualWidth).toBe(464);
expect(Math.round(actualHeight * scale)).toBe(1934);
expect(Math.round(actualWidth * scale)).toBe(1080);
});

test('default case', () => {
const scale = 2;
const {actualWidth, actualHeight} = validateEvenDimensionsWithCodec({
codec: 'h264',
width: 464,
height: 832,
indent: false,
logLevel: 'info',
scale,
wantsImageSequence: false,
});
expect(actualHeight).toBe(832);
expect(actualWidth).toBe(464);
expect(Math.round(actualHeight * scale)).toBe(1664);
expect(Math.round(actualWidth * scale)).toBe(928);
});
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ describe('validateEvenDimensionsWithCodec', () => {
};

describe('H264/H265 codec behavior with odd dimensions', () => {
test('should round down odd width to even when using H264', () => {
test.only('should round down odd width to even when using H264', () => {
const result = validateEvenDimensionsWithCodec({
width: 4000,
height: 2592,
Expand All @@ -18,8 +18,8 @@ describe('validateEvenDimensionsWithCodec', () => {
...defaultConfig,
});

expect(result.actualWidth).toBe(1400); // 4000 * 0.35 = 1400 (even)
expect(result.actualHeight).toBe(906); // 2592 * 0.35 = 907.2 -> 907 (odd) -> 906 (even)
expect(result.actualWidth).toBe(4000); // 4000 * 0.35 = 1400 (even)
expect(result.actualHeight).toBe(2590); // 2592 * 0.35 = 907.2 -> 907 (odd) -> 906 (even)
});

test('should round down odd width to even when using H265', () => {
Expand Down Expand Up @@ -111,8 +111,8 @@ describe('validateEvenDimensionsWithCodec', () => {
...defaultConfig,
});

expect(result.actualWidth).toBe(500);
expect(result.actualHeight).toBe(300);
expect(result.actualWidth).toBe(1000);
expect(result.actualHeight).toBe(600);
});
});

Expand Down Expand Up @@ -185,8 +185,9 @@ describe('validateEvenDimensionsWithCodec', () => {
});

// 1000 * 0.333 = 333, which is odd, should become 332
expect(result.actualWidth).toBe(332);
expect(result.actualHeight).toBe(332);
// 998 * 0.3333 -> 332.334 will be rounded down
expect(result.actualWidth).toBe(998);
expect(result.actualHeight).toBe(998);
});
});
});
50 changes: 16 additions & 34 deletions packages/renderer/src/validate-even-dimensions-with-codec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,10 @@ export const validateEvenDimensionsWithCodec = ({
indent: boolean;
logLevel: LogLevel;
}) => {
let actualWidth = width * scale;
let actualHeight = height * scale;
if (wantsImageSequence) {
return {
actualWidth,
actualHeight,
actualWidth: width,
actualHeight: height,
};
}

Expand All @@ -35,53 +33,37 @@ export const validateEvenDimensionsWithCodec = ({
codec !== 'h264-ts'
) {
return {
actualWidth,
actualHeight,
actualWidth: width,
actualHeight: height,
};
}

if (
actualWidth % 1 !== 0 &&
(actualWidth % 1 < 0.005 || actualWidth % 1 > 0.005)
) {
Log.verbose(
{indent, logLevel},
`Rounding width to an even number from ${actualWidth} to ${Math.round(actualWidth)}`,
);
actualWidth = Math.round(actualWidth);
let heightEvenDimensions = height;
while (Math.round(heightEvenDimensions * scale) % 2 !== 0) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The while loop algorithm can potentially produce zero or negative dimensions when the scale factor is small, which would result in invalid video dimensions.

View Details
📝 Patch Details
diff --git a/packages/renderer/src/validate-even-dimensions-with-codec.ts b/packages/renderer/src/validate-even-dimensions-with-codec.ts
index 55b27343ee..3043c6b7fe 100644
--- a/packages/renderer/src/validate-even-dimensions-with-codec.ts
+++ b/packages/renderer/src/validate-even-dimensions-with-codec.ts
@@ -39,14 +39,34 @@ export const validateEvenDimensionsWithCodec = ({
 	}
 
 	let heightEvenDimensions = height;
-	while (Math.round(heightEvenDimensions * scale) % 2 !== 0) {
+	while (Math.round(heightEvenDimensions * scale) % 2 !== 0 && heightEvenDimensions > 1) {
 		heightEvenDimensions--;
 	}
+	// If we can't find a dimension that produces an even scaled result, find the smallest that does
+	if (Math.round(heightEvenDimensions * scale) % 2 !== 0) {
+		// Search upward from 2 to find the first dimension that produces an even scaled result
+		for (let dim = 2; dim <= Math.max(height, 10); dim++) {
+			if (Math.round(dim * scale) % 2 === 0) {
+				heightEvenDimensions = dim;
+				break;
+			}
+		}
+	}
 
 	let widthEvenDimensions = width;
-	while (Math.round(widthEvenDimensions * scale) % 2 !== 0) {
+	while (Math.round(widthEvenDimensions * scale) % 2 !== 0 && widthEvenDimensions > 1) {
 		widthEvenDimensions--;
 	}
+	// If we can't find a dimension that produces an even scaled result, find the smallest that does
+	if (Math.round(widthEvenDimensions * scale) % 2 !== 0) {
+		// Search upward from 2 to find the first dimension that produces an even scaled result
+		for (let dim = 2; dim <= Math.max(width, 10); dim++) {
+			if (Math.round(dim * scale) % 2 === 0) {
+				widthEvenDimensions = dim;
+				break;
+			}
+		}
+	}
 
 	if (widthEvenDimensions !== width) {
 		Log.verbose(

Analysis

Algorithm can produce zero dimensions with small scale factors

What fails: validateEvenDimensionsWithCodec() in packages/renderer/src/validate-even-dimensions-with-codec.ts decrements dimensions without bounds checking, resulting in zero width or height when the scale factor is small

How to reproduce:

validateEvenDimensionsWithCodec({
  width: 1, height: 100, scale: 0.6, codec: 'h264', 
  wantsImageSequence: false, indent: false, logLevel: 'info'
})
// Returns: { actualWidth: 0, actualHeight: 100 }

Result: Zero width dimension returned, which would create invalid video files that cannot be encoded by H.264

Expected: All dimensions should remain above zero. Algorithm should find valid dimensions that produce even scaled results without going to zero, as confirmed by Stack Overflow documentation that H.264 requires even dimensions but has no documented minimum above zero.

heightEvenDimensions--;
}

if (
actualHeight % 1 !== 0 &&
(actualHeight % 1 < 0.005 || actualHeight % 1 > 0.005)
) {
Log.verbose(
{indent, logLevel},
`Rounding height to an even number from ${actualHeight} to ${Math.round(actualHeight)}`,
);
actualHeight = Math.round(actualHeight);
let widthEvenDimensions = width;
while (Math.round(widthEvenDimensions * scale) % 2 !== 0) {
widthEvenDimensions--;
}

const displayName = codec === 'h265' ? 'H265' : 'H264';

if (actualWidth % 2 !== 0) {
if (widthEvenDimensions !== width) {
Log.verbose(
{indent, logLevel},
`Rounding width down to an even number from ${actualWidth} to ${actualWidth - 1} for ${displayName} codec compatibility`,
`Rounding width to an even number from ${width} to ${widthEvenDimensions}`,
);
actualWidth -= 1;
}

if (actualHeight % 2 !== 0) {
if (heightEvenDimensions !== height) {
Log.verbose(
{indent, logLevel},
`Rounding height down to an even number from ${actualHeight} to ${actualHeight - 1} for ${displayName} codec compatibility`,
`Rounding height to an even number from ${height} to ${heightEvenDimensions}`,
);
actualHeight -= 1;
}

return {
actualWidth,
actualHeight,
actualWidth: widthEvenDimensions,
actualHeight: heightEvenDimensions,
};
};
Loading