shithub: libvpx

Download patch

ref: b4e0986391819a33c71795b2dd76710104761b52
parent: 1e2cfa3f0f75c971ccceb9be0e818b207c86303b
author: Venkatarama NG. Avadhani <[email protected]>
date: Mon Nov 25 11:15:52 EST 2019 

Fix mutex free in multi-thread lpf

The mutex lf_mutex will now be allocated and destroyed, making it easier
to verify if it has been inited before destruction.

BUG=webm:1662

Change-Id: I8169bea9e117bd615d68b8d02da98aeab570b53f


--- a/vp9/common/vp9_thread_common.c
+++ b/vp9/common/vp9_thread_common.c
@@ -298,8 +298,11 @@
         pthread_cond_init(&lf_sync->cond[i], NULL);
       }
     }
-    pthread_mutex_init(&lf_sync->lf_mutex, NULL);
 
+    CHECK_MEM_ERROR(cm, lf_sync->lf_mutex,
+                    vpx_malloc(sizeof(*lf_sync->lf_mutex)));
+    pthread_mutex_init(lf_sync->lf_mutex, NULL);
+
     CHECK_MEM_ERROR(cm, lf_sync->recon_done_mutex,
                     vpx_malloc(sizeof(*lf_sync->recon_done_mutex) * rows));
     if (lf_sync->recon_done_mutex) {
@@ -339,47 +342,50 @@
 
 // Deallocate lf synchronization related mutex and data
 void vp9_loop_filter_dealloc(VP9LfSync *lf_sync) {
-  if (lf_sync != NULL) {
+  assert(lf_sync != NULL);
+
 #if CONFIG_MULTITHREAD
+  if (lf_sync->mutex != NULL) {
     int i;
-
-    if (lf_sync->mutex != NULL) {
-      for (i = 0; i < lf_sync->rows; ++i) {
-        pthread_mutex_destroy(&lf_sync->mutex[i]);
-      }
-      vpx_free(lf_sync->mutex);
+    for (i = 0; i < lf_sync->rows; ++i) {
+      pthread_mutex_destroy(&lf_sync->mutex[i]);
     }
-    if (lf_sync->cond != NULL) {
-      for (i = 0; i < lf_sync->rows; ++i) {
-        pthread_cond_destroy(&lf_sync->cond[i]);
-      }
-      vpx_free(lf_sync->cond);
+    vpx_free(lf_sync->mutex);
+  }
+  if (lf_sync->cond != NULL) {
+    int i;
+    for (i = 0; i < lf_sync->rows; ++i) {
+      pthread_cond_destroy(&lf_sync->cond[i]);
     }
-    if (lf_sync->recon_done_mutex != NULL) {
-      int i;
-      for (i = 0; i < lf_sync->rows; ++i) {
-        pthread_mutex_destroy(&lf_sync->recon_done_mutex[i]);
-      }
-      vpx_free(lf_sync->recon_done_mutex);
+    vpx_free(lf_sync->cond);
+  }
+  if (lf_sync->recon_done_mutex != NULL) {
+    int i;
+    for (i = 0; i < lf_sync->rows; ++i) {
+      pthread_mutex_destroy(&lf_sync->recon_done_mutex[i]);
     }
+    vpx_free(lf_sync->recon_done_mutex);
+  }
 
-    pthread_mutex_destroy(&lf_sync->lf_mutex);
-    if (lf_sync->recon_done_cond != NULL) {
-      int i;
-      for (i = 0; i < lf_sync->rows; ++i) {
-        pthread_cond_destroy(&lf_sync->recon_done_cond[i]);
-      }
-      vpx_free(lf_sync->recon_done_cond);
+  if (lf_sync->lf_mutex != NULL) {
+    pthread_mutex_destroy(lf_sync->lf_mutex);
+    vpx_free(lf_sync->lf_mutex);
+  }
+  if (lf_sync->recon_done_cond != NULL) {
+    int i;
+    for (i = 0; i < lf_sync->rows; ++i) {
+      pthread_cond_destroy(&lf_sync->recon_done_cond[i]);
     }
+    vpx_free(lf_sync->recon_done_cond);
+  }
 #endif  // CONFIG_MULTITHREAD
 
-    vpx_free(lf_sync->lfdata);
-    vpx_free(lf_sync->cur_sb_col);
-    vpx_free(lf_sync->num_tiles_done);
-    // clear the structure as the source of this call may be a resize in which
-    // case this call will be followed by an _alloc() which may fail.
-    vp9_zero(*lf_sync);
-  }
+  vpx_free(lf_sync->lfdata);
+  vpx_free(lf_sync->cur_sb_col);
+  vpx_free(lf_sync->num_tiles_done);
+  // clear the structure as the source of this call may be a resize in which
+  // case this call will be followed by an _alloc() which may fail.
+  vp9_zero(*lf_sync);
 }
 
 static int get_next_row(VP9_COMMON *cm, VP9LfSync *lf_sync) {
@@ -390,7 +396,7 @@
 #if CONFIG_MULTITHREAD
   const int tile_cols = 1 << cm->log2_tile_cols;
 
-  pthread_mutex_lock(&lf_sync->lf_mutex);
+  pthread_mutex_lock(lf_sync->lf_mutex);
   if (cm->lf_row < max_rows) {
     cur_row = cm->lf_row >> MI_BLOCK_SIZE_LOG2;
     return_val = cm->lf_row;
@@ -401,7 +407,7 @@
       cur_row += 1;
     }
   }
-  pthread_mutex_unlock(&lf_sync->lf_mutex);
+  pthread_mutex_unlock(lf_sync->lf_mutex);
 
   if (return_val == -1) return return_val;
 
@@ -411,7 +417,7 @@
                       &lf_sync->recon_done_mutex[cur_row]);
   }
   pthread_mutex_unlock(&lf_sync->recon_done_mutex[cur_row]);
-  pthread_mutex_lock(&lf_sync->lf_mutex);
+  pthread_mutex_lock(lf_sync->lf_mutex);
   if (lf_sync->corrupted) {
     int row = return_val >> MI_BLOCK_SIZE_LOG2;
     pthread_mutex_lock(&lf_sync->mutex[row]);
@@ -420,7 +426,7 @@
     pthread_mutex_unlock(&lf_sync->mutex[row]);
     return_val = -1;
   }
-  pthread_mutex_unlock(&lf_sync->lf_mutex);
+  pthread_mutex_unlock(lf_sync->lf_mutex);
 #else
   (void)lf_sync;
   if (cm->lf_row < max_rows) {
@@ -455,9 +461,9 @@
 void vp9_set_row(VP9LfSync *lf_sync, int num_tiles, int row, int is_last_row,
                  int corrupted) {
 #if CONFIG_MULTITHREAD
-  pthread_mutex_lock(&lf_sync->lf_mutex);
+  pthread_mutex_lock(lf_sync->lf_mutex);
   lf_sync->corrupted |= corrupted;
-  pthread_mutex_unlock(&lf_sync->lf_mutex);
+  pthread_mutex_unlock(lf_sync->lf_mutex);
   pthread_mutex_lock(&lf_sync->recon_done_mutex[row]);
   lf_sync->num_tiles_done[row] += 1;
   if (num_tiles == lf_sync->num_tiles_done[row]) {
--- a/vp9/common/vp9_thread_common.h
+++ b/vp9/common/vp9_thread_common.h
@@ -40,7 +40,7 @@
   int num_active_workers;  // number of scheduled workers.
 
 #if CONFIG_MULTITHREAD
-  pthread_mutex_t lf_mutex;
+  pthread_mutex_t *lf_mutex;
   pthread_mutex_t *recon_done_mutex;
   pthread_cond_t *recon_done_cond;
 #endif
--- a/vp9/decoder/vp9_decodeframe.c
+++ b/vp9/decoder/vp9_decodeframe.c
@@ -1733,9 +1733,9 @@
   int return_val = 0;
 #if CONFIG_MULTITHREAD
   int corrupted;
-  pthread_mutex_lock(&lf_sync->lf_mutex);
+  pthread_mutex_lock(lf_sync->lf_mutex);
   corrupted = lf_sync->corrupted;
-  pthread_mutex_unlock(&lf_sync->lf_mutex);
+  pthread_mutex_unlock(lf_sync->lf_mutex);
   if (!corrupted) {
     pthread_mutex_lock(&lf_sync->recon_done_mutex[row]);
     lf_sync->num_tiles_done[row] += 1;