[mythtv-commits] Ticket #8812: RingBuffer has a number of thread safety issues

[MythTV]

#8812: RingBuffer has a number of thread safety issues
------------------------------+---------------------------------------------
 Reporter:  danielk           |           Owner:  danielk   
     Type:  defect            |          Status:  closed    
 Priority:  minor             |       Milestone:  0.24      
Component:  MythTV - General  |         Version:  Trunk Head
 Severity:  medium            |      Resolution:  fixed     
 Keywords:                    |   Ticket locked:  0         
------------------------------+---------------------------------------------
Changes (by danielk):

  * status:  assigned => closed
  * resolution:  => fixed


Comment:

 (In [26101]) Fixes #8812. This primarily fixes a number of locking
 problems in the RingBuffer code.

 This also introduces 4 optimizations:
   1/ If a seek is to a position already contained in the read ahead
 buffer, we move the read position to that location and do not reset read
 ahead.
   2/ If there is a seek to within 300,000 bytes of the end of the file,
 read ahead is not reset and instead we enter a special state which makes
 reads directly. This prevents the read ahead thread from being cleared
 when in practice this is usually followed by a seek back to the beginning
 of the thread.
   3/ When the read ahead thread has no work to do it now waits for another
 thread to give it work instead of attempting to read repeatedly. This
 happens when the buffer is full, the end of file is seen, we've hit an
 irrecoverable error, etc.
   4/ The read ahead read size is now incresed in size by 50% whenever it's
 not keeping up with the reader instead of increasing in 32K blocks. This
 results in faster response when the bitrate estimation is bogis (ffmpeg
 problem) or absent. Shrinkage is still done slowly in 32K blocks.

 OpenFile and subsequently the RingBuffer ctor and ANN FileTransfer have
 also been changed to use a timeout rather than a retry count parameter.
 OpenFile was already using a timeout internally so that local and remote
 files would be treated similarly, but this had not been extended to the
 interface yet.

 MythPlayer's Peek code now retries it's peeks for a couple seconds if they
 fail due to a too small file initially. This has been a problem ever since
 the LiveTVChain was introduced.. but it became noticable with some ffmpeg
 changes and then went from being relatively rare to an everytime occurance
 with the faster RingBuffer. This fixes #8847.

 Further optimations are possible. For instance, opt #2 could just read in
 the remaining bytes into an aux buffer which would have no effect on local
 files but shave at least 100 ms of mythstreaming. Opt #2 could reserve a
 portion of the buffer for skip backs, while right now those are
 opportunistic. Opt #4 could also resize the read ahead buffer itself, not
 just the block size; easy to do if we used a vector for the buffer. The
 bitrate could be passed from the local ringbuffer to the remote one so it
 could better estimate it's read ahead requirements. We could fix the
 bitrate calculations in ffmpeg or perform our own.

 Note: There are two ANN FileTransfer calls outside of the C++ code, one in
 the python bindings and another in the perl bindings. These are were not
 using the retry param and so should work fine with the change to a
 timeout. If there were any I missed in my grep that you know of please
 check them.

 Note2: This ups both the binary version and protocol version, please
 distclean and upgrade all systems.

-- 
Ticket URL: <http://svn.mythtv.org/trac/ticket/8812#comment:5>
MythTV <http://www.mythtv.org/>
MythTV