Skip to content
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

Videos have the wrong size when exporting them with a filter #125

Merged
merged 4 commits into from
Dec 30, 2021

Conversation

adriana-elizondo
Copy link
Collaborator

@adriana-elizondo adriana-elizondo commented Dec 28, 2021

After applying a filter to a video, when exporting it the dimensions were wrong.
This PR removes the size specification in the renderer used by the VideoCompositor so that the default is used.

To test it in Orangina:
-Open the editor to create a new post.
-Select a video from your phone or record a new one.
-Add a filter to your video and tap on the blue arrow to select it.
-The size of the video in the editor shouldn't look distorted or different.
-Post the video.
-The video should be posted with the selected filter and the correct dimensions.
-Bonus: try with videos in different orientations and with images and GIFs too, to make sure they weren't affected.

@bjtitus
Copy link
Collaborator

bjtitus commented Dec 29, 2021

@adriana-elizondo What do you think about gating this change behind the scaleMediaToFill flag by changing the call sites in Renderer to pass a nil size to these methods? This way the Wordpress app wouldn't be affected.

Renderer.swift:134

---let (finalMediaTransform, outputDimensions) = configureScaleToFill(sampleBuffer: sampleBuffer, size: scaleToFillSize)
+++let (finalMediaTransform, outputDimensions) = configureScaleToFill(sampleBuffer: sampleBuffer, size: settings?.features.scaleMediaToFill == true ? scaleToFillSize : nil)

Renderer.swift:207

---let (finalMediaTransform, outputDimensions) = configureScaleToFill(sampleBuffer: sampleBuffer, size: scaleToFillSize)
+++let (finalMediaTransform, outputDimensions) = configureScaleToFill(sampleBuffer: sampleBuffer, size: settings?.features.scaleMediaToFill == true ? scaleToFillSize : nil)

RendererPatch.patch.zip

@adriana-elizondo
Copy link
Collaborator Author

@adriana-elizondo What do you think about gating this change behind the scaleMediaToFill flag by changing the call sites in Renderer to pass a nil size to these methods? This way the Wordpress app wouldn't be affected.

Renderer.swift:134

---let (finalMediaTransform, outputDimensions) = configureScaleToFill(sampleBuffer: sampleBuffer, size: scaleToFillSize)
+++let (finalMediaTransform, outputDimensions) = configureScaleToFill(sampleBuffer: sampleBuffer, size: settings?.features.scaleMediaToFill == true ? scaleToFillSize : nil)

Renderer.swift:207

---let (finalMediaTransform, outputDimensions) = configureScaleToFill(sampleBuffer: sampleBuffer, size: scaleToFillSize)
+++let (finalMediaTransform, outputDimensions) = configureScaleToFill(sampleBuffer: sampleBuffer, size: settings?.features.scaleMediaToFill == true ? scaleToFillSize : nil)

RendererPatch.patch.zip

yes, you are totally right. Ill update it. Thanks Brandon!

Copy link
Collaborator

@bjtitus bjtitus left a comment

Choose a reason for hiding this comment

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

:shipit: Looks good to me!

@bjtitus
Copy link
Collaborator

bjtitus commented Dec 29, 2021

@adriana-elizondo Do you want to make a new version? I think you should be able to just increment the version number like in this commit and the release will be handled. 0dc70d4

@bjtitus bjtitus merged commit bbfad19 into main Dec 30, 2021
@bjtitus bjtitus deleted the filters-for-videos-size-fix branch July 8, 2022 03:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants