SDAVAssetExportSession icon indicating copy to clipboard operation
SDAVAssetExportSession copied to clipboard

Scaling error in [buildDefaultVideoComposition]

Open gitgjogh opened this issue 6 years ago • 1 comments

Hi Olivier,

Nice work for video export on Apple system by offering customizable audio&video settings.

I recently made an alternative implementation by reference of your codes. And found that there is some error in [buildDefaultVideoComposition] when you try to make original video center inside the target size.

The problem is you scaling the video differently in x/y directions when the target DAR (w/h ratio) is not same to the original video.

	matrix = CGAffineTransformScale(matrix, ratio / xratio, ratio / yratio);  

That means you havn't preserved the aspect ratio of the original video after decoding. avvideoscalingmoderesize

Someone may wondering why the exported video seems all right. That just becasue AVVideoScalingModeResize is used implicitly for AVVideoScalingModeKey in the settings of AVAssetWriterInput

	[AVAssetWriterInput assetWriterInputWithMediaType:AVMediaTypeVideo 
	                                   outputSettings:self.videoSettings];

As Apple comments:

	/* AVVideoScalingModeResize - Crop to remove edge processing region; 
	scale remainder to destination area.  Does not preserve aspect ratio. */

That means the exported video has been deformed twice during the video export. Deformation is not gool for keeping video qualities, expecially when target DAR is reciprocal to the original video DAR.

The fix is quite simple, just apply preferredTransform of the original videoTrack and then set AVVideoScalingModeKey to AVVideoScalingModeResizeAspect. Here is my patch:

diff --git a/SDAVAssetExportSession.m b/SDAVAssetExportSession.m
index e99993b..86c7025 100755
--- a/SDAVAssetExportSession.m
+++ b/SDAVAssetExportSession.m
@@ -325,22 +325,12 @@
 		naturalSize.height = width;
 	}
 	videoComposition.renderSize = naturalSize;
-	// center inside
-	{
-		float ratio;
-		float xratio = targetSize.width / naturalSize.width;
-		float yratio = targetSize.height / naturalSize.height;
-		ratio = MIN(xratio, yratio);
-
-		float postWidth = naturalSize.width * ratio;
-		float postHeight = naturalSize.height * ratio;
-		float transx = (targetSize.width - postWidth) / 2;
-		float transy = (targetSize.height - postHeight) / 2;
-
-		CGAffineTransform matrix = CGAffineTransformMakeTranslation(transx / xratio, transy / yratio);
-		matrix = CGAffineTransformScale(matrix, ratio / xratio, ratio / yratio);
-		transform = CGAffineTransformConcat(transform, matrix);
-	}
+    // center inside
+    if (self.videoSettings && ![self.videoSettings objectForKey:AVVideoScalingModeKey]) {
+        NSMutableDictionary *internalSettings = [NSMutableDictionary dictionaryWithDictionary:self.videoSettings];
+        [internalSettings setObject:AVVideoScalingModeResizeAspect forKey:AVVideoScalingModeKey];
+        self.videoSettings = internalSettings;
+    }
 
 	// Make a "pass through video track" video composition.
 	AVMutableVideoCompositionInstruction *passThroughInstruction = [AVMutableVideoCompositionInstruction videoCompositionInstruction];

gitgjogh avatar Jul 29 '18 07:07 gitgjogh

Hi @gitgjogh. I opened an issue, can you look it? https://github.com/rs/SDAVAssetExportSession/issues/90

KarenK1995 avatar Nov 05 '18 14:11 KarenK1995