-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
@remotion/renderer: More sound calculation to ensure even dimensions for H.264 #5715
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 3 commits
7a472ad
13d72b4
005e58e
e5a9e6a
33e6083
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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 |
|---|---|---|
|
|
@@ -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, | ||
| }; | ||
| } | ||
|
|
||
|
|
@@ -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) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 Detailsdiff --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(
AnalysisAlgorithm can produce zero dimensions with small scale factorsWhat fails: validateEvenDimensionsWithCodec() in 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.
JonnyBurger marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| 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, | ||
| }; | ||
| }; | ||
Uh oh!
There was an error while loading. Please reload this page.