fix a sync problem. 88/18188/1
authorKitae Kim <kt920.kim@samsung.com>
Mon, 3 Mar 2014 10:29:38 +0000 (19:29 +0900)
committerKitae Kim <kt920.kim@samsung.com>
Mon, 17 Mar 2014 10:04:16 +0000 (19:04 +0900)
There is a variable to manage how many codecs are used in a process.
As depending on the count variable, this plugin determine whether release resources or not.
However, there is an exception case that the variable can be invalid,
and it causes SEGV problem when opening and closing CODECs concurrently.

Change-Id: If5bab4c679f986d0cb7108fd36c23ce57c741ab2
Signed-off-by: Kitae Kim <kt920.kim@samsung.com>
packaging/gst-plugins-emulator.spec
src/gstmarudevice.c
src/gstmaruenc.c

index f961ef2..2989d2d 100644 (file)
@@ -1,5 +1,5 @@
 Name: gst-plugins-emulator
-Version: 0.2.4
+Version: 0.2.6
 Release: 0
 Summary: GStreamer Decoder and Encoder Plugins for Emulator
 Group: Multimedia/Libraries
index 1b97342..70ff395 100644 (file)
@@ -45,8 +45,8 @@ static GMutex gst_avcodec_mutex;
 
 #define CODEC_DEVICE_MEM_SIZE 32 * 1024 * 1024
 
-gpointer device_mem = NULL;
-int device_fd = 0;
+gpointer device_mem = MAP_FAILED;
+int device_fd = -1;
 int opened_cnt = 0;
 
 int
@@ -55,49 +55,46 @@ gst_maru_codec_device_open (CodecDevice *dev, int media_type)
   CODEC_LOG (DEBUG, "enter: %s\n", __func__);
 
   g_mutex_lock (&gst_avcodec_mutex);
-  if (!device_fd) {
+  if (device_fd == -1) {
     if ((device_fd = open(CODEC_DEV, O_RDWR)) < 0) {
-      perror("Failed to open codec device.");
+      GST_ERROR ("failed to open codec device.");
       g_mutex_unlock (&gst_avcodec_mutex);
       return -1;
     }
-    CODEC_LOG (INFO, "succeeded to open %s. %d.\n", CODEC_DEV, device_fd);
+    GST_INFO ("succeeded to open %s. %d", CODEC_DEV, device_fd);
   } else {
-    CODEC_LOG (DEBUG, "codec device is already opened\n");
+    GST_DEBUG ("codec device is already opened");
   }
-  g_mutex_unlock (&gst_avcodec_mutex);
-
   dev->fd = device_fd;
+  // g_mutex_unlock (&gst_avcodec_mutex);
 
   // FIXME
   dev->buf_size = CODEC_DEVICE_MEM_SIZE;
-  CODEC_LOG (DEBUG, "mmap_size: %d\n", dev->buf_size);
+  GST_DEBUG ("mmap_size: %d", dev->buf_size);
   dev->mem_info.index = dev->buf_size;
 
-  g_mutex_lock (&gst_avcodec_mutex);
-  if (!device_mem) {
+  // g_mutex_lock (&gst_avcodec_mutex);
+  if (device_mem == MAP_FAILED) {
     device_mem =
       mmap (NULL, CODEC_DEVICE_MEM_SIZE, PROT_READ | PROT_WRITE,
           MAP_SHARED, device_fd, 0);
     if (device_mem == MAP_FAILED) {
-      perror("Failed to map device memory of codec.");
-#if 0
+      GST_ERROR ("failed to map device memory of codec");
       close (device_fd);
-      device_fd = 0;
-#endif
+      dev->fd = device_fd = -1;
       g_mutex_unlock (&gst_avcodec_mutex);
       return -1;
     }
-    CODEC_LOG (INFO, "succeeded to map device memory: %p.\n", device_mem);
+    GST_INFO ("succeeded to map device memory: %p", device_mem);
   } else {
-    CODEC_LOG (DEBUG, "mapping device memory is already done\n");
+    GST_DEBUG ("mapping device memory is already done");
   }
+  dev->buf = device_mem;
+
   opened_cnt++;
-  CODEC_LOG (DEBUG, "open count: %d\n", opened_cnt);
+  GST_DEBUG ("open count: %d", opened_cnt);
   g_mutex_unlock (&gst_avcodec_mutex);
 
-  dev->buf = device_mem;
-
   CODEC_LOG (DEBUG, "leave: %s\n", __func__);
 
   return 0;
@@ -112,27 +109,31 @@ gst_maru_codec_device_close (CodecDevice *dev)
 
   fd = dev->fd;
   if (fd < 0) {
-    GST_ERROR("Failed to get %s fd.\n", CODEC_DEV);
+    GST_ERROR ("Failed to get %s fd.\n", CODEC_DEV);
     return -1;
   }
 
   g_mutex_lock (&gst_avcodec_mutex);
-  opened_cnt--;
-  if (!opened_cnt) {
-    CODEC_LOG (INFO, "Release memory region of %p.\n", device_mem);
+  if (opened_cnt > 0) {
+    opened_cnt--;
+  }
+  GST_DEBUG ("open count: %d", opened_cnt);
+
+  if (opened_cnt == 0) {
+    GST_INFO ("release device memory %p", device_mem);
     if (munmap(device_mem, CODEC_DEVICE_MEM_SIZE) != 0) {
-      CODEC_LOG(ERR, "Failed to release memory region of %s.\n", CODEC_DEV);
+      GST_ERROR ("failed to release device memory of %s", CODEC_DEV);
     }
-    device_mem = NULL;
+    device_mem = MAP_FAILED;
 
-    CODEC_LOG (INFO, "close %s.\n", CODEC_DEV);
+    GST_INFO ("close %s", CODEC_DEV);
     if (close(fd) != 0) {
-      GST_ERROR("Failed to close %s. fd: %d\n", CODEC_DEV, fd);
+      GST_ERROR ("failed to close %s fd: %d\n", CODEC_DEV, fd);
     }
-    device_fd = 0;
+    dev->fd = device_fd = -1;
   }
+  dev->buf = MAP_FAILED;
   g_mutex_unlock (&gst_avcodec_mutex);
-  dev->buf = NULL;
 
   CODEC_LOG (DEBUG, "leave: %s\n", __func__);
 
@@ -147,7 +148,7 @@ gst_maru_avcodec_open (CodecContext *ctx,
   int ret;
 
   if (gst_maru_codec_device_open (dev, codec->media_type) < 0) {
-    perror("failed to open device.\n");
+    GST_ERROR ("failed to open device");
     return -1;
   }
 
@@ -163,7 +164,17 @@ gst_maru_avcodec_close (CodecContext *ctx, CodecDevice *dev)
 {
   int ret;
 
-  CODEC_LOG (DEBUG, "gst_maru_avcodec_close\n");
+  GST_DEBUG ("close %d of context", ctx->index);
+
+  if (ctx && ctx->index == 0) {
+    GST_INFO ("context is not opened yet or context before %d", ctx->index);
+    return -1;
+  }
+
+  if (dev && dev->fd < 0) {
+    GST_INFO ("fd is not opened yet or closed before %d", dev->fd);
+    return -1;
+  }
 
   g_mutex_lock (&gst_avcodec_mutex);
   codec_deinit (ctx, dev);
index b52958c..dec263d 100644 (file)
@@ -436,7 +436,6 @@ gst_maruenc_getcaps (GstPad *pad)
       GST_DEBUG_OBJECT (maruenc, "Opening codec failed with pixfmt: %d", pixfmt);
     }
 
-    gst_maru_avcodec_close (ctx, maruenc->dev);
 #if 0
     if (ctx->priv_data) {
       gst_maru_avcodec_close (ctx, maruenc->dev);
@@ -471,7 +470,6 @@ gst_maruenc_setcaps (GstPad *pad, GstCaps *caps)
   GstCaps *allowed_caps;
   GstCaps *icaps;
   enum PixelFormat pix_fmt;
-  int32_t buf_size;
 
   maruenc = (GstMaruEnc *) (gst_pad_get_parent (pad));
   oclass = (GstMaruEncClass *) (G_OBJECT_GET_CLASS (maruenc));
@@ -527,28 +525,6 @@ gst_maruenc_setcaps (GstPad *pad, GstCaps *caps)
 
   pix_fmt = maruenc->context->video.pix_fmt;
 
-  {
-    switch (oclass->codec->media_type) {
-    case AVMEDIA_TYPE_VIDEO:
-    {
-      int width, height;
-
-      width = maruenc->context->video.width;
-      height = maruenc->context->video.height;
-      buf_size = width * height * 6 + FF_MIN_BUFFER_SIZE + 100;
-      break;
-    }
-    case AVMEDIA_TYPE_AUDIO:
-        buf_size = FF_MAX_AUDIO_FRAME_SIZE + 100;
-        break;
-    default:
-        buf_size = -1;
-        break;
-    }
-  }
-
-  maruenc->dev->buf_size = gst_maru_align_size(buf_size);
-
   // open codec
   if (gst_maru_avcodec_open (maruenc->context,
       oclass->codec, maruenc->dev) < 0) {
@@ -587,7 +563,7 @@ gst_maruenc_setcaps (GstPad *pad, GstCaps *caps)
   other_caps =
   gst_maru_codecname_to_caps (oclass->codec->name, maruenc->context, TRUE);
   if (!other_caps) {
-  GST_DEBUG("Unsupported codec - no caps found");
+    GST_DEBUG ("Unsupported codec - no caps found");
     gst_maru_avcodec_close (maruenc->context, maruenc->dev);
     return FALSE;
   }
@@ -1090,6 +1066,7 @@ gst_maruenc_change_state (GstElement *element, GstStateChange transition)
   case GST_STATE_CHANGE_PAUSED_TO_READY:
     gst_maruenc_flush_buffers (maruenc, FALSE);
     if (maruenc->opened) {
+      GST_DEBUG_OBJECT (maruenc, "change_state: PAUSED_TO_READY, close context");
       gst_maru_avcodec_close (maruenc->context, maruenc->dev);
       maruenc->opened = FALSE;
     }