feat: Throw an Error if Frame is already destroyed (#3099)

mrousavy · web-flow · commit b3f5ab651202 · 2024-07-24T14:42:53.000+02:00
* feat: Throw an error if ObjC `Frame` is invalid

* fix: Remove duplicate Frame closed checks
diff --git a/package/android/src/main/cpp/frameprocessors/FrameHostObject.cpp b/package/android/src/main/cpp/frameprocessors/FrameHostObject.cpp
@@ -55,17 +55,6 @@ std::vector<jsi::PropNameID> FrameHostObject::getPropertyNames(jsi::Runtime& rt)
   return result;
 }
 
-jni::global_ref<JFrame> FrameHostObject::getFrame() {
-  if (!_frame->getIsValid()) [[unlikely]] {
-    throw std::runtime_error("Frame is already closed! "
-                             "Are you trying to access the Image data outside of a Frame Processor's lifetime?\n"
-                             "- If you want to use `console.log(frame)`, use `console.log(frame.toString())` instead.\n"
-                             "- If you want to do async processing, use `runAsync(...)` instead.\n"
-                             "- If you want to use runOnJS, increment it's ref-count: `frame.incrementRefCount()`");
-  }
-  return _frame;
-}
-
 #define JSI_FUNC [=](jsi::Runtime & runtime, const jsi::Value& thisValue, const jsi::Value* arguments, size_t count) -> jsi::Value
 
 jsi::Value FrameHostObject::get(jsi::Runtime& runtime, const jsi::PropNameID& propName) {
@@ -76,40 +65,32 @@ jsi::Value FrameHostObject::get(jsi::Runtime& runtime, const jsi::PropNameID& pr
     return jsi::Value(_frame->getIsValid());
   }
   if (name == "width") {
-    const auto& frame = getFrame();
-    return jsi::Value(frame->getWidth());
+    return jsi::Value(_frame->getWidth());
   }
   if (name == "height") {
-    const auto& frame = getFrame();
-    return jsi::Value(frame->getHeight());
+    return jsi::Value(_frame->getHeight());
   }
   if (name == "isMirrored") {
-    const auto& frame = getFrame();
-    return jsi::Value(frame->getIsMirrored());
+    return jsi::Value(_frame->getIsMirrored());
   }
   if (name == "orientation") {
-    const auto& frame = getFrame();
-    auto orientation = frame->getOrientation();
+    auto orientation = _frame->getOrientation();
     auto string = orientation->getUnionValue();
     return jsi::String::createFromUtf8(runtime, string->toStdString());
   }
   if (name == "pixelFormat") {
-    const auto& frame = getFrame();
-    auto pixelFormat = frame->getPixelFormat();
+    auto pixelFormat = _frame->getPixelFormat();
     auto string = pixelFormat->getUnionValue();
     return jsi::String::createFromUtf8(runtime, string->toStdString());
   }
   if (name == "timestamp") {
-    const auto& frame = getFrame();
-    return jsi::Value(static_cast<double>(frame->getTimestamp()));
+    return jsi::Value(static_cast<double>(_frame->getTimestamp()));
   }
   if (name == "bytesPerRow") {
-    const auto& frame = getFrame();
-    return jsi::Value(frame->getBytesPerRow());
+    return jsi::Value(_frame->getBytesPerRow());
   }
   if (name == "planesCount") {
-    const auto& frame = getFrame();
-    return jsi::Value(frame->getPlanesCount());
+    return jsi::Value(_frame->getPlanesCount());
   }
 
   // Internal Methods
@@ -134,8 +115,7 @@ jsi::Value FrameHostObject::get(jsi::Runtime& runtime, const jsi::PropNameID& pr
   if (name == "getNativeBuffer") {
     jsi::HostFunctionType getNativeBuffer = JSI_FUNC {
 #if __ANDROID_API__ >= 26
-      const auto& frame = getFrame();
-      AHardwareBuffer* hardwareBuffer = frame->getHardwareBuffer();
+      AHardwareBuffer* hardwareBuffer = _frame->getHardwareBuffer();
       AHardwareBuffer_acquire(hardwareBuffer);
       uintptr_t pointer = reinterpret_cast<uintptr_t>(hardwareBuffer);
       jsi::HostFunctionType deleteFunc = [=](jsi::Runtime& runtime, const jsi::Value& thisArg, const jsi::Value* args,
@@ -159,8 +139,7 @@ jsi::Value FrameHostObject::get(jsi::Runtime& runtime, const jsi::PropNameID& pr
   if (name == "toArrayBuffer") {
     jsi::HostFunctionType toArrayBuffer = JSI_FUNC {
 #if __ANDROID_API__ >= 26
-      const auto& frame = getFrame();
-      AHardwareBuffer* hardwareBuffer = frame->getHardwareBuffer();
+      AHardwareBuffer* hardwareBuffer = _frame->getHardwareBuffer();
       AHardwareBuffer_acquire(hardwareBuffer);
 
       AHardwareBuffer_Desc bufferDescription;
diff --git a/package/android/src/main/cpp/frameprocessors/FrameHostObject.h b/package/android/src/main/cpp/frameprocessors/FrameHostObject.h
@@ -27,7 +27,9 @@ class JSI_EXPORT FrameHostObject : public jsi::HostObject, public std::enable_sh
   std::vector<jsi::PropNameID> getPropertyNames(jsi::Runtime& rt) override;
 
 public:
-  jni::global_ref<JFrame> getFrame();
+  inline jni::global_ref<JFrame> getFrame() const noexcept {
+    return _frame;
+  }
 
 private:
   jni::global_ref<JFrame> _frame;
diff --git a/package/ios/FrameProcessors/Frame.m b/package/ios/FrameProcessors/Frame.m
@@ -35,15 +35,28 @@ - (void)decrementRefCount {
 }
 
 - (CMSampleBufferRef)buffer {
+  if (!self.isValid) {
+    @throw [[NSException alloc] initWithName:@"capture/frame-invalid"
+                                      reason:@"Trying to access an already closed Frame! "
+                                              "Are you trying to access the Image data outside of a Frame Processor's lifetime?\n"
+                                              "- If you want to use `console.log(frame)`, use `console.log(frame.toString())` instead.\n"
+                                              "- If you want to do async processing, use `runAsync(...)` instead.\n"
+                                              "- If you want to use runOnJS, increment it's ref-count: `frame.incrementRefCount()`"
+                                    userInfo:nil];
+  }
   return _buffer;
 }
 
+- (BOOL)isValid {
+  return _buffer != nil && CFGetRetainCount(_buffer) > 0 && CMSampleBufferIsValid(_buffer);
+}
+
 - (UIImageOrientation)orientation {
   return _orientation;
 }
 
 - (NSString*)pixelFormat {
-  CMFormatDescriptionRef format = CMSampleBufferGetFormatDescription(_buffer);
+  CMFormatDescriptionRef format = CMSampleBufferGetFormatDescription(self.buffer);
   FourCharCode mediaType = CMFormatDescriptionGetMediaSubType(format);
   switch (mediaType) {
     case kCVPixelFormatType_32BGRA:
@@ -66,32 +79,28 @@ - (BOOL)isMirrored {
   return _isMirrored;
 }
 
-- (BOOL)isValid {
-  return _buffer != nil && CMSampleBufferIsValid(_buffer);
-}
-
 - (size_t)width {
-  CVPixelBufferRef imageBuffer = CMSampleBufferGetImageBuffer(_buffer);
+  CVPixelBufferRef imageBuffer = CMSampleBufferGetImageBuffer(self.buffer);
   return CVPixelBufferGetWidth(imageBuffer);
 }
 
 - (size_t)height {
-  CVPixelBufferRef imageBuffer = CMSampleBufferGetImageBuffer(_buffer);
+  CVPixelBufferRef imageBuffer = CMSampleBufferGetImageBuffer(self.buffer);
   return CVPixelBufferGetHeight(imageBuffer);
 }
 
 - (double)timestamp {
-  CMTime timestamp = CMSampleBufferGetPresentationTimeStamp(_buffer);
+  CMTime timestamp = CMSampleBufferGetPresentationTimeStamp(self.buffer);
   return CMTimeGetSeconds(timestamp) * 1000.0;
 }
 
 - (size_t)bytesPerRow {
-  CVPixelBufferRef imageBuffer = CMSampleBufferGetImageBuffer(_buffer);
+  CVPixelBufferRef imageBuffer = CMSampleBufferGetImageBuffer(self.buffer);
   return CVPixelBufferGetBytesPerRow(imageBuffer);
 }
 
 - (size_t)planesCount {
-  CVPixelBufferRef imageBuffer = CMSampleBufferGetImageBuffer(_buffer);
+  CVPixelBufferRef imageBuffer = CMSampleBufferGetImageBuffer(self.buffer);
   return CVPixelBufferGetPlaneCount(imageBuffer);
 }
 
diff --git a/package/ios/FrameProcessors/FrameHostObject.h b/package/ios/FrameProcessors/FrameHostObject.h
@@ -25,7 +25,9 @@ class JSI_EXPORT FrameHostObject : public jsi::HostObject, public std::enable_sh
   std::vector<jsi::PropNameID> getPropertyNames(jsi::Runtime& rt) override;
 
 public:
-  Frame* getFrame();
+  inline Frame* getFrame() const noexcept {
+    return _frame;
+  }
 
 private:
   Frame* _frame;
diff --git a/package/ios/FrameProcessors/FrameHostObject.mm b/package/ios/FrameProcessors/FrameHostObject.mm
@@ -40,60 +40,39 @@
   return result;
 }
 
-Frame* FrameHostObject::getFrame() {
-  if (!_frame.isValid) [[unlikely]] {
-    throw std::runtime_error("Frame is already closed! "
-                             "Are you trying to access the Image data outside of a Frame Processor's lifetime?\n"
-                             "- If you want to use `console.log(frame)`, use `console.log(frame.toString())` instead.\n"
-                             "- If you want to do async processing, use `runAsync(...)` instead.\n"
-                             "- If you want to use runOnJS, increment it's ref-count: `frame.incrementRefCount()`");
-  }
-  return _frame;
-}
-
 #define JSI_FUNC [=](jsi::Runtime & runtime, const jsi::Value& thisValue, const jsi::Value* arguments, size_t count) -> jsi::Value
 
 jsi::Value FrameHostObject::get(jsi::Runtime& runtime, const jsi::PropNameID& propName) {
   auto name = propName.utf8(runtime);
 
   // Properties
   if (name == "width") {
-    Frame* frame = getFrame();
-    return jsi::Value((double)frame.width);
+    return jsi::Value((double)_frame.width);
   }
   if (name == "height") {
-    Frame* frame = getFrame();
-    return jsi::Value((double)frame.height);
+    return jsi::Value((double)_frame.height);
   }
   if (name == "orientation") {
-    Frame* frame = getFrame();
-    NSString* orientation = [NSString stringWithParsed:frame.orientation];
+    NSString* orientation = [NSString stringWithParsed:_frame.orientation];
     return jsi::String::createFromUtf8(runtime, orientation.UTF8String);
   }
   if (name == "isMirrored") {
-    Frame* frame = getFrame();
-    return jsi::Value(frame.isMirrored);
+    return jsi::Value(_frame.isMirrored);
   }
   if (name == "timestamp") {
-    Frame* frame = getFrame();
-    return jsi::Value(frame.timestamp);
+    return jsi::Value(_frame.timestamp);
   }
   if (name == "pixelFormat") {
-    Frame* frame = getFrame();
-    return jsi::String::createFromUtf8(runtime, frame.pixelFormat.UTF8String);
+    return jsi::String::createFromUtf8(runtime, _frame.pixelFormat.UTF8String);
   }
   if (name == "isValid") {
-    // unsafely access the Frame and try to see if it's valid
-    Frame* frame = _frame;
-    return jsi::Value(frame != nil && frame.isValid);
+    return jsi::Value(_frame != nil && _frame.isValid);
   }
   if (name == "bytesPerRow") {
-    Frame* frame = getFrame();
-    return jsi::Value((double)frame.bytesPerRow);
+    return jsi::Value((double)_frame.bytesPerRow);
   }
   if (name == "planesCount") {
-    Frame* frame = getFrame();
-    return jsi::Value((double)frame.planesCount);
+    return jsi::Value((double)_frame.planesCount);
   }
 
   // Internal methods
@@ -116,8 +95,7 @@
   if (name == "getNativeBuffer") {
     auto getNativeBuffer = JSI_FUNC {
       // Box-cast to uintptr (just 64-bit address)
-      Frame* frame = getFrame();
-      CMSampleBufferRef sampleBuffer = frame.buffer;
+      CMSampleBufferRef sampleBuffer = _frame.buffer;
       CVPixelBufferRef pixelBuffer = CMSampleBufferGetImageBuffer(sampleBuffer);
       uintptr_t pointer = reinterpret_cast<uintptr_t>(pixelBuffer);
       jsi::HostFunctionType deleteFunc = [=](jsi::Runtime& runtime, const jsi::Value& thisArg, const jsi::Value* args,
@@ -137,8 +115,7 @@
   if (name == "toArrayBuffer") {
     auto toArrayBuffer = JSI_FUNC {
       // Get CPU readable Pixel Buffer from Frame and write it to a jsi::ArrayBuffer
-      Frame* frame = getFrame();
-      auto pixelBuffer = CMSampleBufferGetImageBuffer(frame.buffer);
+      auto pixelBuffer = CMSampleBufferGetImageBuffer(_frame.buffer);
       auto bytesPerRow = CVPixelBufferGetBytesPerRow(pixelBuffer);
       auto height = CVPixelBufferGetHeight(pixelBuffer);
 
@@ -172,8 +149,10 @@
   if (name == "toString") {
     auto toString = JSI_FUNC {
       // Print debug description (width, height)
-      Frame* frame = getFrame();
-      NSMutableString* string = [NSMutableString stringWithFormat:@"%lu x %lu %@ Frame", frame.width, frame.height, frame.pixelFormat];
+      if (!_frame.isValid) {
+        return jsi::String::createFromUtf8(runtime, "[closed frame]");
+      }
+      NSMutableString* string = [NSMutableString stringWithFormat:@"%lu x %lu %@ Frame", _frame.width, _frame.height, _frame.pixelFormat];
       return jsi::String::createFromUtf8(runtime, string.UTF8String);
     };
     return jsi::Function::createFromHostFunction(runtime, jsi::PropNameID::forUtf8(runtime, "toString"), 0, toString);
