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

feat: Add onStarted and onStopped events #2273

Merged
merged 4 commits into from
Dec 9, 2023
Merged

Conversation

mrousavy
Copy link
Owner

@mrousavy mrousavy commented Dec 9, 2023

What

Adds two events:

  • onStarted: Called when the Camera started streaming frames
  • onStopped: Called when the Camera stops streaming frames

Changes

Tested on

Related issues

Copy link

vercel bot commented Dec 9, 2023

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 Dec 9, 2023 6:10pm

@mrousavy mrousavy merged commit 4ee52d6 into main Dec 9, 2023
11 of 14 checks passed
@mrousavy mrousavy deleted the feat/on-start-stop-events branch December 9, 2023 18:09
Log.i(CameraView.TAG, "invokeOnStarted()")

val reactContext = context as ReactContext
reactContext.getJSModule(RCTEventEmitter::class.java).receiveEvent(id, "cameraStarted", null)

Choose a reason for hiding this comment

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

Hey @mrousavy Maybe we should probably add the native timestamp in this event?
As otherwise, JS thread may delay a lot to consume these events, so if someone wants absolute precision he won't be able to do so in JS thread, making this feature less useful?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Hey - why would you need the timestamp, what would that actually change?

Choose a reason for hiding this comment

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

I can think of a feature where you want to synchronize video frames with another source, i.e bluetooth data from a device, in matter of ms delay.
To do so, one may want to have absolute control of when the video started/stopped recording frames, and match this with the device's stream input.
Without native timestamp on these events, we can only mark the timestamp on JS thread when this event is captured. Though, in case the JS thread is blocked and this event is processed later in the queue, we can end with completely false timestamp

Copy link
Owner Author

Choose a reason for hiding this comment

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

Tbh I don't agree here - this should definitely be done with a Frame Processor then, where you get a full stream of frames with their exactly accurate timestamps, and they are definitely in time as it is synchronous and blocking.

@xulihang
Copy link
Contributor

It is possible to return the frame's size in the onStarted event so that I don't have to get it in the frame processor?

@mrousavy
Copy link
Owner Author

No, because there is no "frame" if there is no VideoPipeline for example. onStarted also fires if video={false}, in which case I do not have access to the frame.

You could get the size from your format (videoWidth x videoHeight).

If you are not using format, I could add a onFormatChanged callback which gives you the format that was just configured which is just the default format?

@xulihang
Copy link
Contributor

If I specify the video format to 1080P, the frame's size is 1920x1080 on Android and 1080x1920 on iOS. If this is a fixed behavior, I can handle this difference based on platforms and don't need to get the frame size in an event.

But it would be great to have an event to return the actual frame size after the camera starts.

isaaccolson pushed a commit to isaaccolson/deliveries-mobile that referenced this pull request Oct 30, 2024
* feat: Add `onStarted` and `onStopped` events

* Implement `onStart` for Android

* Update CameraSession.kt

* Update CameraSessionDelegate.swift
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.

3 participants