-
-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Ability to set video resolutions (videoFrameWidth & videoFrameHeight) #3121
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No more post-recording cropping in Android to have 4:3 🎉 🎉
I think setting the preview dimensions in iOS and quality 4:3 produces the same result, right?
Haven't tested it myself.
Nice! Do you think we can proceed to merge this @MarcoScabbiolo? |
I'm sorry I forgot to mention that it is Android only. I don't have iPhone, I can fix it later for iOS too. Another thing I would like to mention is that the resolution should be supported by the device otherwise it would fail to record. For that we need to expose getSupportedVideoSizes(). I have looked into OpenCamera source code to see how it generates the list of supported video sizes, I'm currently working on that (Will create another PR for that). |
I think it will be best if you add the method to get the supported sizes to this PR, so the feature is complete once released. Mainly because of the many differences across Android devices. |
Sure I can do that. |
Does this replace setting |
With this change we can deprecate
Correct but all those profiles are 16:9. With this PR the width/height can be overridden to get the desired aspect ratio. For example: Currently it is not possible to record in high quality with aspect ratio of |
I see, I'm slightly confused, but when I set What would happen to iOS? |
…e over getOutputSizes
I have added
(Note: The result is for the ratio set in The sample output is: I originally wanted to return json (width & height in their own fields) instead of string but decided to keep it consistent with
|
Isn't Does this mean that we need to we will need to "parse" the results in order to set videoFrameWidth and videoFrameHeight? It's probably a better idea to return a JSON as you mentioned. |
Yes, it is actual aspect ratio.
Right, I did it to be consistent with |
@makzee what will be the behaviour if invalid values are set? Have you also tried with all available cameras (front, back)? |
…upported. It returns empty array instead of exception
When invalid values are set, it returns an error. Yes I have tested with both the cameras, it works fine. I have added
Reponse: It is ready to be reviewed and merged. |
@cristianoccazinsp, @MarcoScabbiolo do you think we can go on merging this? |
It looks pretty good to me! Although I'm still not sure what's the benefit of setting the frame width/height, if you already "control" that with the quality parameter. For example, what would happen if the following options are used: // change quality but use a fixed width/height ? // The other way around Looks like it may end up causing some confusion because the pixel dimension is usually strictly tied to the quality name. |
Each quality maps to one of the CamcorderProfile (https://developer.android.com/reference/android/media/CamcorderProfile). A profile has a fixed set of settings like fps, bitrate, resolution, codec etc but you are still allowed to change them as per your requirement. The reason for these changes was that we needed to record video in 4:3 at a higher resolution (not just 480p). Most of all these profiles correspond to 16:9, for example: QUALITY_1080P gives you 1920x1080. If I want to record 1080p in 4:3 (i.e. 1440x1080), this PR allows to you do that.
It means "Use 1080p profile but override the width to 1440.". Quality represents a baseline profile and we need to allow flexibility to change specific attributes of that profile. |
@makzee that greatly explains it, and makes complete sense. Good work! Do we need this for iOS as well? |
Thanks @cristianoccazinsp. Yes we need this for iOS as well but I don't have a bandwidth for it right now. I'm planning to implement this in a month or so. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Havent tested it myself
(Note for v4): The options that make a difference between raw recording and re-encoding should be made clear even if they change across devices, mostly on Android. Remove |
It adds the ability to set
videoFrameWidth
andvideoFrameHeight
.Test Plan
Recorded two videos, 1 with the desired resolutions and other without setting them at all. 2nd test is it make sure that it still works as it used to previously.
Test 1
const video = await cameraRef.recordAsync({ mute: true, quality: '4:3', videoFrameWidth: 1280, videoFrameHeight: 960 });
Test 2
const video = await cameraRef.recordAsync({ mute: true, quality: '4:3' });