react-native-vision-camera icon indicating copy to clipboard operation
react-native-vision-camera copied to clipboard

Feature add in torch control

Open fahadhaque007 opened this issue 10 months ago • 5 comments

Changes to code to allow devs to control background torch when the camera is on

What

This PR adds capacity to control the timing and intensity of the torch during video recording. It allows the developer to specify Camera props that will activate the torch at two different levels that activate separately after a delay relative to recording onset and extinguish after a specified duration. Timestamps are appended as metadata indicating when each component (torch, background) was activated or extinguished. See attached diagram.

Screenshot 2024-04-09 at 4 11 36 PM

Changes

This PR adds props to the Camera that define the onset and duration of two different torch levels.

Tested on

iPhone12, 14, 15

Related issues

fahadhaque007 avatar Apr 12 '24 05:04 fahadhaque007

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
react-native-vision-camera ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 30, 2024 0:19am

vercel[bot] avatar Apr 12 '24 05:04 vercel[bot]

Hey @fahadhaque007 - thanks for your PR, much appreciated!!

I have a few concerns regarding this though, mainly about making sure the codebase is clean and doesn't involve a lot of "hardcoded procedures for specific situations", in this case it would be your use-case of specifying how long the torch is on or off during a video.

  1. I think the only prop we should add is torchLevel. The other props (torchDelay, torchDuration, backgroundLevel, backgroundDelay, backgroundDuration) shouldn't be part of the native codebase in my opinion. I think this is very specific to your use-case and I've never seen another app where stuff like this is required, so I feel like it makes more sense to put this into your application logic instead. So in your code you have something like a setTimeout after the recording started, and then you set the torch (or torchLevel) values on the Camera accordingly.

    Note: Yes, I know that timing in React Native is a bit off and not exactly millisecond-perfect with native code, but if I make startRecording() awaitable you know exactly when the recording actually started and can configure this yourself. Also, jfyi torch while recording is currently controlled through JS, which I prefer a lot since all of that logic doesn't need to be in native. Not everything that could be in native code, should be in native code, ya know?

  2. There is no Android equivalent, which is why I don't want to merge this as-is. Everything should ideally be cross-platform. I can take a look if there is a torch level control for Camera2 in the meantime, will get back to you on this one.

mrousavy avatar Apr 12 '24 09:04 mrousavy

Also, what is backgroundLevel?

Specifies the background torch level of the current camera. This value ranges from 0.0 to 1.0.

i still don't get this?

mrousavy avatar Apr 12 '24 09:04 mrousavy

Hi @mrousavy: 👋

@fahadhaque007 and I have been working on this together so I thought I would add some context here.

I have a few concerns regarding this though, mainly about making sure the codebase is clean and doesn't involve a lot of "hardcoded procedures for specific situations", in this case it would be your use-case of specifying how long the torch is on or off during a video.

  1. I think the only prop we should add is torchLevel. The other props (torchDelay, torchDuration, backgroundLevel, backgroundDelay, backgroundDuration) shouldn't be part of the native codebase in my opinion. I think this is very specific to your use-case and I've never seen another app where stuff like this is required, so I feel like it makes more sense to put this into your application logic instead. So in your code you have something like a setTimeout after the recording started, and then you set the torch (or torchLevel) values on the Camera accordingly.

    Note: Yes, I know that timing in React Native is a bit off and not exactly millisecond-perfect with native code, but if I make startRecording() awaitable you know exactly when the recording actually started and can configure this yourself. Also, jfyi torch while recording is currently controlled through JS, which I prefer a lot since all of that logic doesn't need to be in native. Not everything that could be in native code, should be in native code, ya know?

That's certainly a fair criticism. Your RNVC library drives a lot of apps and I'm sure many of them having nothing to do with the torch. For those that do, we simply thought others might find our solution useful. Unfortunately, we require millisecond precision for our application so a JS solution won't work. We designed it to be as light touch as possible so that maintenance wouldn't be burdensome.

  1. There is no Android equivalent, which is why I don't want to merge this as-is. Everything should ideally be cross-platform. I can take a look if there is a torch level control for Camera2 in the meantime, will get back to you on this one.

We do have the code to drive android, not pushing it up was simply an oversight on our part (Apologies!). Coming later today. Our testing has been limited to the Samsung Galaxy S23 so we're not sure how broadly applicable those changes will be.

Also, what is backgroundLevel?

BackgroundLevel ( also backgroundDelay, backgroundDuration) is perhaps not the best choice for a name, it's intended kind of like a solution for redeye reduction. A low-level torch turns on early, followed by a later high intensity flash (see diagram above). Since their parameters are handled independently, however, they might be more simply named flash1 and flash2.

If you think this might be too specific a use case to be merged into RNVC what would you recommend?

titanium-cranium avatar Apr 15 '24 02:04 titanium-cranium

Hey - thanks for the details! The code is definitely good for your use-case, but I don't think this should make it into VisionCamera. The only thing I'm happy to merge would be the torchLevel prop. Sorry.

mrousavy avatar Apr 15 '24 07:04 mrousavy

Hey - do you want to create a PR for only the torchLevel prop? Otherwise I can also do that, that part was still good.

mrousavy avatar May 01 '24 10:05 mrousavy

Hey @mrousavy yes I can create that. I will also implement the Anrdoid changes.

fahadhaque007 avatar May 03 '24 01:05 fahadhaque007

Thank you!

mrousavy avatar May 03 '24 08:05 mrousavy