[x265] [PATCH] frameencoder: avoid race hazard in end of frame logic

[Steve Borho]

# HG changeset patch
# User Steve Borho <steve at borho.org>
# Date 1425510936 21600
#      Wed Mar 04 17:15:36 2015 -0600
# Node ID 7b5d44d7831ee389e8883f530bc1c02cd6678d00
# Parent  7605e562bef66bcb4b5f5c938ede4a7966bff7b7
frameencoder: avoid race hazard in end of frame logic

This bug has been in the encoder, latent, for about 6 months so it deserves an
obituary (being somewhat presumptuous that this commit actually kills it). The
bug worked in this way:

1. user starts an encode of a small resolution encode at superfast preset,
   generating frame rates of about 200-300 fps depending on the hardware.

2. a worker thread finishes compressing all of the CTUs in its row, but then
   gets evicted from its core before it is allowed to flush its row bitstream
   object (leaving four bytes within the Entropy object unwritten)

3. The other threads quickly finish the rest of the frame and the worker which
   compressed the last row signals frame completion.

4. The frame encoder thread concatenates the row bitstreams together and emits
   the NAL. If you were lucky, the row that was evicted would have been 4 bytes
   long since the 0 byte length triggers a check failure indicating something is
   seriously wrong. If you're un-lucky the NAL is garbage and does evil things
   to your decoder.

5. the evicted worker finally gets back on the core and flushes its bitstream,
   to the aid of no-one.

With this commit, we note when the last row has been compressed (indicating all
CTUs in the frame are compressed) but we do not signal that the frame is
complete until the last worker thread leaves processRow() (taking advantage of
the recently introduced atomic count of workers).

diff -r 7605e562bef6 -r 7b5d44d7831e source/encoder/frameencoder.cpp
--- a/source/encoder/frameencoder.cpp	Wed Mar 04 14:40:44 2015 -0600
+++ b/source/encoder/frameencoder.cpp	Wed Mar 04 17:15:36 2015 -0600
@@ -663,6 +663,7 @@
 {
     Slice* slice = m_frame->m_encData->m_slice;
 
+    m_bLastRowCompleted = false;
     m_bAllRowsStop = false;
     m_vbvResetTriggerRow = -1;
 
@@ -769,11 +770,15 @@
         if (realRow != m_numRows - 1)
             enqueueRowFilter(realRow + 1);
         else
-            m_completionEvent.trigger();
+            m_bLastRowCompleted = true;
     }
 
     if (ATOMIC_DEC(&m_activeWorkerCount) == 0)
+    {
+        if (m_bLastRowCompleted)
+            m_completionEvent.trigger();
         m_stallStartTime = x265_mdate();
+    }
 
     m_totalWorkerElapsedTime += x265_mdate() - startTime; // not thread safe, but good enough
 }
diff -r 7605e562bef6 -r 7b5d44d7831e source/encoder/frameencoder.h
--- a/source/encoder/frameencoder.h	Wed Mar 04 14:40:44 2015 -0600
+++ b/source/encoder/frameencoder.h	Wed Mar 04 17:15:36 2015 -0600
@@ -145,6 +145,7 @@
     uint32_t                 m_refLagRows;
 
     volatile bool            m_bAllRowsStop;
+    volatile bool            m_bLastRowCompleted;
     volatile int             m_vbvResetTriggerRow;
 
     CTURow*                  m_rows;