S5PC11X: libcamera: buffer cleanup

Cleanup how buffers are managed.  Before, buffers for preview
and capture and record were mixed together, with one thread
cleaning up buffers for another.  Now, each mode cleans up
its own buffers.  Implement asychronous buffer release for record
case to fix a bug where we allowed FIMC to update a buffer that
was still in use by encoder, resulting in encoding a frame that
was part one image and part another.

Change-Id: I42d0032cea61197a1f3a665a7248b700599d5a6a
Signed-off-by: Mike J. Chen <mjchen@sta.samsung.com>
diff --git a/libcamera/SecCamera.cpp b/libcamera/SecCamera.cpp
index 38f675c..6998a5d 100755
--- a/libcamera/SecCamera.cpp
+++ b/libcamera/SecCamera.cpp
@@ -81,21 +81,6 @@
     return time;
 }
 
-static int close_buffers(struct fimc_buffer *buffers)
-{
-    int i;
-
-    for (i = 0; i < MAX_BUFFERS; i++) {
-        if (buffers[i].start) {
-            munmap(buffers[i].start, buffers[i].length);
-            //LOGV("munmap():virt. addr[%d]: 0x%x size = %d\n", i, (unsigned int) buffers[i].start, buffers[i].length);
-            buffers[i].start = NULL;
-        }
-    }
-
-    return 0;
-}
-
 static int get_pixel_depth(unsigned int fmt)
 {
     int depth = 0;
@@ -137,21 +122,19 @@
 #define ALIGN_H(x)      (((x) + 0x1F) & (~0x1F))    // Set as multiple of 32
 #define ALIGN_BUF(x)    (((x) + 0x1FFF)& (~0x1FFF)) // Set as multiple of 8K
 
-static int init_yuv_buffers(struct fimc_buffer *buffers, int width, int height, unsigned int fmt)
+static int init_preview_buffers(struct fimc_buffer *buffers, int width, int height, unsigned int fmt)
 {
     int i, len;
 
-    len = (width * height * get_pixel_depth(fmt)) / 8;
+    if (fmt==V4L2_PIX_FMT_NV12T) {
+        len = ALIGN_BUF(ALIGN_W(width) * ALIGN_H(height)) +
+              ALIGN_BUF(ALIGN_W(width) * ALIGN_H(height / 2));
+    } else {
+        len = (width * height * get_pixel_depth(fmt)) / 8;
+    }
 
     for (i = 0; i < MAX_BUFFERS; i++) {
-        if (fmt==V4L2_PIX_FMT_NV12T) {
-            buffers[i].start = NULL;
-            buffers[i].length = ALIGN_BUF(ALIGN_W(width) * ALIGN_H(height)) +
-                                ALIGN_BUF(ALIGN_W(width) * ALIGN_H(height / 2));
-        } else {
-            buffers[i].start = NULL;
-            buffers[i].length = len;
-        }
+        buffers[i].length = len;
     }
 
     return 0;
@@ -178,11 +161,28 @@
     return ret;
 }
 
-static int fimc_esd_poll(struct pollfd *events)
+int SecCamera::previewPoll(bool preview)
 {
     int ret;
 
-    ret = poll(events, 1, 1000);
+    if (preview) {
+#ifdef ENABLE_ESD_PREVIEW_CHECK
+        int status = 0;
+
+        if (!(++m_esd_check_count % 60)) {
+            status = getCameraSensorESDStatus();
+            m_esd_check_count = 0;
+            if (status) {
+               LOGE("ERR(%s) ESD status(%d)", __func__, status);
+               return status;
+            }
+        }
+#endif
+
+        ret = poll(&m_events_c, 1, 1000);
+    } else {
+        ret = poll(&m_events_c2, 1, 1000);
+    }
 
     if (ret < 0) {
         LOGE("ERR(%s):poll error\n", __func__);
@@ -355,34 +355,34 @@
     return req.count;
 }
 
-static int fimc_v4l2_querybuf(int fp, struct fimc_buffer *buffers, enum v4l2_buf_type type, int nr_frames)
+static int fimc_v4l2_querybuf(int fp, struct fimc_buffer *buffer, enum v4l2_buf_type type)
 {
     struct v4l2_buffer v4l2_buf;
-    int i, ret;
+    int ret;
 
-    for (i = 0; i < nr_frames; i++) {
-        v4l2_buf.type = type;
-        v4l2_buf.memory = V4L2_MEMORY_MMAP;
-        v4l2_buf.index = i;
+    LOGI("%s :", __func__);
 
-        ret = ioctl(fp , VIDIOC_QUERYBUF, &v4l2_buf);
-        if (ret < 0) {
-            LOGE("ERR(%s):VIDIOC_QUERYBUF failed\n", __func__);
-            return -1;
-        }
+    v4l2_buf.type = type;
+    v4l2_buf.memory = V4L2_MEMORY_MMAP;
+    v4l2_buf.index = 0;
 
-        if (nr_frames == 1) {
-            buffers[i].length = v4l2_buf.length;
-            if ((buffers[i].start = (char *)mmap(0, v4l2_buf.length, PROT_READ | PROT_WRITE, MAP_SHARED,
-                    fp, v4l2_buf.m.offset)) < 0) {
-                LOGE("%s %d] mmap() failed\n",__func__, __LINE__);
-                return -1;
-            }
-
-            //LOGV("buffers[%d].start = %p v4l2_buf.length = %d", i, buffers[i].start, v4l2_buf.length);
-        }
+    ret = ioctl(fp , VIDIOC_QUERYBUF, &v4l2_buf);
+    if (ret < 0) {
+        LOGE("ERR(%s):VIDIOC_QUERYBUF failed\n", __func__);
+        return -1;
     }
 
+    buffer->length = v4l2_buf.length;
+    if ((buffer->start = (char *)mmap(0, v4l2_buf.length,
+                                         PROT_READ | PROT_WRITE, MAP_SHARED,
+                                         fp, v4l2_buf.m.offset)) < 0) {
+         LOGE("%s %d] mmap() failed\n",__func__, __LINE__);
+         return -1;
+    }
+
+    LOGI("%s: buffer->start = %p v4l2_buf.length = %d",
+         __func__, buffer->start, v4l2_buf.length);
+
     return 0;
 }
 
@@ -443,7 +443,7 @@
 
     ret = ioctl(fp, VIDIOC_DQBUF, &v4l2_buf);
     if (ret < 0) {
-        LOGE("ERR(%s):VIDIOC_DQBUF failed\n", __func__);
+        LOGE("ERR(%s):VIDIOC_DQBUF failed, dropped frame\n", __func__);
         return ret;
     }
 
@@ -604,6 +604,8 @@
     m_params->sharpness = -1;
     m_params->white_balance = -1;
 
+    memset(&m_capture_buf, 0, sizeof(m_capture_buf));
+
     LOGV("%s :", __func__);
 }
 
@@ -756,9 +758,9 @@
 
         stopRecord();
 
-	/* close m_cam_fd after stopRecord() because stopRecord()
-	 * uses m_cam_fd to change frame rate
-	 */
+        /* close m_cam_fd after stopRecord() because stopRecord()
+         * uses m_cam_fd to change frame rate
+         */
         LOGI("DeinitCamera: m_cam_fd(%d)", m_cam_fd);
         if (m_cam_fd > -1) {
             close(m_cam_fd);
@@ -822,11 +824,8 @@
     ret = fimc_v4l2_s_fmt(m_cam_fd, m_preview_width,m_preview_height,m_preview_v4lformat, 0);
     CHECK(ret);
 
-    init_yuv_buffers(m_buffers_c, m_preview_width, m_preview_height, m_preview_v4lformat);
     ret = fimc_v4l2_reqbufs(m_cam_fd, V4L2_BUF_TYPE_VIDEO_CAPTURE, MAX_BUFFERS);
     CHECK(ret);
-    ret = fimc_v4l2_querybuf(m_cam_fd, m_buffers_c, V4L2_BUF_TYPE_VIDEO_CAPTURE, MAX_BUFFERS);
-    CHECK(ret);
 
     LOGV("%s : m_preview_width: %d m_preview_height: %d m_angle: %d\n",
             __func__, m_preview_width, m_preview_height, m_angle);
@@ -880,10 +879,8 @@
 
     LOGV("%s :", __func__);
 
-    close_buffers(m_buffers_c);
-
     if (m_flag_camera_start == 0) {
-        LOGV("%s: doing nothing because m_flag_camera_start is zero", __func__);
+        LOGW("%s: doing nothing because m_flag_camera_start is zero", __func__);
         return 0;
     }
 
@@ -896,9 +893,9 @@
     }
 
     ret = fimc_v4l2_streamoff(m_cam_fd);
+    CHECK(ret);
 
     m_flag_camera_start = 0;
-    CHECK(ret);
 
     return ret;
 }
@@ -906,6 +903,8 @@
 //Recording
 int SecCamera::startRecord(void)
 {
+    int ret, i;
+
     LOGV("%s :", __func__);
 
     // aleady started
@@ -919,30 +918,22 @@
         return -1;
     }
 
-    memset(&m_events_c2, 0, sizeof(m_events_c2));
-    m_events_c2.fd = m_cam_fd2;
-    m_events_c2.events = POLLIN | POLLERR;
-
-    int m_record_v4lformat = V4L2_PIX_FMT_NV12T;
     /* enum_fmt, s_fmt sample */
-    int ret = fimc_v4l2_enum_fmt(m_cam_fd2,m_record_v4lformat);
+    ret = fimc_v4l2_enum_fmt(m_cam_fd2, V4L2_PIX_FMT_NV12T);
     CHECK(ret);
 
-    LOGE("%s: m_recording_width = %d, m_recording_height = %d\n", __func__, m_recording_width, m_recording_height);
+    LOGI("%s: m_recording_width = %d, m_recording_height = %d\n",
+         __func__, m_recording_width, m_recording_height);
 
-    ret = fimc_v4l2_s_fmt(m_cam_fd2, m_recording_width, m_recording_height, m_record_v4lformat, 0);
-
+    ret = fimc_v4l2_s_fmt(m_cam_fd2, m_recording_width,
+                          m_recording_height, V4L2_PIX_FMT_NV12T, 0);
     CHECK(ret);
 
-    init_yuv_buffers(m_buffers_c2, m_recording_width, m_recording_height, m_record_v4lformat);
-
     ret = fimc_v4l2_reqbufs(m_cam_fd2, V4L2_BUF_TYPE_VIDEO_CAPTURE, MAX_BUFFERS);
     CHECK(ret);
-    ret = fimc_v4l2_querybuf(m_cam_fd2, m_buffers_c2, V4L2_BUF_TYPE_VIDEO_CAPTURE, MAX_BUFFERS);
-    CHECK(ret);
 
     /* start with all buffers in queue */
-    for (int i = 0; i < MAX_BUFFERS; i++) {
+    for (i = 0; i < MAX_BUFFERS; i++) {
         ret = fimc_v4l2_qbuf(m_cam_fd2, i);
         CHECK(ret);
     }
@@ -950,7 +941,10 @@
     ret = fimc_v4l2_streamon(m_cam_fd2);
     CHECK(ret);
 
-    // It is a delay for a new frame, not to show the previous bigger ugly picture frame.
+    // Get and throw away the first frame since it is often garbled.
+    memset(&m_events_c2, 0, sizeof(m_events_c2));
+    m_events_c2.fd = m_cam_fd2;
+    m_events_c2.events = POLLIN | POLLERR;
     ret = fimc_poll(&m_events_c2);
     CHECK(ret);
 
@@ -963,21 +957,21 @@
 {
     int ret;
 
-    if (m_flag_record_start == 0)
-        return 0;
-
     LOGV("%s :", __func__);
 
-    close_buffers(m_buffers_c2);
+    if (m_flag_record_start == 0) {
+        LOGW("%s: doing nothing because m_flag_record_start is zero", __func__);
+        return 0;
+    }
 
     if (m_cam_fd2 <= 0) {
         LOGE("ERR(%s):Camera was closed\n", __func__);
         return -1;
     }
 
-    ret = fimc_v4l2_streamoff(m_cam_fd2);
-
     m_flag_record_start = 0;
+
+    ret = fimc_v4l2_streamoff(m_cam_fd2);
     CHECK(ret);
 
     return 0;
@@ -1029,20 +1023,7 @@
     int index;
     int ret;
 
-#ifdef ENABLE_ESD_PREVIEW_CHECK
-    int status = 0;
-
-    if (!(++m_esd_check_count % 60)) {
-        status = getCameraSensorESDStatus();
-        m_esd_check_count = 0;
-    }
-#endif // ENABLE_ESD_PREVIEW_CHECK
-
-#ifdef ENABLE_ESD_PREVIEW_CHECK
-    if (m_flag_camera_start == 0 || fimc_esd_poll(&m_events_c) == 0 || status) {
-#else
-    if (m_flag_camera_start == 0 || fimc_esd_poll(&m_events_c) == 0) {
-#endif
+    if (m_flag_camera_start == 0 || previewPoll(true) == 0) {
         LOGE("ERR(%s):Start Camera Device Reset \n", __func__);
         /* GAUDI Project([arun.c@samsung.com]) 2010.05.20. [Implemented ESD code] */
         /*
@@ -1060,12 +1041,12 @@
         ret = fimc_v4l2_s_input(m_cam_fd, 1000);
         CHECK(ret);
         ret = startPreview();
-
         if (ret < 0) {
             LOGE("ERR(%s): startPreview() return %d\n", __func__, ret);
             return 0;
         }
     }
+
     index = fimc_v4l2_dqbuf(m_cam_fd);
     if (!(0 <= index && index < MAX_BUFFERS)) {
         LOGE("ERR(%s):wrong index = %d\n", __func__, index);
@@ -1078,25 +1059,31 @@
     return index;
 }
 
-int SecCamera::getRecord()
+int SecCamera::getRecordFrame()
 {
-    int index;
-
     if (m_flag_record_start == 0) {
         LOGE("%s: m_flag_record_start is 0", __func__);
-        startRecord();
-    }
-    fimc_poll(&m_events_c2);
-    index = fimc_v4l2_dqbuf(m_cam_fd2);
-    if (!(0 <= index && index < MAX_BUFFERS)) {
-        LOGE("ERR(%s):wrong index = %d\n", __func__, index);
         return -1;
     }
 
-    int ret = fimc_v4l2_qbuf(m_cam_fd2, index);
-    CHECK(ret);
+    previewPoll(false);
+    return fimc_v4l2_dqbuf(m_cam_fd2);
+}
 
-    return index;
+int SecCamera::releaseRecordFrame(int index)
+{
+    if (!m_flag_record_start) {
+        /* this can happen when recording frames are returned after
+         * the recording is stopped at the driver level.  we don't
+         * need to return the buffers in this case and we've seen
+         * cases where fimc could crash if we called qbuf and it
+         * wasn't expecting it.
+         */
+        LOGI("%s: recording not in progress, ignoring", __func__);
+        return 0;
+    }
+
+    return fimc_v4l2_qbuf(m_cam_fd2, index);
 }
 
 int SecCamera::setPreviewSize(int width, int height, int pixel_format)
@@ -1175,6 +1162,7 @@
 
     if (m_flag_camera_start > 0) {
         LOG_TIME_START(0)
+        LOGW("WARN(%s):Camera was in preview, should have been stopped\n", __func__);
         stopPreview();
         LOG_TIME_END(0)
     }
@@ -1190,11 +1178,9 @@
     CHECK(ret);
     ret = fimc_v4l2_s_fmt_cap(m_cam_fd, m_snapshot_width, m_snapshot_height, V4L2_PIX_FMT_JPEG);
     CHECK(ret);
-    init_yuv_buffers(m_buffers_c, m_snapshot_width, m_snapshot_height, m_snapshot_v4lformat);
-
     ret = fimc_v4l2_reqbufs(m_cam_fd, V4L2_BUF_TYPE_VIDEO_CAPTURE, nframe);
     CHECK(ret);
-    ret = fimc_v4l2_querybuf(m_cam_fd, m_buffers_c, V4L2_BUF_TYPE_VIDEO_CAPTURE, nframe);
+    ret = fimc_v4l2_querybuf(m_cam_fd, &m_capture_buf, V4L2_BUF_TYPE_VIDEO_CAPTURE);
     CHECK(ret);
 
     ret = fimc_v4l2_qbuf(m_cam_fd, 0);
@@ -1207,6 +1193,21 @@
     return 0;
 }
 
+int SecCamera::endSnapshot(void)
+{
+    int ret;
+
+    LOGI("%s :", __func__);
+    if (m_capture_buf.start) {
+        munmap(m_capture_buf.start, m_capture_buf.length);
+        LOGI("munmap():virt. addr %p size = %d\n",
+             m_capture_buf.start, m_capture_buf.length);
+        m_capture_buf.start = NULL;
+        m_capture_buf.length = 0;
+    }
+    return 0;
+}
+
 /*
  * Set Jpeg quality & exif info and get JPEG data from camera ISP
  */
@@ -1223,7 +1224,7 @@
     ret = fimc_poll(&m_events_c);
     CHECK_PTR(ret);
     index = fimc_v4l2_dqbuf(m_cam_fd);
-    if (!(0 <= index && index < MAX_BUFFERS)) {
+    if (index != 0) {
         LOGE("ERR(%s):wrong index = %d\n", __func__, index);
         return NULL;
     }
@@ -1241,7 +1242,7 @@
     LOGV("\nsnapshot dqueued buffer = %d snapshot_width = %d snapshot_height = %d, size = %d\n\n",
             index, m_snapshot_width, m_snapshot_height, *jpeg_size);
 
-    addr = (unsigned char*)(m_buffers_c[index].start) + main_offset;
+    addr = (unsigned char*)(m_capture_buf.start) + main_offset;
     *phyaddr = getPhyAddrY(index) + m_postview_offset;
 
     LOG_TIME_START(2) // post
@@ -1381,6 +1382,7 @@
 
     if (m_flag_camera_start > 0) {
         LOG_TIME_START(0)
+        LOGW("WARN(%s):Camera was in preview, should have been stopped\n", __func__);
         stopPreview();
         LOG_TIME_END(0)
     }
@@ -1417,11 +1419,9 @@
     CHECK(ret);
     ret = fimc_v4l2_s_fmt_cap(m_cam_fd, m_snapshot_width, m_snapshot_height, m_snapshot_v4lformat);
     CHECK(ret);
-    init_yuv_buffers(m_buffers_c, m_snapshot_width, m_snapshot_height, m_snapshot_v4lformat);
-
     ret = fimc_v4l2_reqbufs(m_cam_fd, V4L2_BUF_TYPE_VIDEO_CAPTURE, nframe);
     CHECK(ret);
-    ret = fimc_v4l2_querybuf(m_cam_fd, m_buffers_c, V4L2_BUF_TYPE_VIDEO_CAPTURE, nframe);
+    ret = fimc_v4l2_querybuf(m_cam_fd, &m_capture_buf, V4L2_BUF_TYPE_VIDEO_CAPTURE);
     CHECK(ret);
 
     ret = fimc_v4l2_qbuf(m_cam_fd, 0);
@@ -1440,7 +1440,8 @@
 
     LOG_TIME_END(2)
 
-    memcpy(yuv_buf, (unsigned char*)m_buffers_c[index].start, m_snapshot_width * m_snapshot_height * 2);
+    LOGI("%s : calling memcpy from m_capture_buf", __func__);
+    memcpy(yuv_buf, (unsigned char*)m_capture_buf.start, m_snapshot_width * m_snapshot_height * 2);
     LOG_TIME_START(5) // post
     fimc_v4l2_streamoff(m_cam_fd);
     LOG_TIME_END(5)
@@ -1579,21 +1580,21 @@
     if (m_snapshot_v4lformat == V4L2_PIX_FMT_YUV420)
         LOGE("%s : SnapshotFormat:V4L2_PIX_FMT_YUV420", __func__);
     else if (m_snapshot_v4lformat == V4L2_PIX_FMT_NV12)
-        LOGE("%s : SnapshotFormat:V4L2_PIX_FMT_NV12", __func__);
+        LOGD("%s : SnapshotFormat:V4L2_PIX_FMT_NV12", __func__);
     else if (m_snapshot_v4lformat == V4L2_PIX_FMT_NV12T)
-        LOGE("%s : SnapshotFormat:V4L2_PIX_FMT_NV12T", __func__);
+        LOGD("%s : SnapshotFormat:V4L2_PIX_FMT_NV12T", __func__);
     else if (m_snapshot_v4lformat == V4L2_PIX_FMT_NV21)
-        LOGE("%s : SnapshotFormat:V4L2_PIX_FMT_NV21", __func__);
+        LOGD("%s : SnapshotFormat:V4L2_PIX_FMT_NV21", __func__);
     else if (m_snapshot_v4lformat == V4L2_PIX_FMT_YUV422P)
-        LOGE("%s : SnapshotFormat:V4L2_PIX_FMT_YUV422P", __func__);
+        LOGD("%s : SnapshotFormat:V4L2_PIX_FMT_YUV422P", __func__);
     else if (m_snapshot_v4lformat == V4L2_PIX_FMT_YUYV)
-        LOGE("%s : SnapshotFormat:V4L2_PIX_FMT_YUYV", __func__);
+        LOGD("%s : SnapshotFormat:V4L2_PIX_FMT_YUYV", __func__);
     else if (m_snapshot_v4lformat == V4L2_PIX_FMT_UYVY)
-        LOGE("%s : SnapshotFormat:V4L2_PIX_FMT_UYVY", __func__);
+        LOGD("%s : SnapshotFormat:V4L2_PIX_FMT_UYVY", __func__);
     else if (m_snapshot_v4lformat == V4L2_PIX_FMT_RGB565)
-        LOGE("%s : SnapshotFormat:V4L2_PIX_FMT_RGB565", __func__);
+        LOGD("%s : SnapshotFormat:V4L2_PIX_FMT_RGB565", __func__);
     else
-        LOGE("SnapshotFormat:UnknownFormat");
+        LOGD("SnapshotFormat:UnknownFormat");
 #endif
     return 0;
 }
@@ -1603,11 +1604,6 @@
     return m_snapshot_v4lformat;
 }
 
-int SecCamera::cancelPicture(void)
-{
-    return close_buffers(m_buffers_c);
-}
-
 // ======================================================================
 // Settings
 
@@ -1752,6 +1748,7 @@
             }
         }
     }
+
     return 0;
 }
 
@@ -1777,6 +1774,7 @@
             }
         }
     }
+
     return 0;
 }
 
diff --git a/libcamera/SecCamera.h b/libcamera/SecCamera.h
index 7710ce4..0580373 100644
--- a/libcamera/SecCamera.h
+++ b/libcamera/SecCamera.h
@@ -292,11 +292,11 @@
 
     int             startRecord(void);
     int             stopRecord(void);
-    int             getRecord(void);
+    int             getRecordFrame(void);
+    int             releaseRecordFrame(int index);
     unsigned int    getRecPhyAddrY(int);
     unsigned int    getRecPhyAddrC(int);
 
-    int             cancelPicture(void);
     int             getPreview(void);
     int             setPreviewSize(int width, int height, int pixel_format);
     int             getPreviewSize(int *width, int *height, int *frame_size);
@@ -410,6 +410,7 @@
     int             setExifOrientationInfo(int orientationInfo);
     int             setBatchReflection(void);
     int             setSnapshotCmd(void);
+    int             endSnapshot(void);
     int             setCameraSensorReset(void);
     int             setSensorMode(int sensor_mode); /* Camcorder fix fps */
     int             setShotMode(int shot_mode);     /* Shot mode */
@@ -424,6 +425,7 @@
     int             setDefultIMEI(int imei);
     int             getDefultIMEI(void);
     const __u8*     getCameraSensorName(void);
+    int             previewPoll(bool preview);
 #ifdef ENABLE_ESD_PREVIEW_CHECK
     int             getCameraSensorESDStatus(void);
 #endif // ENABLE_ESD_PREVIEW_CHECK
@@ -495,7 +497,6 @@
     int             m_cam_fd2;
     struct pollfd   m_events_c2;
     int             m_flag_record_start;
-    struct          fimc_buffer m_buffers_c2[MAX_BUFFERS];
 
     int             m_preview_v4lformat;
     int             m_preview_width;
@@ -553,7 +554,7 @@
 
     exif_attribute_t mExifInfo;
 
-    struct fimc_buffer m_buffers_c[MAX_BUFFERS];
+    struct fimc_buffer m_capture_buf;
     struct pollfd   m_events_c;
 
     inline int      m_frameSize(int format, int width, int height);
diff --git a/libcamera/SecCameraHWInterface.cpp b/libcamera/SecCameraHWInterface.cpp
index bbbb48c..7e36a9d 100644
--- a/libcamera/SecCameraHWInterface.cpp
+++ b/libcamera/SecCameraHWInterface.cpp
@@ -64,7 +64,6 @@
 
 CameraHardwareSec::CameraHardwareSec(int cameraId)
         :
-          mPreviewRunning(false),
           mCaptureInProgress(false),
           mParameters(),
           mPreviewHeap(0),
@@ -129,6 +128,11 @@
     initDefaultParameters(cameraId);
 
     mExitAutoFocusThread = false;
+    mExitPreviewThread = false;
+    /* whether the PreviewThread is active in preview or stopped.  we
+     * create the thread but it is initially in stopped state.
+     */
+    mPreviewRunning = false;
     mPreviewThread = new PreviewThread(this);
     mAutoFocusThread = new AutoFocusThread(this);
     mPictureThread = new PictureThread(this);
@@ -414,9 +418,37 @@
     mSkipFrame = frame;
 }
 
+int CameraHardwareSec::previewThreadWrapper()
+{
+    LOGI("%s: starting", __func__);
+    while (1) {
+        mPreviewLock.lock();
+        while (!mPreviewRunning) {
+            LOGI("%s: calling mSecCamera->stopPreview() and waiting", __func__);
+            mSecCamera->stopPreview();
+            /* signal that we're stopping */
+            mPreviewStoppedCondition.signal();
+            mPreviewCondition.wait(mPreviewLock);
+            LOGI("%s: return from wait", __func__);
+        }
+        mPreviewLock.unlock();
+
+        if (mExitPreviewThread) {
+            LOGI("%s: exiting", __func__);
+            mSecCamera->stopPreview();
+            return 0;
+        }
+        previewThread();
+    }
+}
+
 int CameraHardwareSec::previewThread()
 {
     int index;
+    nsecs_t timestamp;
+    unsigned int phyYAddr;
+    unsigned int phyCAddr;
+    struct addrs *addrs;
 
     index = mSecCamera->getPreview();
     if (index < 0) {
@@ -431,7 +463,15 @@
     }
     mSkipFrameLock.unlock();
 
-    nsecs_t timestamp = systemTime(SYSTEM_TIME_MONOTONIC);
+    timestamp = systemTime(SYSTEM_TIME_MONOTONIC);
+
+    phyYAddr = mSecCamera->getPhyAddrY(index);
+    phyCAddr = mSecCamera->getPhyAddrC(index);
+
+    if (phyYAddr == 0xffffffff || phyCAddr == 0xffffffff) {
+        LOGE("ERR(%s):Fail on SecCamera getPhyAddr Y addr = %0x C addr = %0x", __func__, phyYAddr, phyCAddr);
+        return UNKNOWN_ERROR;
+     }
 
     int width, height, frame_size, offset;
 
@@ -440,45 +480,29 @@
     offset = (frame_size + 16) * index;
     sp<MemoryBase> buffer = new MemoryBase(mPreviewHeap, offset, frame_size);
 
-    unsigned int phyYAddr = mSecCamera->getPhyAddrY(index);
-    unsigned int phyCAddr = mSecCamera->getPhyAddrC(index);
-
-    if (phyYAddr == 0xffffffff || phyCAddr == 0xffffffff) {
-        LOGE("ERR(%s):Fail on SecCamera. Invalid PhyAddr, Y addr = %0x C addr = %0x",
-                __func__, phyYAddr, phyCAddr);
-        return UNKNOWN_ERROR;
-    }
     memcpy(static_cast<unsigned char *>(mPreviewHeap->base()) + (offset + frame_size    ), &phyYAddr, 4);
     memcpy(static_cast<unsigned char *>(mPreviewHeap->base()) + (offset + frame_size + 4), &phyCAddr, 4);
 
 #if defined(BOARD_USES_OVERLAY)
     if (mUseOverlay) {
         int ret;
+        overlay_buffer_t overlay_buffer;
+
         mOverlayBufferIdx ^= 1;
         memcpy(static_cast<unsigned char*>(mPreviewHeap->base()) + offset + frame_size + sizeof(phyYAddr) + sizeof(phyCAddr),
-                    &mOverlayBufferIdx, sizeof(mOverlayBufferIdx));
+                &mOverlayBufferIdx, sizeof(mOverlayBufferIdx));
 
         ret = mOverlay->queueBuffer((void*)(static_cast<unsigned char *>(mPreviewHeap->base()) + (offset + frame_size)));
 
-        if (ret == ALL_BUFFERS_FLUSHED) {
-            goto OverlayEnd;
-        } else if (ret == -1) {
+        if (ret == -1 ) {
             LOGE("ERR(%s):overlay queueBuffer fail", __func__);
-            goto OverlayEnd;
-        }
-
-        overlay_buffer_t overlay_buffer;
-        ret = mOverlay->dequeueBuffer(&overlay_buffer);
-
-        if (ret == ALL_BUFFERS_FLUSHED) {
-            goto OverlayEnd;
-        } else if (ret == -1) {
-            LOGE("ERR(%s):overlay dequeueBuffer fail", __func__);
-            goto OverlayEnd;
-        }
-    }
-
-OverlayEnd:
+        } else if (ret != ALL_BUFFERS_FLUSHED) {
+            ret = mOverlay->dequeueBuffer(&overlay_buffer);
+            if (ret == -1) {
+                LOGE("ERR(%s):overlay dequeueBuffer fail", __func__);
+            }
+         }
+     }
 #endif
 
     // Notify the client of a new frame.
@@ -487,35 +511,32 @@
     }
 
     if (mRecordRunning == true) {
-        int index = mSecCamera->getRecord();
-
+        index = mSecCamera->getRecordFrame();
         if (index < 0) {
             LOGE("ERR(%s):Fail on SecCamera->getRecord()", __func__);
             return UNKNOWN_ERROR;
         }
 
-        unsigned int phyYAddr = mSecCamera->getRecPhyAddrY(index);
-        unsigned int phyCAddr = mSecCamera->getRecPhyAddrC(index);
+        phyYAddr = mSecCamera->getRecPhyAddrY(index);
+        phyCAddr = mSecCamera->getRecPhyAddrC(index);
 
         if (phyYAddr == 0xffffffff || phyCAddr == 0xffffffff) {
             LOGE("ERR(%s):Fail on SecCamera getRectPhyAddr Y addr = %0x C addr = %0x", __func__, phyYAddr, phyCAddr);
             return UNKNOWN_ERROR;
         }
-        struct addrs *addrs = (struct addrs *)mRecordHeap->base();
+
+        addrs = (struct addrs *)mRecordHeap->base();
 
         sp<MemoryBase> buffer = new MemoryBase(mRecordHeap, index * sizeof(struct addrs), sizeof(struct addrs));
         addrs[index].addr_y = phyYAddr;
         addrs[index].addr_cbcr = phyCAddr;
+        addrs[index].buf_index = index;
 
         // Notify the client of a new frame.
         if (mMsgEnabled & CAMERA_MSG_VIDEO_FRAME) {
-            //nsecs_t timestamp = systemTime(SYSTEM_TIME_MONOTONIC);
             mDataCbTimestamp(timestamp, CAMERA_MSG_VIDEO_FRAME, buffer, mCallbackCookie);
-        }
-    } else if (mRecordRunning == false) {
-        if (mSecCamera->stopRecord() < 0) {
-            LOGE("ERR(%s):Fail on mSecCamera->stopRecord()", __func__);
-            return UNKNOWN_ERROR;
+        } else {
+            mSecCamera->releaseRecordFrame(index);
         }
     }
 
@@ -529,18 +550,18 @@
     LOGV("%s :", __func__);
 
     Mutex::Autolock lock(mStateLock);
-    if (mPreviewRunning) {
-        // already running
-        LOGE("%s : preview thread already running", __func__);
-        return INVALID_OPERATION;
-    }
-
     if (mCaptureInProgress) {
         LOGE("%s : capture in progress, not allowed", __func__);
         return INVALID_OPERATION;
     }
 
-    mSecCamera->stopPreview();
+    mPreviewLock.lock();
+    if (mPreviewRunning) {
+        // already running
+        LOGE("%s : preview thread already running", __func__);
+        mPreviewLock.unlock();
+        return INVALID_OPERATION;
+    }
 
     setSkipFrame(INITIAL_SKIP_FRAME);
 
@@ -568,7 +589,8 @@
     LOGV("CameraHardwareSec: mPostViewWidth = %d mPostViewHeight = %d mPostViewSize = %d",mPostViewWidth,mPostViewHeight,mPostViewSize);
 
     mPreviewRunning = true;
-    mPreviewThread->run("CameraPreviewThread", PRIORITY_URGENT_DISPLAY);
+    mPreviewCondition.signal();
+    mPreviewLock.unlock();
 
     return NO_ERROR;
 }
@@ -636,24 +658,22 @@
 {
     LOGV("%s :", __func__);
 
-    if (!previewEnabled())
-        return;
-
-    /* request that the preview thread exit. we can wait because we're
-     * called by CameraServices with a lock but it has disabled all preview
-     * related callbacks so previewThread should not invoke any callbacks.
-     */
-    mPreviewThread->requestExitAndWait();
-
-    if (mSecCamera->stopPreview() < 0)
-        LOGE("ERR(%s):Fail on mSecCamera->stopPreview()", __func__);
-
-    mPreviewRunning = false;
+    /* request that the preview thread stop. */
+    mPreviewLock.lock();
+    if (mPreviewRunning) {
+        mPreviewRunning = false;
+        mPreviewCondition.signal();
+        /* wait until preview thread is stopped */
+        mPreviewStoppedCondition.wait(mPreviewLock);
+    } else {
+        LOGI("%s : preview not running, doing nothing", __func__);
+    }
+    mPreviewLock.unlock();
 }
 
 bool CameraHardwareSec::previewEnabled()
 {
-    Mutex::Autolock lock(mStateLock);
+    Mutex::Autolock lock(mPreviewLock);
     LOGV("%s : %d", __func__, mPreviewRunning);
     return mPreviewRunning;
 }
@@ -678,7 +698,13 @@
 {
     LOGV("%s :", __func__);
 
-    mRecordRunning = false;
+    if (mRecordRunning == true) {
+        if (mSecCamera->stopRecord() < 0) {
+            LOGE("ERR(%s):Fail on mSecCamera->stopRecord()", __func__);
+            return;
+        }
+        mRecordRunning = false;
+    }
 }
 
 bool CameraHardwareSec::recordingEnabled()
@@ -690,15 +716,11 @@
 
 void CameraHardwareSec::releaseRecordingFrame(const sp<IMemory>& mem)
 {
-    LOG_CAMERA_PREVIEW("%s :", __func__);
+    ssize_t offset;
+    sp<IMemoryHeap> heap = mem->getMemory(&offset, NULL);
+    struct addrs *addrs = (struct addrs *)((uint8_t *)heap->base() + offset);
 
-//    ssize_t offset; size_t size;
-//    sp<MemoryBase>       mem1 = mem;
-//    sp<MemoryHeapBase> heap = mem->getMemory(&offset, &size);
-//    sp<IMemoryHeap> heap = mem->getMemory(&offset, &size);
-
-//    mem1.clear();
-//    heap.clear();
+    mSecCamera->releaseRecordFrame(addrs->buf_index);
 }
 
 // ---------------------------------------------------------------------------
@@ -717,12 +739,20 @@
      * in CameraServices layer.
      */
     mFocusLock.lock();
-    mCondition.wait(mFocusLock);
-    mFocusLock.unlock();
-
     /* check early exit request */
-    if (mExitAutoFocusThread)
+    if (mExitAutoFocusThread) {
+        mFocusLock.unlock();
+        LOGV("%s : exiting on request0", __func__);
         return NO_ERROR;
+    }
+    mFocusCondition.wait(mFocusLock);
+    /* check early exit request */
+    if (mExitAutoFocusThread) {
+        mFocusLock.unlock();
+        LOGV("%s : exiting on request1", __func__);
+        return NO_ERROR;
+    }
+    mFocusLock.unlock();
 
     LOGV("%s : calling setAutoFocus", __func__);
     if (mSecCamera->setAutofocus() < 0) {
@@ -752,6 +782,7 @@
             mNotifyCb(CAMERA_MSG_FOCUS, false, 0, mCallbackCookie);
     }
 
+    LOGV("%s : exiting with no error", __func__);
     return NO_ERROR;
 }
 
@@ -759,7 +790,7 @@
 {
     LOGV("%s :", __func__);
     /* signal autoFocusThread to run once */
-    mCondition.signal();
+    mFocusCondition.signal();
     return NO_ERROR;
 }
 
@@ -983,14 +1014,6 @@
         LOG_CAMERA("getSnapshotAndJpeg interval: %lu us", LOG_TIME(1));
     }
 
-    /* the capture is done at this point so we can allow sensor commands
-     * again, we still need to do jpeg and thumbnail processing, but the
-     * sensor is available for something else
-     */
-    mStateLock.lock();
-    mCaptureInProgress = false;
-    mStateLock.unlock();
-
     int JpegImageSize, JpegExifSize;
     bool isLSISensor = false;
 
@@ -1046,21 +1069,14 @@
     ret = mOverlay->queueBuffer((void*)(static_cast<unsigned char *>(mPreviewHeap->base()) + offset +
                                 (mPostViewWidth*mPostViewHeight * 3 / 2)));
 
-    if (ret == ALL_BUFFERS_FLUSHED) {
-        goto PostviewOverlayEnd;
-    } else if (ret == -1) {
+    if (ret == -1) {
         LOGE("ERR(%s):overlay queueBuffer fail", __func__);
-        goto PostviewOverlayEnd;
-    }
-
-    overlay_buffer_t overlay_buffer;
-    ret = mOverlay->dequeueBuffer(&overlay_buffer);
-
-    if (ret == ALL_BUFFERS_FLUSHED) {
-        goto PostviewOverlayEnd;
-    } else if (ret == -1) {
-        LOGE("ERR(%s):overlay dequeueBuffer fail", __func__);
-        goto PostviewOverlayEnd;
+    } else if (ret != ALL_BUFFERS_FLUSHED) {
+        overlay_buffer_t overlay_buffer;
+        ret = mOverlay->dequeueBuffer(&overlay_buffer);
+        if (ret == -1) {
+            LOGE("ERR(%s):overlay dequeueBuffer fail", __func__);
+        }
     }
 
 PostviewOverlayEnd:
@@ -1076,7 +1092,8 @@
         LOGV("JpegExifSize=%d", JpegExifSize);
 
         if (JpegExifSize < 0) {
-            return UNKNOWN_ERROR;
+            ret = UNKNOWN_ERROR;
+            goto out;
         }
 
         unsigned char *ExifStart = (unsigned char *)JpegHeap->base() + 2;
@@ -1093,6 +1110,12 @@
     LOG_CAMERA("pictureThread interval: %lu us", LOG_TIME(0));
 
     LOGV("%s : pictureThread end", __func__);
+
+out:
+    mStateLock.lock();
+    mCaptureInProgress = false;
+    mStateLock.unlock();
+
     return ret;
 }
 
@@ -1121,9 +1144,6 @@
 {
     mPictureThread->requestExitAndWait();
 
-    mSecCamera->cancelPicture();
-
-    LOGW("%s : not supported, just returning NO_ERROR", __func__);
     return NO_ERROR;
 }
 
@@ -2038,6 +2058,14 @@
      * for ourself to exit, which is a deadlock.
      */
     if (mPreviewThread != NULL) {
+        /* this thread is normally already in it's threadLoop but blocked
+         * on the condition variable or running.  signal it so it wakes
+         * up and can exit.
+         */
+        mPreviewThread->requestExit();
+        mExitPreviewThread = true;
+        mPreviewRunning = true; /* let it run so it can exit */
+        mPreviewCondition.signal();
         mPreviewThread->requestExitAndWait();
         mPreviewThread.clear();
     }
@@ -2045,9 +2073,11 @@
         /* this thread is normally already in it's threadLoop but blocked
          * on the condition variable.  signal it so it wakes up and can exit.
          */
+        mFocusLock.lock();
         mAutoFocusThread->requestExit();
         mExitAutoFocusThread = true;
-        mCondition.signal();
+        mFocusCondition.signal();
+        mFocusLock.unlock();
         mAutoFocusThread->requestExitAndWait();
         mAutoFocusThread.clear();
     }
@@ -2061,8 +2091,11 @@
     if (mJpegHeap != NULL)
         mJpegHeap.clear();
 
-    if (mPreviewHeap != NULL)
+    if (mPreviewHeap != NULL) {
+        LOGI("%s: calling mPreviewHeap.dispose()", __func__);
+        mPreviewHeap->dispose();
         mPreviewHeap.clear();
+    }
 
     if (mRecordHeap != NULL)
         mRecordHeap.clear();
diff --git a/libcamera/SecCameraHWInterface.h b/libcamera/SecCameraHWInterface.h
index f416d9f..724d1bc 100644
--- a/libcamera/SecCameraHWInterface.h
+++ b/libcamera/SecCameraHWInterface.h
@@ -82,13 +82,12 @@
         PreviewThread(CameraHardwareSec *hw):
         Thread(false),
         mHardware(hw) { }
+        virtual void onFirstRef() {
+            run("CameraPreviewThread", PRIORITY_URGENT_DISPLAY);
+        }
         virtual bool threadLoop() {
-            int ret = mHardware->previewThread();
-            // loop until we need to quit
-            if(ret == NO_ERROR)
-                return true;
-            else
-                return false;
+            mHardware->previewThreadWrapper();
+            return false;
         }
     };
 
@@ -100,6 +99,7 @@
         mHardware(hw) { }
         virtual bool threadLoop() {
             mHardware->pictureThread();
+            mHardware->mSecCamera->endSnapshot();
             return false;
         }
     };
@@ -122,7 +122,7 @@
 
     sp<PreviewThread>   mPreviewThread;
             int         previewThread();
-            bool        mPreviewRunning;
+            int         previewThreadWrapper();
 
     sp<AutoFocusThread> mAutoFocusThread;
             int         autoFocusThread();
@@ -158,9 +158,16 @@
             void        setSkipFrame(int frame);
     /* used by auto focus thread to block until it's told to run */
     mutable Mutex       mFocusLock;
-    mutable Condition   mCondition;
+    mutable Condition   mFocusCondition;
             bool        mExitAutoFocusThread;
 
+    /* used by preview thread to block until it's told to run */
+    mutable Mutex       mPreviewLock;
+    mutable Condition   mPreviewCondition;
+    mutable Condition   mPreviewStoppedCondition;
+            bool        mPreviewRunning;
+            bool        mExitPreviewThread;
+
     /* used to guard threading state */
     mutable Mutex       mStateLock;