Skip to content

added new video parameters and new aar - #11

Merged
OS-ruimoreiramendes merged 4 commits into
feat/unified-cordova-pluginfrom
feat/add-video-parameters
Mar 26, 2026
Merged

added new video parameters and new aar#11
OS-ruimoreiramendes merged 4 commits into
feat/unified-cordova-pluginfrom
feat/add-video-parameters

Conversation

@OS-ruimoreiramendes

Copy link
Copy Markdown
Contributor

No description provided.

@OS-ruimoreiramendes
OS-ruimoreiramendes marked this pull request as ready for review March 25, 2026 16:36
= true // Should we save the recorded video persistently or not
private var includeMetadata =
false // Should we allow the app to obtain metadata about the media item
private var editParameters = IONCAMREditParameters(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is it ok to remove these values? I had problems with this before but maybe this isn't needed anymore

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think it’s safe to remove the default values since editParameters is always initialized right before use, so we don’t rely on a default instance anymore, from the tests I’ve done so far, I haven’t run into any issues.
Do you remember what kind of issues you had before? If you have more details, I can try to replicate it to make sure I'm not missing anything

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

After analysis, and given you mentioned that you have issues with this, I think is better to keep editParameters initialized with default values at class level. Removing it would require carefully controlling its value before every call to callEditPhoto and callEditURIPhoto, which adds complexity and risk. To keep things safe and predictable at this stage, I'd prefer not to change this behaviour for now.

same for ionic-team/capacitor-camera#17

args.getJSONObject(0).getBoolean(INCLUDE_METADATA),
args.getJSONObject(0).optBoolean(IS_PERSISTENT, true)
)
callCaptureVideo(videoParameters!!)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we need the force-unwrap? Could we use a let instead?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yeah, we can change this to a let. I used a force unwrap because we create videoParameters right before calling captureVideo, so it will always exist, but I can change it.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ok yeah if we can use a let instead aI think it would be better

@OS-ruimoreiramendes
OS-ruimoreiramendes merged commit 829b836 into feat/unified-cordova-plugin Mar 26, 2026
@OS-ruimoreiramendes
OS-ruimoreiramendes deleted the feat/add-video-parameters branch March 26, 2026 12:10
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.

2 participants