From c1a4f5f23a4b4e55320a5b94455f3b59ff38835d Mon Sep 17 00:00:00 2001 From: PetkoKuzmanov Date: Tue, 3 Nov 2026 18:56:43 +0200 Subject: [PATCH 1/3] Android hit tests: hold the result listener with a global ref performARHitTestWithPoint, the ray, position and Quest OpenXR hit tests and the scene hit test kept the bridge's one-shot listener only as a weak global ref. Nothing else holds that listener, so the GC could collect it before the renderer thread answered, onHitTestFinished never ran and the JS promise stayed pending. On a Pixel 6 Pro this happened about 11 s into a recording that hit-tests every 250 ms. The listener is now a global ref, deleted after the callback runs. With the rebuilt libviro_renderer.so in a host app, 120 probes all answered. --- CHANGELOG.md | 6 ++ .../main/cpp/jni/VRORendererARCore_JNI.cpp | 66 +++++++++---------- .../src/main/cpp/jni/VRORenderer_JNI.cpp | 59 ++++++++--------- 3 files changed, 68 insertions(+), 63 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2c69afa10..84ccd2b68 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # CHANGELOG +## Unreleased + +### Fixed + +- **Android hit tests always deliver their results (`VRORendererARCore_JNI`, `VRORenderer_JNI`).** The AR point, ray and position hit tests, the Quest OpenXR hit tests and the scene hit test kept the caller's one-shot listener only as a weak global reference, so the garbage collector could take it before the renderer thread answered, and the results never arrived. It is now a global reference, deleted once the callback has run. + ## v3.1.0 — 27 October 2026 ### Added diff --git a/android/sharedCode/src/main/cpp/jni/VRORendererARCore_JNI.cpp b/android/sharedCode/src/main/cpp/jni/VRORendererARCore_JNI.cpp index 5710594c9..d5d9ba9d0 100644 --- a/android/sharedCode/src/main/cpp/jni/VRORendererARCore_JNI.cpp +++ b/android/sharedCode/src/main/cpp/jni/VRORendererARCore_JNI.cpp @@ -115,15 +115,17 @@ VRO_METHOD(void, nativeSetAnchorDetectionTypes)(VRO_ARGS } -void invokeEmptyARResultsCallback(jweak weakCallback); +void invokeEmptyARResultsCallback(jobject callbackRef); -void invokeARResultsCallback(std::vector> &results, jweak weakCallback) { +// callbackRef is a global ref, deleted once the callback runs. Callers pass a one-shot listener +// nothing else holds, so a weak ref let the GC collect it first and the result never arrived. +void invokeARResultsCallback(std::vector> &results, jobject callbackRef) { JNIEnv *env = VROPlatformGetJNIEnv(); // Runs on the renderer thread, which on Quest is a native OpenXR thread: plain FindClass // does not see app classes there, and the pending ClassNotFoundException aborted the app. jclass arHitTestResultClass = VROPlatformFindHostClass(env, "com/viro/core/ARHitTestResult"); if (arHitTestResultClass == nullptr) { - invokeEmptyARResultsCallback(weakCallback); + invokeEmptyARResultsCallback(callbackRef); return; } @@ -137,62 +139,60 @@ void invokeARResultsCallback(std::vector> &r jobject globalArrayRef = env->NewGlobalRef(resultsArray); env->DeleteLocalRef(resultsArray); env->DeleteLocalRef(arHitTestResultClass); - VROPlatformDispatchAsyncApplication([weakCallback, globalArrayRef] { + VROPlatformDispatchAsyncApplication([callbackRef, globalArrayRef] { JNIEnv *env = VROPlatformGetJNIEnv(); - jobject callback = env->NewLocalRef(weakCallback); - VROPlatformCallHostFunction(callback, "onHitTestFinished", + VROPlatformCallHostFunction(callbackRef, "onHitTestFinished", "([Lcom/viro/core/ARHitTestResult;)V", globalArrayRef); env->DeleteGlobalRef(globalArrayRef); - env->DeleteWeakGlobalRef(weakCallback); + env->DeleteGlobalRef(callbackRef); }); } -void invokeEmptyARResultsCallback(jweak weakCallback) { - VROPlatformDispatchAsyncApplication([weakCallback] { +void invokeEmptyARResultsCallback(jobject callbackRef) { + VROPlatformDispatchAsyncApplication([callbackRef] { JNIEnv *env = VROPlatformGetJNIEnv(); - jobject callback = env->NewLocalRef(weakCallback); jclass arHitTestResultClass = env->FindClass("com/viro/core/ARHitTestResult"); jobjectArray emptyArray = env->NewObjectArray(0, arHitTestResultClass, NULL); - VROPlatformCallHostFunction(callback, "onHitTestFinished", + VROPlatformCallHostFunction(callbackRef, "onHitTestFinished", "([Lcom/viro/core/ARHitTestResult;)V", emptyArray); - env->DeleteWeakGlobalRef(weakCallback); + env->DeleteGlobalRef(callbackRef); }); } void performARHitTest(VROVector3f rayVec, std::weak_ptr arRenderer_w, - jweak weakCallback) { + jobject callbackRef) { std::shared_ptr arRenderer = arRenderer_w.lock(); if (!arRenderer) { - invokeEmptyARResultsCallback(weakCallback); + invokeEmptyARResultsCallback(callbackRef); } else { std::vector> results = arRenderer->performARHitTest(rayVec); - invokeARResultsCallback(results, weakCallback); + invokeARResultsCallback(results, callbackRef); } } void performARHitTestRay(VROVector3f rayOrigin, VROVector3f rayDestination, std::weak_ptr arRenderer_w, - jweak weakCallback) { + jobject callbackRef) { std::shared_ptr arRenderer = arRenderer_w.lock(); if (!arRenderer) { - invokeEmptyARResultsCallback(weakCallback); + invokeEmptyARResultsCallback(callbackRef); } else { std::vector> results = arRenderer->performARHitTest(rayOrigin, rayDestination); - invokeARResultsCallback(results, weakCallback); + invokeARResultsCallback(results, callbackRef); } } void performARHitTestPoint(JNIEnv *env, float x, float y, std::weak_ptr arRenderer_w, - jweak weakCallback) { + jobject callbackRef) { std::shared_ptr arRenderer = arRenderer_w.lock(); if (!arRenderer) { - invokeEmptyARResultsCallback(weakCallback); + invokeEmptyARResultsCallback(callbackRef); } else { std::vector> results = arRenderer->performARHitTest(x, y); - invokeARResultsCallback(results, weakCallback); + invokeARResultsCallback(results, callbackRef); } } @@ -208,10 +208,10 @@ VRO_METHOD(void, nativePerformARHitTestWithRay) (VRO_ARGS // Create weak pointers for dispatching std::shared_ptr renderer = Renderer::native(native_renderer); std::weak_ptr arRenderer_w = std::dynamic_pointer_cast(renderer); - jweak weakCallback = env->NewWeakGlobalRef(callback); + jobject callbackRef = env->NewGlobalRef(callback); - VROPlatformDispatchAsyncRenderer([arRenderer_w, weakCallback, rayVec] { - performARHitTest(rayVec, arRenderer_w, weakCallback); + VROPlatformDispatchAsyncRenderer([arRenderer_w, callbackRef, rayVec] { + performARHitTest(rayVec, arRenderer_w, callbackRef); }); } @@ -233,10 +233,10 @@ VRO_METHOD(void, nativePerformARHitTestWithOriginDestRay) (VRO_ARGS // Create weak pointers for dispatching std::shared_ptr renderer = Renderer::native(native_renderer); std::weak_ptr arRenderer_w = std::dynamic_pointer_cast(renderer); - jweak weakCallback = env->NewWeakGlobalRef(callback); + jobject callbackRef = env->NewGlobalRef(callback); - VROPlatformDispatchAsyncRenderer([arRenderer_w, weakCallback, rayOrigVec, rayDestVec] { - performARHitTestRay(rayOrigVec, rayDestVec, arRenderer_w, weakCallback); + VROPlatformDispatchAsyncRenderer([arRenderer_w, callbackRef, rayOrigVec, rayDestVec] { + performARHitTestRay(rayOrigVec, rayDestVec, arRenderer_w, callbackRef); }); } @@ -257,10 +257,10 @@ VRO_METHOD(void, nativePerformARHitTestWithPosition) (VRO_ARGS // Create weak pointers for dispatching std::weak_ptr arRenderer_w = std::dynamic_pointer_cast(renderer); - jweak weakCallback = env->NewWeakGlobalRef(callback); + jobject callbackRef = env->NewGlobalRef(callback); - VROPlatformDispatchAsyncRenderer([arRenderer_w, weakCallback, rayVec] { - performARHitTest(rayVec, arRenderer_w, weakCallback); + VROPlatformDispatchAsyncRenderer([arRenderer_w, callbackRef, rayVec] { + performARHitTest(rayVec, arRenderer_w, callbackRef); }); } @@ -270,10 +270,10 @@ VRO_METHOD(void, nativePerformARHitTestWithPoint) (VRO_ARGS jobject callback) { std::shared_ptr renderer = Renderer::native(native_renderer); std::weak_ptr arRenderer_w = std::dynamic_pointer_cast(renderer); - jweak weakCallback = env->NewWeakGlobalRef(callback); + jobject callbackRef = env->NewGlobalRef(callback); - VROPlatformDispatchAsyncRenderer([env, arRenderer_w, weakCallback, x, y] { - performARHitTestPoint(env, x, y, arRenderer_w, weakCallback); + VROPlatformDispatchAsyncRenderer([env, arRenderer_w, callbackRef, x, y] { + performARHitTestPoint(env, x, y, arRenderer_w, callbackRef); }); } diff --git a/android/sharedCode/src/main/cpp/jni/VRORenderer_JNI.cpp b/android/sharedCode/src/main/cpp/jni/VRORenderer_JNI.cpp index 6892f253d..7c2a61510 100644 --- a/android/sharedCode/src/main/cpp/jni/VRORenderer_JNI.cpp +++ b/android/sharedCode/src/main/cpp/jni/VRORenderer_JNI.cpp @@ -220,8 +220,8 @@ VRO_METHOD(jint, nativeGetEyeHeight)(VRO_ARGS // Defined in VRORendererARCore_JNI.cpp: convert AR hit results to // com.viro.core.ARHitTestResult[] and deliver them on the application thread. -void invokeARResultsCallback(std::vector> &results, jweak weakCallback); -void invokeEmptyARResultsCallback(jweak weakCallback); +void invokeARResultsCallback(std::vector> &results, jobject callbackRef); +void invokeEmptyARResultsCallback(jobject callbackRef); // AR hit test on Quest, against the planes the OpenXR AR session tracks. Runs on // the renderer thread, where those planes are updated. `origin` null means the @@ -229,7 +229,7 @@ void invokeEmptyARResultsCallback(jweak weakCallback); // performARHitTestWithRay(ray) takes it. static void performARHitTestOpenXR(JNIEnv *env, jlong rendererRef, jfloatArray origin, jfloatArray destination, jobject callback) { - jweak weakCallback = env->NewWeakGlobalRef(callback); + jobject callbackRef = env->NewGlobalRef(callback); auto xrRenderer_w = std::weak_ptr( std::dynamic_pointer_cast(Renderer::native(rendererRef))); @@ -244,16 +244,16 @@ static void performARHitTestOpenXR(JNIEnv *env, jlong rendererRef, jfloatArray o VROVector3f destVec = VROVector3f(d[0], d[1], d[2]); VRO_FLOAT_ARRAY_RELEASE_ELEMENTS(destination, d); - VROPlatformDispatchAsyncRenderer([xrRenderer_w, weakCallback, fromCamera, originVec, destVec] { + VROPlatformDispatchAsyncRenderer([xrRenderer_w, callbackRef, fromCamera, originVec, destVec] { std::shared_ptr xrRenderer = xrRenderer_w.lock(); if (!xrRenderer) { - invokeEmptyARResultsCallback(weakCallback); + invokeEmptyARResultsCallback(callbackRef); return; } std::vector> results = fromCamera ? xrRenderer->performARHitTestWithRay(destVec) : xrRenderer->performARHitTest(originVec, destVec); - invokeARResultsCallback(results, weakCallback); + invokeARResultsCallback(results, callbackRef); }); } @@ -738,14 +738,15 @@ VRO_METHOD(void, nativeSetBloomEnabled)(VRO_ARGS } -void invokeEmptyHitTestResultsCallback(jweak weakCallback); +void invokeEmptyHitTestResultsCallback(jobject callbackRef); -void invokeHitTestResultsCallback(std::vector &results, jweak weakCallback) { +// callbackRef is a global ref, deleted once the callback runs; see invokeARResultsCallback. +void invokeHitTestResultsCallback(std::vector &results, jobject callbackRef) { JNIEnv *env = VROPlatformGetJNIEnv(); // Runs on the renderer thread; see VROPlatformFindHostClass. jclass hitTestResultClass = VROPlatformFindHostClass(env, "com/viro/core/HitTestResult"); if (hitTestResultClass == nullptr) { - invokeEmptyHitTestResultsCallback(weakCallback); + invokeEmptyHitTestResultsCallback(callbackRef); return; } @@ -759,51 +760,49 @@ void invokeHitTestResultsCallback(std::vector &results, jweak jobject globalArrayRef = env->NewGlobalRef(resultsArray); env->DeleteLocalRef(resultsArray); env->DeleteLocalRef(hitTestResultClass); - VROPlatformDispatchAsyncApplication([weakCallback, globalArrayRef] { + VROPlatformDispatchAsyncApplication([callbackRef, globalArrayRef] { JNIEnv *env = VROPlatformGetJNIEnv(); - jobject callback = env->NewLocalRef(weakCallback); - VROPlatformCallHostFunction(callback, "onHitTestFinished", + VROPlatformCallHostFunction(callbackRef, "onHitTestFinished", "([Lcom/viro/core/HitTestResult;)V", globalArrayRef); env->DeleteGlobalRef(globalArrayRef); - env->DeleteWeakGlobalRef(weakCallback); + env->DeleteGlobalRef(callbackRef); }); } -void invokeEmptyHitTestResultsCallback(jweak weakCallback) { - VROPlatformDispatchAsyncApplication([weakCallback] { +void invokeEmptyHitTestResultsCallback(jobject callbackRef) { + VROPlatformDispatchAsyncApplication([callbackRef] { JNIEnv *env = VROPlatformGetJNIEnv(); - jobject callback = env->NewLocalRef(weakCallback); jclass arHitTestResultClass = env->FindClass("com/viro/core/HitTestResult"); jobjectArray emptyArray = env->NewObjectArray(0, arHitTestResultClass, NULL); - VROPlatformCallHostFunction(callback, "onHitTestFinished", + VROPlatformCallHostFunction(callbackRef, "onHitTestFinished", "([Lcom/viro/core/HitTestResult;)V", emptyArray); - env->DeleteWeakGlobalRef(weakCallback); + env->DeleteGlobalRef(callbackRef); }); } void performHitTestRay(VROVector3f origin, VROVector3f ray, bool boundsOnly, std::weak_ptr renderer_w, - jweak weakCallback) { + jobject callbackRef) { std::shared_ptr renderer = renderer_w.lock(); if (!renderer) { - invokeEmptyHitTestResultsCallback(weakCallback); + invokeEmptyHitTestResultsCallback(callbackRef); } else { std::vector results = renderer->performHitTest(origin, ray, boundsOnly); - invokeHitTestResultsCallback(results, weakCallback); + invokeHitTestResultsCallback(results, callbackRef); } } void performHitTestPoint(float x, float y, bool boundsOnly, std::weak_ptr renderer_w, - jweak weakCallback) { + jobject callbackRef) { std::shared_ptr renderer = renderer_w.lock(); if (!renderer) { - invokeEmptyHitTestResultsCallback(weakCallback); + invokeEmptyHitTestResultsCallback(callbackRef); } else { std::vector results = renderer->performHitTest(x, y, boundsOnly); - invokeHitTestResultsCallback(results, weakCallback); + invokeHitTestResultsCallback(results, callbackRef); } } @@ -817,10 +816,10 @@ VRO_METHOD(void, nativePerformHitTestWithPoint) (VRO_ARGS pinfo("TEST_nativePerformHitTestWithPoint IS NULL!!"); } - jweak weakCallback = env->NewWeakGlobalRef(callback); - VROPlatformDispatchAsyncRenderer([env, renderer_w, boundsOnly, weakCallback, x, y] { + jobject callbackRef = env->NewGlobalRef(callback); + VROPlatformDispatchAsyncRenderer([env, renderer_w, boundsOnly, callbackRef, x, y] { std::shared_ptr sceneRenderer = renderer_w.lock(); - performHitTestPoint(x, y, boundsOnly, sceneRenderer, weakCallback); + performHitTestPoint(x, y, boundsOnly, sceneRenderer, callbackRef); }); } @@ -842,10 +841,10 @@ VRO_METHOD(void, nativePerformHitTestWithRay) (VRO_ARGS // Create weak pointers for dispatching std::weak_ptr renderer_w = Renderer::native(native_renderer); - jweak weakCallback = env->NewWeakGlobalRef(callback); + jobject callbackRef = env->NewGlobalRef(callback); - VROPlatformDispatchAsyncRenderer([renderer_w, boundsOnly, weakCallback, originVec, rayVec] { - performHitTestRay(originVec, rayVec, boundsOnly, renderer_w, weakCallback); + VROPlatformDispatchAsyncRenderer([renderer_w, boundsOnly, callbackRef, originVec, rayVec] { + performHitTestRay(originVec, rayVec, boundsOnly, renderer_w, callbackRef); }); } From f5407868b659a6023b442dab134738e3ac78f9d6 Mon Sep 17 00:00:00 2001 From: PetkoKuzmanov Date: Tue, 3 Nov 2026 20:26:58 +0200 Subject: [PATCH 2/3] Return what a recording wrote from stopRecording, and whether depth can be recorded VROARSession::stopRecording takes a completion receiving a VROARRecordingResult (success, error, durationSec, frames, depthFrames, gpsFixes, bytes), called once video.mp4, session.jsonl and any depth files are complete; the no-argument form remains. ARScene.stopRecording() returns an ARScene.RecordingResult with the same fields. isRecordingDepthSupported() answers LiDAR scene depth on iOS and the Depth API on ARCore, and ARScene.isRecordingSupported(boolean depth) exposes it on Android. Android reports an encoder that did not finish as a failure, as iOS does. A failure while stopping goes into the result only: it used to reach the scene after the stop had reset the status, leaving getRecordingStatus() at IOError with nothing recording. ARScene.stopRecording() may run off the UI thread. On iOS the pose counters are kept under the sidecar lock, counting only lines actually written. --- CHANGELOG.md | 7 ++ ViroRenderer/VROARSession.h | 41 +++++++++++- .../main/cpp/arcore/ARSceneController_JNI.cpp | 16 +++++ .../main/cpp/arcore/VROARSessionARCore.cpp | 17 ++++- .../src/main/cpp/arcore/VROARSessionARCore.h | 8 ++- .../src/main/java/com/viro/core/ARScene.java | 63 +++++++++++++++-- .../viro/core/internal/ARSessionRecorder.java | 67 +++++++++++++++++-- ios/ViroKit/VROARSessionRecorderIOS.h | 16 ++++- ios/ViroKit/VROARSessionRecorderIOS.mm | 65 +++++++++++++++++- ios/ViroKit/VROARSessioniOS.cpp | 17 ++++- ios/ViroKit/VROARSessioniOS.h | 4 +- 11 files changed, 295 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 84ccd2b68..5866d196e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,8 +2,15 @@ ## Unreleased +### Added + +- **`VROARSession::stopRecording` takes a completion receiving a `VROARRecordingResult`** (`success`, `error`, `durationSec`, `frames`, `depthFrames`, `gpsFixes`, `bytes`), called once the files are complete. The no-argument form remains. On Android, `ARScene.stopRecording()` returns an `ARScene.RecordingResult` with the same fields. +- **`VROARSession::isRecordingDepthSupported()` and `ARScene.isRecordingSupported(boolean depth)`.** iOS checks for LiDAR scene depth and ARCore for the Depth API. On Android `ARScene.isRecordingSupported` answers false, with or without depth, until the ARCore session exists. + ### Fixed +- **Android reports a recording whose encoder did not finish as failed.** It logged a warning and the recording counted as complete, while iOS reports the same case as an error. +- **Android: a failure while a recording is being stopped no longer leaves `getRecordingStatus()` at IOError (`ARSessionRecorder`).** `encoder.stop()` and `muxer.stop()` failures reported it to the scene after the stop had already set the status back to None, so the status read IOError with nothing recording. Such a failure is now reported in the stop's result only. `ARScene.stopRecording()` may be called off the UI thread. - **Android hit tests always deliver their results (`VRORendererARCore_JNI`, `VRORenderer_JNI`).** The AR point, ray and position hit tests, the Quest OpenXR hit tests and the scene hit test kept the caller's one-shot listener only as a weak global reference, so the garbage collector could take it before the renderer thread answered, and the results never arrived. It is now a global reference, deleted once the callback has run. ## v3.1.0 — 27 October 2026 diff --git a/ViroRenderer/VROARSession.h b/ViroRenderer/VROARSession.h index e722c6ced..18b50b919 100644 --- a/ViroRenderer/VROARSession.h +++ b/ViroRenderer/VROARSession.h @@ -27,6 +27,7 @@ #ifndef VROARSession_h #define VROARSession_h +#include #include #include #include @@ -148,6 +149,20 @@ struct VROARRecordingConfig { bool depth = false; }; +/* + What a finished recording wrote, passed to stopRecording()'s callback once + video.mp4, session.jsonl and any depth files are complete. + */ +struct VROARRecordingResult { + bool success = false; + std::string error; // Empty on success + double durationSec = 0; // First to last recorded frame + int frames = 0; // Camera frames, one `pose` line each + int depthFrames = 0; // `depth` lines (PNG pairs) written + int gpsFixes = 0; // `pose` lines carrying a `gps` object + int64_t bytes = -1; // Size of every file written; < 0 when unknown +}; + /* Manages the device camera and motion tracking for AR. */ @@ -981,6 +996,14 @@ class VROARSession { return false; } + /* + Returns true if this session can also record depth (see + VROARRecordingConfig::depth): LiDAR on iOS, the Depth API on ARCore. + */ + virtual bool isRecordingDepthSupported() const { + return false; + } + /* Start recording video + IMU + ground-truth pose/anchors to config.outputDir. onSuccess is invoked once recording has actually started (files created, @@ -996,10 +1019,22 @@ class VROARSession { } /* - Stop recording and finalize video.mp4 + session.jsonl. No-op if not - currently recording. + Stop recording and finalize video.mp4 + session.jsonl. onStopped is invoked + once the files are complete, with success false (error "Not recording") + if no recording was running. May block the calling thread while the + writers finish. */ - virtual void stopRecording() {} + virtual void stopRecording(std::function onStopped) { + if (onStopped) { + VROARRecordingResult result; + result.error = "Recording not supported"; + onStopped(result); + } + } + + void stopRecording() { + stopRecording(nullptr); + } /* The current recording state. See VROARRecordingStatus. diff --git a/android/sharedCode/src/main/cpp/arcore/ARSceneController_JNI.cpp b/android/sharedCode/src/main/cpp/arcore/ARSceneController_JNI.cpp index cb9b47d58..124f7a889 100644 --- a/android/sharedCode/src/main/cpp/arcore/ARSceneController_JNI.cpp +++ b/android/sharedCode/src/main/cpp/arcore/ARSceneController_JNI.cpp @@ -1971,6 +1971,22 @@ VRO_METHOD(VRO_INT, nativeGetRecordingStatus)(VRO_ARGS return (VRO_INT) session->getRecordingStatus(); } +VRO_METHOD(VRO_BOOL, nativeIsRecordingSupported)(VRO_ARGS + VRO_REF(VROARSceneController) sceneController_j, + VRO_BOOL depth) { + VRO_METHOD_PREAMBLE; + std::shared_ptr scene = std::dynamic_pointer_cast( + VRO_REF_GET(VROARSceneController, sceneController_j)->getScene()); + if (!scene) { + return false; + } + std::shared_ptr session = scene->getARSession(); + if (!session || !session->isRecordingSupported()) { + return false; + } + return !depth || session->isRecordingDepthSupported(); +} + // Called by Java (ARSessionRecorder, via ARScene) when the encoder/sidecar // hits an IOException mid-recording — flips native's getRecordingStatus() to // IOError so the JS layer can surface it instead of silently losing frames. diff --git a/android/sharedCode/src/main/cpp/arcore/VROARSessionARCore.cpp b/android/sharedCode/src/main/cpp/arcore/VROARSessionARCore.cpp index b5222f754..168462025 100644 --- a/android/sharedCode/src/main/cpp/arcore/VROARSessionARCore.cpp +++ b/android/sharedCode/src/main/cpp/arcore/VROARSessionARCore.cpp @@ -3149,10 +3149,25 @@ void VROARSessionARCore::startRecording(const VROARRecordingConfig &config, } } -void VROARSessionARCore::stopRecording() { +bool VROARSessionARCore::isRecordingDepthSupported() const { + return _session != nullptr && _session->isDepthModeSupported(arcore::DepthMode::Automatic); +} + +void VROARSessionARCore::stopRecording(std::function onStopped) { + VROARRecordingStatus status = _recordingStatus; _recordingStatus = VROARRecordingStatus::None; _recordingDepth = false; setRecordingJavaCallback(nullptr); + if (onStopped) { + VROARRecordingResult result; + if (status == VROARRecordingStatus::Recording) { + result.success = true; + } else { + result.error = status == VROARRecordingStatus::IOError ? "Recording stopped after a write error" + : "Not recording"; + } + onStopped(result); + } } VROARRecordingStatus VROARSessionARCore::getRecordingStatus() const { diff --git a/android/sharedCode/src/main/cpp/arcore/VROARSessionARCore.h b/android/sharedCode/src/main/cpp/arcore/VROARSessionARCore.h index e3ce1c63c..8f40d6e28 100644 --- a/android/sharedCode/src/main/cpp/arcore/VROARSessionARCore.h +++ b/android/sharedCode/src/main/cpp/arcore/VROARSessionARCore.h @@ -305,10 +305,16 @@ class VROARSessionARCore : public VROARSession, recordFrameForRecording() and setRecordingJavaCallback() below. */ bool isRecordingSupported() const override { return true; } + bool isRecordingDepthSupported() const override; void startRecording(const VROARRecordingConfig &config, std::function onSuccess, std::function onFailure) override; - void stopRecording() override; + /* + The files are written in Java, so the result passed here carries only + success; ARScene.stopRecording() returns the counts and sizes. + */ + using VROARSession::stopRecording; + void stopRecording(std::function onStopped) override; VROARRecordingStatus getRecordingStatus() const override; /* diff --git a/android/sharedCode/src/main/java/com/viro/core/ARScene.java b/android/sharedCode/src/main/java/com/viro/core/ARScene.java index 24e1616c8..617ffcd2a 100644 --- a/android/sharedCode/src/main/java/com/viro/core/ARScene.java +++ b/android/sharedCode/src/main/java/com/viro/core/ARScene.java @@ -178,6 +178,38 @@ public enum RecordingStatus { NONE, RECORDING, IO_ERROR, UNSUPPORTED } + /** + * What a finished recording wrote, returned by {@link #stopRecording()} once video.mp4, + * session.jsonl and any depth files are complete. + */ + public static final class RecordingResult { + /** True when every file was written and closed and at least one frame was recorded. */ + public final boolean success; + /** Why the recording failed, or null on success. */ + public final String error; + /** Seconds from the first to the last recorded frame. */ + public final double durationSec; + /** Camera frames recorded, one {@code pose} line each in session.jsonl. */ + public final int frames; + /** Depth frames written under depth/; 0 for a recording without depth. */ + public final int depthFrames; + /** {@code pose} lines carrying a {@code gps} object. */ + public final int gpsFixes; + /** Size in bytes of every file written, or -1 when unknown. */ + public final long bytes; + + public RecordingResult(boolean success, String error, double durationSec, int frames, + int depthFrames, int gpsFixes, long bytes) { + this.success = success; + this.error = error; + this.durationSec = durationSec; + this.frames = frames; + this.depthFrames = depthFrames; + this.gpsFixes = gpsFixes; + this.bytes = bytes; + } + } + /** * Callback interface for {@link #startRecording(String, Context, RecordingStartListener)}. */ @@ -499,7 +531,8 @@ public int getId() { // AR Session Recording API state. mSessionRecorder does the actual work // (MediaCodec/MediaMuxer/SensorManager/file I/O — see ARSessionRecorder); // this class only bridges native's per-frame calls to it. - private com.viro.core.internal.ARSessionRecorder mSessionRecorder; + // stopRecording() may run off the UI thread. + private volatile com.viro.core.internal.ARSessionRecorder mSessionRecorder; private RecordingStartListener mRecordingStartListener; // Geospatial API state @@ -1058,14 +1091,31 @@ public void onRecordingError(String message) { /** * Stop the recording started by {@link #startRecording(String, Context, RecordingStartListener)} - * and finalize video.mp4 + session.jsonl. No-op if not currently recording. + * and finalize video.mp4 + session.jsonl. Blocks until the files are complete. + * + * @return What was written. When no recording was running, success is false and the + * error is "Not recording". */ - public void stopRecording() { + public RecordingResult stopRecording() { nativeStopRecording(mNativeRef); - if (mSessionRecorder != null) { - mSessionRecorder.stop(); - mSessionRecorder = null; + if (mSessionRecorder == null) { + return new RecordingResult(false, "Not recording", 0, 0, 0, 0, -1); + } + RecordingResult result = mSessionRecorder.stop(); + mSessionRecorder = null; + return result; + } + + /** + * Whether this session can record, and with depth true, also record depth (the device + * supports the ARCore Depth API). Depth is only recorded if + * {@link #setDepthCaptureEnabled(boolean)} was also called with true. + */ + public boolean isRecordingSupported(boolean depth) { + if (mNativeRef == 0) { + return false; } + return nativeIsRecordingSupported(mNativeRef, depth); } /** The current recording state. See {@link RecordingStatus}. */ @@ -2302,6 +2352,7 @@ private native void nativeSetWorldMeshConfig(long sceneControllerRef, private native void nativeStartRecording(long sceneControllerRef, String outputDir, boolean depth); private native void nativeStopRecording(long sceneControllerRef); private native int nativeGetRecordingStatus(long sceneControllerRef); + private native boolean nativeIsRecordingSupported(long sceneControllerRef, boolean depth); private native void nativeReportRecordingError(long sceneControllerRef, String error); // Called by JNI diff --git a/android/sharedCode/src/main/java/com/viro/core/internal/ARSessionRecorder.java b/android/sharedCode/src/main/java/com/viro/core/internal/ARSessionRecorder.java index ff6a00929..27af101ed 100644 --- a/android/sharedCode/src/main/java/com/viro/core/internal/ARSessionRecorder.java +++ b/android/sharedCode/src/main/java/com/viro/core/internal/ARSessionRecorder.java @@ -35,6 +35,8 @@ import android.os.Looper; import android.util.Log; +import com.viro.core.ARScene; + import java.io.File; import java.io.FileOutputStream; import java.io.FileWriter; @@ -45,6 +47,7 @@ import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicLong; public class ARSessionRecorder { @@ -70,6 +73,15 @@ public interface ErrorListener { private boolean mWroteHeader; private final Object mSidecarLock = new Object(); private long mFirstTimestampNs = -1; + private long mLastTimestampNs = -1; + // `pose` lines written, and those of them carrying a `gps` object. + private int mPoseCount; + private int mGpsFixCount; + // The first failure reportError() saw; the recording's result is a failure when set. + private volatile String mFirstError; + // Set once stop() begins: its errors go into the result, not to the listener, which + // would mark a session that has stopped recording as failed. + private volatile boolean mStopping; // Raw IMU (SensorManager.TYPE_ACCELEROMETER/TYPE_GYROSCOPE — NOT the fused // rotation-vector sensor VRTARSceneNavigator uses elsewhere for heading). @@ -105,6 +117,9 @@ public interface ErrorListener { private ExecutorService mDepthExecutor; private final AtomicInteger mPendingDepthFrames = new AtomicInteger(); private int mNextDepthIndex = 0; + // Depth frames whose PNGs were written and line buffered, and the PNGs' size. + private final AtomicInteger mDepthFramesWritten = new AtomicInteger(); + private final AtomicLong mDepthBytesWritten = new AtomicLong(); public ARSessionRecorder(Context context, ErrorListener errorListener) { mContext = context; @@ -119,9 +134,16 @@ public synchronized void start(String outputDir) throws IOException { mOutputDir = outputDir; mWroteHeader = false; mFirstTimestampNs = -1; + mLastTimestampNs = -1; + mPoseCount = 0; + mGpsFixCount = 0; + mFirstError = null; + mStopping = false; mHaveGyro = false; mDepthActive = false; mNextDepthIndex = 0; + mDepthFramesWritten.set(0); + mDepthBytesWritten.set(0); mSidecar = new FileWriter(new File(dir, "session.jsonl"), false); @@ -193,7 +215,12 @@ public synchronized void setDepthActive(boolean active) { } } - public synchronized void stop() { + /** + * Stops recording and blocks until video.mp4, session.jsonl and any depth files are + * complete, then returns what was written. + */ + public synchronized ARScene.RecordingResult stop() { + mStopping = true; if (mSensorManager != null && mSensorListener != null) { mSensorManager.unregisterListener(mSensorListener); } @@ -238,6 +265,22 @@ public synchronized void stop() { drainDepthWrites(); flushBufferedLinesSorted(); closeSidecarQuietly(); + return buildResult(); + } + + private ARScene.RecordingResult buildResult() { + long bytes = mDepthBytesWritten.get(); + if (mOutputDir != null) { + bytes += new File(mOutputDir, "video.mp4").length() + new File(mOutputDir, "session.jsonl").length(); + } + double durationSec = mFirstTimestampNs >= 0 && mLastTimestampNs >= mFirstTimestampNs + ? (mLastTimestampNs - mFirstTimestampNs) / 1e9 : 0; + String error = mFirstError; + if (error == null && mPoseCount == 0) { + error = "No frames were recorded"; + } + return new ARScene.RecordingResult(error == null, error, durationSec, mPoseCount, + mDepthFramesWritten.get(), mGpsFixCount, bytes); } // Every depth line must be buffered before the sorted flush, so wait for the queue. @@ -276,7 +319,10 @@ private void closeSidecarQuietly() { private void reportError(String message) { Log.e(TAG, "ARSessionRecorder: " + message); - if (mErrorListener != null) { + if (mFirstError == null) { + mFirstError = message; + } + if (mErrorListener != null && !mStopping) { mErrorListener.onRecordingError(message); } } @@ -312,6 +358,7 @@ public synchronized void onRecordingFrame(byte[] y, byte[] u, byte[] v, int[] di if (mFirstTimestampNs < 0) { mFirstTimestampNs = timestampNs; } + mLastTimestampNs = timestampNs; writeHeaderIfNeeded(width, height, pose[7], pose[8], pose[9], pose[10]); writePoseLine(timestampNs, pose, geo); @@ -379,9 +426,11 @@ && isFinite(geo[1]) && isFinite(geo[2]) && isFinite(geo[3])) { ",\"gps\":{\"lat\":%.8f,\"lon\":%.8f,\"alt\":%.3f," + "\"h_accuracy\":%s,\"heading\":%s,\"heading_accuracy\":%s}", geo[1], geo[2], geo[3], hAcc, heading, headingAcc)); + mGpsFixCount++; } sb.append("}"); bufferLine(timestampNs, sb.toString()); + mPoseCount++; } // Intrinsics are the camera image's; the depth map is a different size, so each axis is @@ -411,11 +460,13 @@ private void enqueueDepthFrame(final byte[] depth, final byte[] confidence, executor.execute(new Runnable() { @Override public void run() { + long pngBytes; try { - writeFile(new File(outputDir, depthName), - PngGrayEncoder.encodeGray16(depth, depthWidth, depthHeight, true)); - writeFile(new File(outputDir, confidenceName), - PngGrayEncoder.encodeGray8(confidence, depthWidth, depthHeight)); + byte[] depthPng = PngGrayEncoder.encodeGray16(depth, depthWidth, depthHeight, true); + byte[] confidencePng = PngGrayEncoder.encodeGray8(confidence, depthWidth, depthHeight); + writeFile(new File(outputDir, depthName), depthPng); + writeFile(new File(outputDir, confidenceName), confidencePng); + pngBytes = depthPng.length + confidencePng.length; } catch (Exception e) { Log.w(TAG, "ARSessionRecorder: depth frame " + depthName + " not written", e); new File(outputDir, depthName).delete(); @@ -431,6 +482,8 @@ public void run() { depthTimestampNs, depthName, confidenceName, depthWidth, depthHeight, dfx, dfy, dcx, dcy); bufferLine(depthTimestampNs, line); + mDepthFramesWritten.incrementAndGet(); + mDepthBytesWritten.addAndGet(pngBytes); } }); } @@ -684,7 +737,7 @@ private void drainEncoder(boolean endOfStream) { continue; // the encoder is still flushing the frames before the end } else { if (waitForEnd) { - Log.w(TAG, "ARSessionRecorder: encoder did not finish; the last frames may be missing"); + reportError("encoder did not finish; the last frames may be missing"); } break; // INFO_TRY_AGAIN_LATER or INFO_OUTPUT_BUFFERS_CHANGED (deprecated path, ignored) } diff --git a/ios/ViroKit/VROARSessionRecorderIOS.h b/ios/ViroKit/VROARSessionRecorderIOS.h index c77073d4e..a94c183b3 100644 --- a/ios/ViroKit/VROARSessionRecorderIOS.h +++ b/ios/ViroKit/VROARSessionRecorderIOS.h @@ -57,9 +57,10 @@ class API_AVAILABLE(ios(12.0)) VROARSessionRecorderIOS { /* Stop recording: stop the IMU taps, finish the AVAssetWriter, close the - sidecar. Safe to call when not recording (no-op). + sidecar. Blocks until the files are complete and returns what was written. + Safe to call when not recording: returns success false, "Not recording". */ - void stop(); + VROARRecordingResult stop(); VROARRecordingStatus getStatus() const; @@ -114,6 +115,9 @@ class API_AVAILABLE(ios(12.0)) VROARSessionRecorderIOS { private: VROARRecordingStatus _status; + // True from a successful start() until stop(); _status alone cannot tell, since a + // failed finish leaves it at IOError after the recording has ended. + bool _running; std::string _outputDir; double _startTimestamp; // ARFrame.timestamp of the first recorded frame, for relative t @@ -129,6 +133,11 @@ class API_AVAILABLE(ios(12.0)) VROARSessionRecorderIOS { size_t _videoHeight; // Last recorded ARFrame.timestamp; drops duplicates so poses match frames 1:1. double _lastFrameTimestamp; + // `pose` lines buffered, those of them carrying a `gps` object, and the last one's + // ARFrame.timestamp. Guarded by _sidecarMutex: stop() reads them off the AR thread. + int _poseCount; + int _gpsFixCount; + double _lastPoseTimestamp; // --- Depth (ARFrame.sceneDepth on LiDAR devices; PNGs written off the AR thread) --- bool _depthRequested; @@ -139,6 +148,9 @@ class API_AVAILABLE(ios(12.0)) VROARSessionRecorderIOS { dispatch_queue_t _depthQueue; // Depth frames handed to _depthQueue and not yet written; bounds memory if the disk stalls. std::atomic _depthPending; + // Depth frames whose PNGs and `depth` line were written, and the PNGs' size. + std::atomic _depthWritten; + std::atomic _depthBytes; // --- Raw IMU tap (independent of ARKit; CMMotionManager's raw, un-fused // accelerometer/gyro APIs, not the fused CMDeviceMotion path) --- diff --git a/ios/ViroKit/VROARSessionRecorderIOS.mm b/ios/ViroKit/VROARSessionRecorderIOS.mm index ce9044996..ce3000f51 100644 --- a/ios/ViroKit/VROARSessionRecorderIOS.mm +++ b/ios/ViroKit/VROARSessionRecorderIOS.mm @@ -71,6 +71,7 @@ static bool writeGrayscalePNG(NSString *path, const void *pixels, size_t width, VROARSessionRecorderIOS::VROARSessionRecorderIOS() : _status(VROARRecordingStatus::None), + _running(false), _startTimestamp(0), _videoWriter(nil), _videoWriterInput(nil), @@ -80,11 +81,16 @@ static bool writeGrayscalePNG(NSString *path, const void *pixels, size_t width, _videoWidth(0), _videoHeight(0), _lastFrameTimestamp(0), + _poseCount(0), + _gpsFixCount(0), + _lastPoseTimestamp(0), _depthRequested(false), _lastDepthTimestamp(0), _depthCount(0), _depthQueue(nil), _depthPending(0), + _depthWritten(0), + _depthBytes(0), _motionManager(nil), _imuQueue(nil), _wroteHeader(false) { @@ -121,9 +127,14 @@ static bool writeGrayscalePNG(NSString *path, const void *pixels, size_t width, _videoWidth = 0; _videoHeight = 0; _lastFrameTimestamp = 0; + _lastPoseTimestamp = 0; + _poseCount = 0; + _gpsFixCount = 0; _lastDepthTimestamp = 0; _depthCount = 0; _depthPending = 0; + _depthWritten = 0; + _depthBytes = 0; _depthRequested = config.depth; if (_depthRequested) { @@ -207,13 +218,16 @@ static bool writeGrayscalePNG(NSString *path, const void *pixels, size_t width, } _status = VROARRecordingStatus::Recording; + _running = true; if (onSuccess) onSuccess(); return true; } -void VROARSessionRecorderIOS::stop() { +VROARRecordingResult VROARSessionRecorderIOS::stop() { + VROARRecordingResult result; AVAssetWriter *writerToFinish = nil; AVAssetWriterInput *inputToFinish = nil; + bool failedWhileRecording = false; { // Flip _status away from Recording under the same lock recordFrame() // takes around its own status check + appendPixelBuffer call — see @@ -223,9 +237,12 @@ static bool writeGrayscalePNG(NSString *path, const void *pixels, size_t width, // returns early, so markAsFinished below can never race a concurrent // append. std::lock_guard lock(_stateMutex); - if (_status != VROARRecordingStatus::Recording && _status != VROARRecordingStatus::IOError) { - return; + if (!_running) { + result.error = "Not recording"; + return result; } + _running = false; + failedWhileRecording = _status == VROARRecordingStatus::IOError; _status = VROARRecordingStatus::None; writerToFinish = _videoWriter; inputToFinish = _videoWriterInput; @@ -248,6 +265,7 @@ static bool writeGrayscalePNG(NSString *path, const void *pixels, size_t width, } _imuQueue = nil; + bool finishFailed = false; if (writerToFinish && writerToFinish.status == AVAssetWriterStatusWriting) { [inputToFinish markAsFinished]; dispatch_semaphore_t sema = dispatch_semaphore_create(0); @@ -270,10 +288,41 @@ static bool writeGrayscalePNG(NSString *path, const void *pixels, size_t width, writerToFinish.error ? [writerToFinish.error.localizedDescription UTF8String] : "none"); std::lock_guard lock(_stateMutex); _status = VROARRecordingStatus::IOError; + finishFailed = true; } } closeSidecar(); + + { + std::lock_guard lock(_sidecarMutex); + result.frames = _poseCount; + result.gpsFixes = _gpsFixCount; + result.durationSec = _poseCount > 0 ? _lastPoseTimestamp - _startTimestamp : 0; + } + result.depthFrames = _depthWritten.load(); + + NSFileManager *fileManager = [NSFileManager defaultManager]; + NSString *dir = [NSString stringWithUTF8String:_outputDir.c_str()]; + int64_t bytes = _depthBytes.load(); + for (NSString *name in @[ @"video.mp4", @"session.jsonl" ]) { + NSDictionary *attributes = [fileManager attributesOfItemAtPath:[dir stringByAppendingPathComponent:name] + error:nil]; + bytes += (int64_t)[attributes fileSize]; + } + result.bytes = bytes; + + if (finishFailed) { + result.error = "video.mp4 could not be finalized"; + } else if (failedWhileRecording) { + result.error = "Recording stopped after a write error"; + } else if (_videoDisabled || _loggedPixelFormatMismatch) { + result.error = "Video could not be written, video.mp4 is incomplete"; + } else if (result.frames == 0) { + result.error = "No frames were recorded"; + } + result.success = result.error.empty(); + return result; } VROARRecordingStatus VROARSessionRecorderIOS::getStatus() const { @@ -374,6 +423,7 @@ static bool writeGrayscalePNG(NSString *path, const void *pixels, size_t width, // A position without a valid heading is still written, with the heading // fields null: the reader judges position and heading separately. + bool hasGps = false; if (geo.hasGps && std::isfinite(geo.latitude) && std::isfinite(geo.longitude) && std::isfinite(geo.altitude)) { char gpsBuf[256]; @@ -403,6 +453,7 @@ static bool writeGrayscalePNG(NSString *path, const void *pixels, size_t width, geo.latitude, geo.longitude, geo.altitude, hAccField, headingField, headingAccField); line.insert(line.size() - 1, gpsBuf); // before the closing '}' + hasGps = true; } // Buffered, not written immediately — see the _bufferedLines comment in @@ -410,6 +461,9 @@ static bool writeGrayscalePNG(NSString *path, const void *pixels, size_t width, std::lock_guard lock(_sidecarMutex); if (_sidecar.is_open()) { _bufferedLines.push_back({tNs, line}); + _poseCount++; + if (hasGps) _gpsFixCount++; + _lastPoseTimestamp = frame.timestamp; } } @@ -511,9 +565,14 @@ static bool writeGrayscalePNG(NSString *path, const void *pixels, size_t width, "\"fx\":%.4f,\"fy\":%.4f,\"cx\":%.4f,\"cy\":%.4f,\"source\":\"lidar\"}", (long long)tNs, [depthRel UTF8String], [confidenceRel UTF8String], (int)width, (int)height, fx, fy, cx, cy); + NSFileManager *fileManager = [NSFileManager defaultManager]; + int64_t pngBytes = (int64_t)[[fileManager attributesOfItemAtPath:depthPath error:nil] fileSize] + + (int64_t)[[fileManager attributesOfItemAtPath:confidencePath error:nil] fileSize]; std::lock_guard lock(_sidecarMutex); if (_sidecar.is_open()) { _bufferedLines.push_back({tNs, std::string(buf)}); + _depthWritten++; + _depthBytes += pngBytes; } } else { perr("VROARSessionRecorderIOS: could not write depth frame %s, skipping it", [name UTF8String]); diff --git a/ios/ViroKit/VROARSessioniOS.cpp b/ios/ViroKit/VROARSessioniOS.cpp index b09544617..5b6a550ef 100644 --- a/ios/ViroKit/VROARSessioniOS.cpp +++ b/ios/ViroKit/VROARSessioniOS.cpp @@ -3348,6 +3348,13 @@ bool VROARSessioniOS::isRecordingSupported() const { return true; } +bool VROARSessioniOS::isRecordingDepthSupported() const { + if (@available(iOS 14.0, *)) { + return [ARWorldTrackingConfiguration supportsFrameSemantics:ARFrameSemanticSceneDepth]; + } + return false; +} + void VROARSessioniOS::startRecording(const VROARRecordingConfig &config, std::function onSuccess, std::function onFailure) { @@ -3374,9 +3381,15 @@ void VROARSessioniOS::startRecording(const VROARRecordingConfig &config, _recorder->start(recordingConfig, onSuccess, onFailure); } -void VROARSessioniOS::stopRecording() { +void VROARSessioniOS::stopRecording(std::function onStopped) { + VROARRecordingResult result; if (_recorder) { - _recorder->stop(); + result = _recorder->stop(); + } else { + result.error = "Not recording"; + } + if (onStopped) { + onStopped(result); } } diff --git a/ios/ViroKit/VROARSessioniOS.h b/ios/ViroKit/VROARSessioniOS.h index ce3494238..783dc6c4e 100644 --- a/ios/ViroKit/VROARSessioniOS.h +++ b/ios/ViroKit/VROARSessioniOS.h @@ -261,10 +261,12 @@ class API_AVAILABLE(ios(12.0)) VROARSessioniOS : public VROARSession, public std // AR Session Recording API (see VROARSessionRecorderIOS) bool isRecordingSupported() const override; + bool isRecordingDepthSupported() const override; void startRecording(const VROARRecordingConfig &config, std::function onSuccess, std::function onFailure) override; - void stopRecording() override; + using VROARSession::stopRecording; + void stopRecording(std::function onStopped) override; VROARRecordingStatus getRecordingStatus() const override; /* From ddc299a1b609d8df0d6a6ac45f4dc089b606263d Mon Sep 17 00:00:00 2001 From: Ernesto Mendoza Date: Tue, 3 Nov 2026 20:54:12 -0600 Subject: [PATCH 3/3] Recording: no start while a stop is saving (iOS), one stop per recording (Android) --- .../src/main/java/com/viro/core/ARScene.java | 47 +++++++++++++------ ios/ViroKit/VROARSessionRecorderIOS.h | 3 ++ ios/ViroKit/VROARSessionRecorderIOS.mm | 7 +++ 3 files changed, 42 insertions(+), 15 deletions(-) diff --git a/android/sharedCode/src/main/java/com/viro/core/ARScene.java b/android/sharedCode/src/main/java/com/viro/core/ARScene.java index 617ffcd2a..2b5003d55 100644 --- a/android/sharedCode/src/main/java/com/viro/core/ARScene.java +++ b/android/sharedCode/src/main/java/com/viro/core/ARScene.java @@ -533,6 +533,9 @@ public int getId() { // this class only bridges native's per-frame calls to it. // stopRecording() may run off the UI thread. private volatile com.viro.core.internal.ARSessionRecorder mSessionRecorder; + // Start and stop run on their own threads; taking or setting mSessionRecorder under this lock makes + // each stop own one recorder, so a second stop answers "Not recording" as on iOS. + private final Object mRecorderLock = new Object(); private RecordingStartListener mRecordingStartListener; // Geospatial API state @@ -1064,22 +1067,29 @@ public void startRecording(String outputDir, Context context, RecordingStartList * @param depth Also record the device's depth. */ public void startRecording(String outputDir, Context context, boolean depth, RecordingStartListener listener) { - if (mSessionRecorder != null) { - if (listener != null) { - listener.onFailure("Already recording"); - } - return; - } - mSessionRecorder = new com.viro.core.internal.ARSessionRecorder(context, new com.viro.core.internal.ARSessionRecorder.ErrorListener() { + com.viro.core.internal.ARSessionRecorder recorder = new com.viro.core.internal.ARSessionRecorder(context, new com.viro.core.internal.ARSessionRecorder.ErrorListener() { @Override public void onRecordingError(String message) { nativeReportRecordingError(mNativeRef, message); } }); + synchronized (mRecorderLock) { + if (mSessionRecorder != null) { + if (listener != null) { + listener.onFailure("Already recording"); + } + return; + } + mSessionRecorder = recorder; + } try { - mSessionRecorder.start(outputDir); + recorder.start(outputDir); } catch (Exception e) { - mSessionRecorder = null; + synchronized (mRecorderLock) { + if (mSessionRecorder == recorder) { + mSessionRecorder = null; + } + } if (listener != null) { listener.onFailure(e.getMessage()); } @@ -1098,12 +1108,15 @@ public void onRecordingError(String message) { */ public RecordingResult stopRecording() { nativeStopRecording(mNativeRef); - if (mSessionRecorder == null) { + com.viro.core.internal.ARSessionRecorder recorder; + synchronized (mRecorderLock) { + recorder = mSessionRecorder; + mSessionRecorder = null; + } + if (recorder == null) { return new RecordingResult(false, "Not recording", 0, 0, 0, 0, -1); } - RecordingResult result = mSessionRecorder.stop(); - mSessionRecorder = null; - return result; + return recorder.stop(); } /** @@ -1144,10 +1157,14 @@ void onRecordingStartSuccess(boolean depthActive) { // in that path native never got as far as calling setRecordingJavaCallback; // the Java-side encoder/sensors we already started need to be stopped too. void onRecordingStartFailure(String error) { - if (mSessionRecorder != null) { - mSessionRecorder.stop(); + com.viro.core.internal.ARSessionRecorder recorder; + synchronized (mRecorderLock) { + recorder = mSessionRecorder; mSessionRecorder = null; } + if (recorder != null) { + recorder.stop(); + } if (mRecordingStartListener != null) { mRecordingStartListener.onFailure(error); } diff --git a/ios/ViroKit/VROARSessionRecorderIOS.h b/ios/ViroKit/VROARSessionRecorderIOS.h index a94c183b3..5f9078c9c 100644 --- a/ios/ViroKit/VROARSessionRecorderIOS.h +++ b/ios/ViroKit/VROARSessionRecorderIOS.h @@ -118,6 +118,9 @@ class API_AVAILABLE(ios(12.0)) VROARSessionRecorderIOS { // True from a successful start() until stop(); _status alone cannot tell, since a // failed finish leaves it at IOError after the recording has ended. bool _running; + // True while stop() finishes the video and closes the files (up to 20 s). start() refuses + // meanwhile: a new recording would share the folder, counters and sidecar the stop is closing. + std::atomic _stopping; std::string _outputDir; double _startTimestamp; // ARFrame.timestamp of the first recorded frame, for relative t diff --git a/ios/ViroKit/VROARSessionRecorderIOS.mm b/ios/ViroKit/VROARSessionRecorderIOS.mm index ce3000f51..395e424f4 100644 --- a/ios/ViroKit/VROARSessionRecorderIOS.mm +++ b/ios/ViroKit/VROARSessionRecorderIOS.mm @@ -72,6 +72,7 @@ static bool writeGrayscalePNG(NSString *path, const void *pixels, size_t width, VROARSessionRecorderIOS::VROARSessionRecorderIOS() : _status(VROARRecordingStatus::None), _running(false), + _stopping(false), _startTimestamp(0), _videoWriter(nil), _videoWriterInput(nil), @@ -107,6 +108,10 @@ static bool writeGrayscalePNG(NSString *path, const void *pixels, size_t width, if (onFailure) onFailure("Already recording"); return false; } + if (_stopping) { + if (onFailure) onFailure("The previous recording is still being saved"); + return false; + } NSString *dir = [NSString stringWithUTF8String:config.outputDir.c_str()]; NSError *dirError = nil; @@ -242,6 +247,7 @@ static bool writeGrayscalePNG(NSString *path, const void *pixels, size_t width, return result; } _running = false; + _stopping = true; failedWhileRecording = _status == VROARRecordingStatus::IOError; _status = VROARRecordingStatus::None; writerToFinish = _videoWriter; @@ -322,6 +328,7 @@ static bool writeGrayscalePNG(NSString *path, const void *pixels, size_t width, result.error = "No frames were recorded"; } result.success = result.error.empty(); + _stopping = false; return result; }