ios/ff_movie_controller: fix potential crash for inject opaque

sjy-nankai · Oct 27, 2016 · 4787a6c · 4787a6c
1 parent 13f3bf3
commit 4787a6c
Show file tree

Hide file tree

Showing 5 changed files with 25 additions and 10 deletions.
diff --git a/ijkmedia/ijkplayer/ff_ffplay.c b/ijkmedia/ijkplayer/ff_ffplay.c
@@ -3337,18 +3337,19 @@ static int app_func_event(AVApplicationContext *h, int message ,void *data, size
     return inject_callback(ffp->inject_opaque, message , data, size);
 }
 
-void ffp_set_inject_opaque(FFPlayer *ffp, void *opaque)
+void *ffp_set_inject_opaque(FFPlayer *ffp, void *opaque)
 {
     if (!ffp)
-        return;
-
+        return NULL;
+    void *prev_weak_thiz = ffp->inject_opaque;
     ffp->inject_opaque = opaque;
 
     av_application_closep(&ffp->app_ctx);
     av_application_open(&ffp->app_ctx, ffp);
     ffp_set_option_int(ffp, FFP_OPT_CATEGORY_FORMAT, "ijkapplication", (int64_t)(intptr_t)ffp->app_ctx);
 
     ffp->app_ctx->func_on_app_event = app_func_event;
+    return prev_weak_thiz;
 }
 
 void ffp_set_option(FFPlayer *ffp, int opt_category, const char *name, const char *value)

diff --git a/ijkmedia/ijkplayer/ff_ffplay.h b/ijkmedia/ijkplayer/ff_ffplay.h
@@ -45,7 +45,7 @@ void      ffp_reset(FFPlayer *ffp);
 
 /* set options before ffp_prepare_async_l() */
 
-void      ffp_set_inject_opaque(FFPlayer *ffp, void *opaque);
+void     *ffp_set_inject_opaque(FFPlayer *ffp, void *opaque);
 void      ffp_set_option(FFPlayer *ffp, int opt_category, const char *name, const char *value);
 void      ffp_set_option_int(FFPlayer *ffp, int opt_category, const char *name, int64_t value);
 

diff --git a/ijkmedia/ijkplayer/ijkplayer.c b/ijkmedia/ijkplayer/ijkplayer.c
@@ -135,13 +135,14 @@ IjkMediaPlayer *ijkmp_create(int (*msg_loop)(void*))
     return NULL;
 }
 
-void ijkmp_set_inject_opaque(IjkMediaPlayer *mp, void *opaque)
+void *ijkmp_set_inject_opaque(IjkMediaPlayer *mp, void *opaque)
 {
     assert(mp);
 
     MPTRACE("%s(%p)\n", __func__, opaque);
-    ffp_set_inject_opaque(mp->ffplayer, opaque);
+    void *prev_weak_thiz = ffp_set_inject_opaque(mp->ffplayer, opaque);
     MPTRACE("%s()=void\n", __func__);
+    return prev_weak_thiz;
 }
 
 void ijkmp_set_option(IjkMediaPlayer *mp, int opt_category, const char *name, const char *value)

diff --git a/ijkmedia/ijkplayer/ijkplayer.h b/ijkmedia/ijkplayer/ijkplayer.h
@@ -163,7 +163,7 @@ void            ijkmp_io_stat_complete_register(void (*cb)(const char *url,
 
 // ref_count is 1 after open
 IjkMediaPlayer *ijkmp_create(int (*msg_loop)(void*));
-void            ijkmp_set_inject_opaque(IjkMediaPlayer *mp, void *opaque);
+void*            ijkmp_set_inject_opaque(IjkMediaPlayer *mp, void *opaque);
 
 void            ijkmp_set_option(IjkMediaPlayer *mp, int opt_category, const char *name, const char *value);
 void            ijkmp_set_option_int(IjkMediaPlayer *mp, int opt_category, const char *name, int64_t value);

diff --git a/ios/IJKMediaPlayer/IJKMediaPlayer/IJKFFMoviePlayerController.m b/ios/IJKMediaPlayer/IJKMediaPlayer/IJKFFMoviePlayerController.m
@@ -34,6 +34,14 @@
 
 static const char *kIJKFFRequiredFFmpegVersion = "ff3.1--ijk0.6.2--20160926--001";
 
+// It means you didn't call shutdown if you found this object leaked.
+@interface IJKWeakHolder : NSObject
+@property (nonatomic, weak) id object;
+@end
+
+@implementation IJKWeakHolder
+@end
+
 @interface IJKFFMoviePlayerController()
 
 @end
@@ -183,9 +191,11 @@ - (id)initWithContentURLString:(NSString *)aUrlString
         // init player
         _mediaPlayer = ijkmp_ios_create(media_player_msg_loop);
         _msgPool = [[IJKFFMoviePlayerMessagePool alloc] init];
+        IJKWeakHolder *weakHolder = [IJKWeakHolder new];
+        weakHolder.object = self;
 
         ijkmp_set_weak_thiz(_mediaPlayer, (__bridge_retained void *) self);
-        ijkmp_set_inject_opaque(_mediaPlayer, (__bridge void *) self);
+        ijkmp_set_inject_opaque(_mediaPlayer, (__bridge_retained void *) weakHolder);
         ijkmp_set_option_int(_mediaPlayer, IJKMP_OPT_CATEGORY_PLAYER, "start-on-prepared", _shouldAutoplay ? 1 : 0);
 
         // init video sink
@@ -467,6 +477,7 @@ - (void)shutdownClose:(IJKFFMoviePlayerController *) mySelf
     _liveOpenDelegate       = nil;
     _nativeInvokeDelegate   = nil;
 
+    __unused id weakHolder = (__bridge_transfer IJKWeakHolder*)ijkmp_set_inject_opaque(_mediaPlayer, NULL);
     ijkmp_dec_ref_p(&_mediaPlayer);
 
     [self didShutdown];
@@ -1089,7 +1100,6 @@ int media_player_msg_loop(void* arg)
     @autoreleasepool {
         IjkMediaPlayer *mp = (IjkMediaPlayer*)arg;
         __weak IJKFFMoviePlayerController *ffpController = ffplayerRetain(ijkmp_set_weak_thiz(mp, NULL));
-
         while (ffpController) {
             @autoreleasepool {
                 IJKFFMoviePlayerMessage *msg = [ffpController obtainMessage];
@@ -1295,7 +1305,10 @@ static int onInjectOnHttpEvent(IJKFFMoviePlayerController *mpc, int type, void *
 // NOTE: could be called from multiple thread
 static int ijkff_inject_callback(void *opaque, int message, void *data, size_t data_size)
 {
-    IJKFFMoviePlayerController *mpc = (__bridge IJKFFMoviePlayerController*)opaque;
+    IJKWeakHolder *weakHolder = (__bridge IJKWeakHolder*)opaque;
+    IJKFFMoviePlayerController *mpc = weakHolder.object;
+    if (!mpc)
+        return 0;
 
     switch (message) {
         case AVAPP_CTRL_WILL_CONCAT_SEGMENT_OPEN: