changeset 9609:1ce139389b3b

encoder: divide init/start and stop/delete to prevent race hazards Problems identified by helgrind, mostly theoretical hazards that are very unlikely to occur or to cause problems if they did.
author Steve Borho <steve@borho.org>
date Mon, 02 Mar 2015 19:54:42 -0600
parents e1b402399735
children e212eee15e98
files source/encoder/api.cpp source/encoder/encoder.cpp source/encoder/encoder.h source/encoder/frameencoder.cpp
diffstat 4 files changed, 36 insertions(+-), 20 deletions(-) [+]
line wrap: on
line diff
--- a/source/encoder/api.cpp	Mon Mar 02 19:24:06 2015 -0600
+++ b/source/encoder/api.cpp	Mon Mar 02 19:54:42 2015 -0600
@@ -173,6 +173,7 @@ void x265_encoder_close(x265_encoder *en
     {
         Encoder *encoder = static_cast<Encoder*>(enc);
 
+        encoder->stop();
         encoder->printSummary();
         encoder->destroy();
         delete encoder;
--- a/source/encoder/encoder.cpp	Mon Mar 02 19:24:06 2015 -0600
+++ b/source/encoder/encoder.cpp	Mon Mar 02 19:54:42 2015 -0600
@@ -246,8 +246,10 @@ void Encoder::create()
             x265_log(m_param, X265_LOG_ERROR, "Unable to initialize frame encoder, aborting\n");
             m_aborted = true;
         }
+    }
+    for (int i = 0; i < m_param->frameNumThreads; i++)
         m_frameEncoder[i]->start();
-    }
+
     if (m_param->bEmitHRDSEI)
         m_rateControl->initHRD(m_sps);
     if (!m_rateControl->init(m_sps))
@@ -276,6 +278,26 @@ void Encoder::create()
     m_encodeStartTime = x265_mdate();
 }
 
+void Encoder::stop()
+{
+    if (m_rateControl)
+        m_rateControl->terminate(); // unblock all blocked RC calls
+
+    if (m_lookahead)
+        m_lookahead->stop();
+    
+    for (int i = 0; i < m_param->frameNumThreads; i++)
+    {
+        if (m_frameEncoder[i])
+        {
+            m_frameEncoder[i]->getEncodedPicture(m_nalList);
+            m_frameEncoder[i]->m_threadActive = false;
+            m_frameEncoder[i]->m_enable.trigger();
+            m_frameEncoder[i]->stop();
+        }
+    }
+}
+
 void Encoder::destroy()
 {
     if (m_exportedPic)
@@ -284,21 +306,14 @@ void Encoder::destroy()
         m_exportedPic = NULL;
     }
 
-    if (m_rateControl)
-        m_rateControl->terminate(); // unblock all blocked RC calls
-
-    if (m_lookahead)
-        m_lookahead->stop();
-    
     for (int i = 0; i < m_param->frameNumThreads; i++)
-        if (m_frameEncoder[i]) m_frameEncoder[i]->getEncodedPicture(m_nalList);
-		
-    for (int i = 0; i < m_param->frameNumThreads; i++)
+    {
         if (m_frameEncoder[i])
         {
             m_frameEncoder[i]->destroy();
             delete m_frameEncoder[i];
         }
+    }
 
     // thread pools can be cleaned up now that all the JobProviders are
     // known to be shutdown
@@ -324,13 +339,18 @@ void Encoder::destroy()
 
     if (m_analysisFile)
         fclose(m_analysisFile);
-    free(m_param->analysisFileName);
-    free(m_param->csvfn);
     if (m_csvfpt)
         fclose(m_csvfpt);
-    free(m_param->rc.statFileName); // alloc'd by strdup
 
-    X265_FREE(m_param);
+    if (m_param)
+    {
+        free(m_param->rc.statFileName); // allocs by strdup
+        free(m_param->analysisFileName);
+        free(m_param->csvfn);
+        free(m_param->numaPools);
+
+        X265_FREE(m_param);
+    }
 }
 
 void Encoder::updateVbvPlan(RateControl* rc)
--- a/source/encoder/encoder.h	Mon Mar 02 19:24:06 2015 -0600
+++ b/source/encoder/encoder.h	Mon Mar 02 19:54:42 2015 -0600
@@ -136,6 +136,7 @@ public:
     ~Encoder() {}
 
     void create();
+    void stop();
     void destroy();
 
     int encode(const x265_picture* pic, x265_picture *pic_out);
--- a/source/encoder/frameencoder.cpp	Mon Mar 02 19:24:06 2015 -0600
+++ b/source/encoder/frameencoder.cpp	Mon Mar 02 19:54:42 2015 -0600
@@ -81,9 +81,6 @@ void FrameEncoder::destroy()
         delete m_tld;
     }
 
-    m_threadActive = false;
-    m_enable.trigger();
-
     delete[] m_rows;
     delete[] m_outStreams;
     X265_FREE(m_cuGeoms);
@@ -98,9 +95,6 @@ void FrameEncoder::destroy()
         delete m_rce.picTimingSEI;
         delete m_rce.hrdTiming;
     }
-
-    // wait for worker thread to exit
-    stop();
 }
 
 bool FrameEncoder::init(Encoder *top, int numRows, int numCols)