Skip to content

Commit f3b3d69

Browse files
committed
camera/avfoundation: reuse frameBuffer for color-bars fallback, null after free
Audit of the frameBuffer lifecycle turned up three small issues in camera/drivers/avfoundation.m that all cluster around the same ivar: 1. avfoundation_init's setupCameraSession-failed error path does free(avf->manager.frameBuffer) but does not null the field. The manager is a singleton, so the next avfoundation_init inherits a dangling pointer until its own calloc overwrites the slot. Everything in this driver runs on the main thread (capture delegate queue is dispatch_get_main_queue(), poll runs from rarch_draw_observer on the main run loop, init/free are on the main thread) so no concurrent access can observe the dangling value - but it is still defensive-programming- wrong to leave a singleton ivar pointing at freed memory. 2. avfoundation_poll's color-bars fallback calloc'd and freed a throwaway buffer on every poll while the camera session was warming up or stopped. For a typical 1280x720 camera that is ~3.5 MB of allocator churn per poll, fired at runloop rate until the session reports isRunning. Since we already have a cached per-driver frameBuffer on the manager (allocated in init, size matches avf->width x avf->height exactly), paint the color-bars pattern directly into it and hand that to frame_raw_cb. The captureOutput callback will overwrite the same buffer with real camera frames once the session starts. 3. The 'Camera not running, generating color bars...' log at the top of that fallback path fires every poll during the warmup window - the same per-frame log-spam pattern that commit 141d2a6 already fixed for two other logs in the file. Gate this one behind #ifdef DEBUG to match. Threading model (unchanged): the capture delegate is invoked on the main queue (set via setSampleBufferDelegate:queue: to dispatch_get_main_queue()) and the consumer of frameBuffer reads on the main thread as well (runloop_iterate runs from a CFRunLoopObserver attached to CFRunLoopGetMain() on macOS, from CADisplayLink on iOS/tvOS - both main-thread). Writer and reader are fully serialized by the main run loop, so reuse of frameBuffer across both paths needs no additional synchronization.
1 parent 0ed525c commit f3b3d69

1 file changed

Lines changed: 24 additions & 9 deletions

File tree

camera/drivers/avfoundation.m

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -656,7 +656,14 @@ static void generateColorBars(uint32_t *buffer, size_t width, size_t height) {
656656
if (!setupSuccess)
657657
{
658658
RARCH_ERR("[Camera] Failed to setup camera.\n");
659+
/* Null out frameBuffer on the singleton manager after freeing,
660+
* so a subsequent init doesn't observe a stale pointer. The
661+
* main-thread-only access pattern means nothing actually sees
662+
* the dangling value between these two statements and the next
663+
* init's calloc on the same field, but leaving a dangling
664+
* pointer in a singleton ivar is defensive-programming-wrong. */
659665
free(avf->manager.frameBuffer);
666+
avf->manager.frameBuffer = NULL;
660667
free(avf);
661668
return NULL;
662669
}
@@ -741,16 +748,24 @@ static bool avfoundation_poll(void *data,
741748

742749
if (!avf->manager.session.isRunning)
743750
{
751+
/* Session not running yet (or already stopped). Deliver a
752+
* color-bars test pattern so the core gets a well-formed
753+
* frame rather than nothing. Paint into the manager's own
754+
* frameBuffer rather than allocating a throwaway: the
755+
* generation cost is trivial (two nested loops of direct
756+
* assignments, no conversion) and it avoids a calloc+free
757+
* pair on every poll while the camera warms up. Once the
758+
* session starts and captureOutput:didOutputSampleBuffer:
759+
* begins overwriting frameBuffer with real frames, this
760+
* branch stops firing. */
761+
#ifdef DEBUG
744762
RARCH_LOG("[Camera] Camera not running, generating color bars...\n");
745-
uint32_t *tempBuffer = (uint32_t*)calloc(avf->width * avf->height, sizeof(uint32_t));
746-
if (tempBuffer)
747-
{
748-
generateColorBars(tempBuffer, avf->width, avf->height);
749-
frame_raw_cb(tempBuffer, avf->width, avf->height, avf->width * 4);
750-
free(tempBuffer);
751-
return true;
752-
}
753-
return false;
763+
#endif
764+
if (!avf->manager.frameBuffer)
765+
return false;
766+
generateColorBars(avf->manager.frameBuffer, avf->width, avf->height);
767+
frame_raw_cb(avf->manager.frameBuffer, avf->width, avf->height, avf->width * 4);
768+
return true;
754769
}
755770

756771
#ifdef DEBUG

0 commit comments

Comments
 (0)