Skip to content

Commit 0ed525c

Browse files
committed
record/avfoundation: use cocoa_defines.h macros instead of rolling our own
libretro-common/include/defines/cocoa_defines.h already provides RARCH_RELEASE / RARCH_RETAIN / RARCH_AUTORELEASE / RARCH_SUPER_DEALLOC for ARC/MRR-neutral Objective-C memory management, and is the established convention across ui_cocoa.m / cocoa_common.m / ui_cocoatouch.m / dispserv_apple.m / coreaudio.c. The local RA_RELEASE_OBJ / RA_RELEASE_DQ macros added in b327848 solved the same problem with ad-hoc spelling. Adopt the project convention. Changes: * libretro-common/include/defines/cocoa_defines.h: add RARCH_DISPATCH_RELEASE(x) alongside the existing four macros. The GCD variant needs its own spelling because [x release] is a compile error on pre-10.8 SDKs where OS_OBJECT_USE_OBJC is 0 and dispatch_queue_t is a plain C handle rather than an ObjC object; dispatch_release() works uniformly on all MRR-capable SDKs. NULL-guarded because dispatch_release(NULL) is explicitly undefined (unlike [nil release] which is a defined no-op). * record/drivers/record_avfoundation.m: drop the local ~30-line macro block, #include <defines/cocoa_defines.h>, and replace the five call sites. The project convention splits the release and the nil-assignment into two lines rather than bundling them into a compound macro; follow suit. Semantics are identical: MRC: [field release]; field = nil; ARC: ((void)0); field = nil; (release happens via the __strong auto-qualifier on the struct member triggered by the assignment) No behavior change on any build path. The existing tests for the MRC leak fix from b327848 continue to pass (the ivars end up nil- valued and the objects released exactly once in both modes). Camera-side counterpart (camera/drivers/avfoundation.m) does not need the same treatment: it uses @Property (strong, nonatomic) on an @interface (clang auto-synthesizes correct MRC retain semantics from the strong attribute), its private struct ivars are plain C pointers that ObjC memory management does not apply to, and there is no -dealloc override.
1 parent 3171f79 commit 0ed525c

2 files changed

Lines changed: 31 additions & 36 deletions

File tree

libretro-common/include/defines/cocoa_defines.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,11 +97,20 @@
9797
#define RARCH_RELEASE(x) ((void)0)
9898
#define RARCH_AUTORELEASE(x) ((void)0)
9999
#define RARCH_SUPER_DEALLOC() ((void)0)
100+
#define RARCH_DISPATCH_RELEASE(x) ((void)0)
100101
#else
101102
#define RARCH_RETAIN(x) [(x) retain]
102103
#define RARCH_RELEASE(x) [(x) release]
103104
#define RARCH_AUTORELEASE(x) [(x) autorelease]
104105
#define RARCH_SUPER_DEALLOC() [super dealloc]
106+
/* GCD object release for MRR. Use dispatch_release() rather than
107+
* [x release] because the latter is a compile error on pre-10.8 SDKs
108+
* where OS_OBJECT_USE_OBJC is 0 and dispatch_queue_t is a plain C
109+
* handle rather than an Objective-C object. dispatch_release works
110+
* uniformly across all MRR-capable SDKs. NULL-guarded because
111+
* dispatch_release(NULL) is explicitly undefined, unlike
112+
* [nil release] which is a defined no-op. */
113+
#define RARCH_DISPATCH_RELEASE(x) do { if (x) dispatch_release(x); } while (0)
105114
#endif
106115

107116
#endif

record/drivers/record_avfoundation.m

Lines changed: 22 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
#include <string.h>
2626

2727
#include <boolean.h>
28+
#include <defines/cocoa_defines.h>
2829
#include <gfx/scaler/scaler.h>
2930
#include <gfx/video_frame.h>
3031
#include <string/stdstring.h>
@@ -33,36 +34,6 @@
3334
#include "../../retroarch.h"
3435
#include "../../verbosity.h"
3536

36-
/* Release helpers that work identically under MRC and ARC.
37-
*
38-
* The ivars on record_avfoundation_t are Objective-C pointers
39-
* (AVAssetWriter*, AVAssetWriterInput*, etc. and dispatch_queue_t)
40-
* held in a plain C struct. Modern clang auto-qualifies ObjC struct
41-
* members as __strong under ARC, so assignment through them releases
42-
* the prior value automatically - plain `field = nil` is correct.
43-
*
44-
* Under MRC (RetroArch's top-level qb Makefile builds .m files without
45-
* -fobjc-arc, and so do the RetroArch/_Metal/_OSX107/_iOS13 Xcode
46-
* projects) no such auto-release happens, so `field = nil` would
47-
* overwrite a +1-retained pointer and leak the object. That is the
48-
* historical behaviour: every record_stop leaked four AVAssetWriter
49-
* objects and one GCD queue.
50-
*
51-
* Fix with the macros below: explicit release on the MRC path,
52-
* assignment-only on the ARC path. `[nil release]` is a safe no-op,
53-
* but `dispatch_release(NULL)` is undefined, so the DQ macro guards.
54-
*
55-
* The field expression is evaluated twice by the MRC expansion; all
56-
* five call sites use a direct struct-member lvalue with no aliasing
57-
* so this is safe. */
58-
#if __has_feature(objc_arc)
59-
# define RA_RELEASE_OBJ(_x) do { (_x) = nil; } while (0)
60-
# define RA_RELEASE_DQ(_x) do { (_x) = nil; } while (0)
61-
#else
62-
# define RA_RELEASE_OBJ(_x) do { [(_x) release]; (_x) = nil; } while (0)
63-
# define RA_RELEASE_DQ(_x) do { if (_x) { dispatch_release(_x); (_x) = nil; } } while (0)
64-
#endif
65-
6637
typedef struct record_avfoundation
6738
{
6839
AVAssetWriter *assetWriter;
@@ -170,11 +141,25 @@ static void avfoundation_release_handle(record_avfoundation_t *handle)
170141
if (!handle)
171142
return;
172143
scaler_ctx_gen_reset(&handle->scaler);
173-
RA_RELEASE_OBJ(handle->pixelBufferAdaptor);
174-
RA_RELEASE_OBJ(handle->videoInput);
175-
RA_RELEASE_OBJ(handle->audioInput);
176-
RA_RELEASE_OBJ(handle->assetWriter);
177-
RA_RELEASE_DQ (handle->encodingQueue);
144+
145+
/* Release (MRC) / auto-release (ARC) the owned Objective-C objects,
146+
* then clear the field so any subsequent access sees nil rather
147+
* than a dangling pointer. Under ARC the RARCH_RELEASE is a no-op
148+
* and the field-assignment-to-nil releases via __strong auto-
149+
* qualification; under MRC RARCH_RELEASE sends -release and the
150+
* field-assignment-to-nil just overwrites the now-stale pointer.
151+
* See libretro-common/include/defines/cocoa_defines.h. */
152+
RARCH_RELEASE(handle->pixelBufferAdaptor);
153+
handle->pixelBufferAdaptor = nil;
154+
RARCH_RELEASE(handle->videoInput);
155+
handle->videoInput = nil;
156+
RARCH_RELEASE(handle->audioInput);
157+
handle->audioInput = nil;
158+
RARCH_RELEASE(handle->assetWriter);
159+
handle->assetWriter = nil;
160+
RARCH_DISPATCH_RELEASE(handle->encodingQueue);
161+
handle->encodingQueue = nil;
162+
178163
free(handle);
179164
}
180165

@@ -373,7 +358,8 @@ static void avfoundation_release_handle(record_avfoundation_t *handle)
373358
else
374359
{
375360
RARCH_WARN("[AVFoundation] Cannot add audio input, continuing without audio\n");
376-
RA_RELEASE_OBJ(handle->audioInput);
361+
RARCH_RELEASE(handle->audioInput);
362+
handle->audioInput = nil;
377363
}
378364
}
379365

0 commit comments

Comments
 (0)