Ticket #1871: 1871-v2.patch

libs/libmythtv/videobuffers.h

@@ -137 +137 @@
   private:
     frame_queue_t         *queue(BufferType type);
     const frame_queue_t   *queue(BufferType type) const;
+    VideoFrame            *GetNextFreeFrameInternal(
+        bool with_lock, bool allow_unsafe, BufferType enqueue_to);
 
     frame_queue_t          available, used, limbo, pause, displayed, decode;
     vbuffer_map_t          vbufferMap; // videobuffers to buffer's index
@@ -160 +162 @@
     uint                   vpos;
 
     mutable QMutex         global_lock;
-    QMutex                 inheritence_lock;
 
     bool                   use_frame_locks;
     QMutex                 frame_lock;

libs/libmythtv/videobuffers.cpp

@@ -84 +84 @@
     : numbuffers(0), needfreeframes(0), needprebufferframes(0),
       needprebufferframes_normal(0), needprebufferframes_small(0),
       keepprebufferframes(0), need_extra_for_pause(false), rpos(0), vpos(0),
-      global_lock(true), inheritence_lock(false), use_frame_locks(true),
+      global_lock(true), use_frame_locks(true),
       frame_lock(true)
 {
 }
@@ -123 +123 @@
                         uint needprebuffer_small, uint keepprebuffer,
                         bool enable_frame_locking)
 {
-    global_lock.lock();
+    QMutexLocker locker(&global_lock);
 
     Reset();
 
@@ -154 +154 @@
 
     for (uint i = 0; i < numdecode; i++)
         enqueue(kVideoBuffer_avail, at(i));
-    
-    global_lock.unlock();
 }
 
 /**
@@ -164 +162 @@
  */
 void VideoBuffers::Reset()
 {
-    global_lock.lock();
+    QMutexLocker locker(&global_lock);
 
     // Delete ffmpeg VideoFrames so we can create 
     // a different number of buffers below
@@ -185 +183 @@
     displayed.clear();
     parents.clear();
     children.clear();
-
     vbufferMap.clear();
-
-    global_lock.unlock();
 }
 
 /**
@@ -197 +192 @@
  */
 void VideoBuffers::SetPrebuffering(bool normal) 
 {
-    global_lock.lock();
-
+    QMutexLocker locker(&global_lock);
     needprebufferframes = (normal) ?
         needprebufferframes_normal : needprebufferframes_small;
-
-    global_lock.unlock();
 };
 
+VideoFrame *VideoBuffers::GetNextFreeFrameInternal(
+    bool with_lock, bool allow_unsafe, BufferType enqueue_to)
+{
+    QMutexLocker locker(&global_lock);
+    VideoFrame *frame = available.dequeue();
+
+    // Try to get a frame not being used by the decoder
+    for (uint i = 0; i <= available.size() && decode.contains(frame); i++)
+    {
+        if (!available.contains(frame))
+            available.enqueue(frame);
+        frame = available.dequeue();
+    }
+
+    while (frame && used.contains(frame))
+    {
+        VERBOSE(VB_IMPORTANT,
+                QString("GetNextFreeFrame() served a busy frame %1. "
+                        "Dropping. %2")
+                .arg(DebugString(frame, true)).arg(GetStatus()));
+        frame = available.dequeue();
+    }
+
+    // only way this should be triggered if we're in unsafe mode
+    if (!frame && allow_unsafe)
+    {
+        VERBOSE(VB_PLAYBACK,
+                QString("GetNextFreeFrame() is getting a busy frame %1. "
+                        "      %2")
+                .arg(DebugString(frame, true)).arg(GetStatus()));
+        frame = used.dequeue();
+        if (EnoughFreeFrames())
+            available_wait.wakeAll();
+    }
+
+    if (frame)
+    {
+        safeEnqueue(enqueue_to, frame);
+
+        bool success = true;
+        if (with_lock)
+            success = TryLockFrame(frame, "GetNextFreeFrame");
+
+        if (!success)
+        {
+            safeEnqueue(kVideoBuffer_avail, frame);
+            VERBOSE(VB_IMPORTANT,
+                QString("GetNextFreeFrame() unable to lock frame %1. "
+                        "Dropping %2. w/lock(%3) unsafe(%4)")
+                    .arg(DebugString(frame)).arg(GetStatus())
+                    .arg(with_lock).arg(allow_unsafe));
+            DiscardFrame(frame);
+            frame = NULL;
+        }
+    }
+
+    return frame;
+}
+
 /**
  * \fn VideoBuffers::GetNextFreeFrame(bool,bool,BufferType)
  *  Gets a frame from available buffers list.
@@ -219 +270 @@
                                            bool allow_unsafe,
                                            BufferType enqueue_to)
 {
-    VideoFrame *frame = NULL;
-    uint tries = 0;
-    bool success = false;
-    while (!success)
+    for (uint tries = 1; true; tries++)
     {
-        success = false;
-        global_lock.lock();
-        frame = available.dequeue();
+        VideoFrame *frame = VideoBuffers::GetNextFreeFrameInternal(
+            with_lock, allow_unsafe, enqueue_to);
 
-        // Try to get a frame not being used by the decoder
-        for (uint i = 0; i <= available.size() && decode.contains(frame); i++)
-        {
-            if (!available.contains(frame))
-                available.enqueue(frame);
-            frame = available.dequeue();
-        }
+        if (frame)
+            return frame;
 
-        while (frame && used.contains(frame))
+        if (tries >= TRY_LOCK_SPINS)
         {
-            VERBOSE(VB_IMPORTANT,
-                    QString("GetNextFreeFrame() served a busy frame %1. "
-                            "Dropping. %2")
-                    .arg(DebugString(frame, true)).arg(GetStatus()));
-            frame = available.dequeue();
+            VERBOSE(VB_IMPORTANT, 
+                    QString("GetNextFreeFrame() unable to "
+                            "lock frame %1 times. Discarding Frames.")
+                    .arg(TRY_LOCK_SPINS));
+            DiscardFrames(true);
+            continue;
         }
 
-        // only way this should be triggered if we're in unsafe mode
-        if (!frame && allow_unsafe)
+        if (tries && !(tries % TRY_LOCK_SPINS_BEFORE_WARNING))
         {
             VERBOSE(VB_PLAYBACK,
-                    QString("GetNextFreeFrame() is getting a busy frame %1. "
-                            "      %2")
-                    .arg(DebugString(frame, true)).arg(GetStatus()));
-            frame = used.dequeue();
-            if (EnoughFreeFrames())
-                available_wait.wakeAll();
+                    QString("GetNextFreeFrame() TryLock has "
+                            "spun %1 times, this is a lot.").arg(tries));
         }
-        if (frame)
-        {
-            safeEnqueue(enqueue_to, frame);
-
-            success = true;
-            if (with_lock)
-                success = TryLockFrame(frame, "GetNextFreeFrame");
-            if (!success)
-                safeEnqueue(kVideoBuffer_avail, frame);
-        }
-
-        global_lock.unlock();
-
-        if (!success)
-        {
-            if (frame)
-            {
-                VERBOSE(VB_IMPORTANT,
-                        QString("GetNextFreeFrame() unable to lock frame %1. "
-                                "Dropping %2. w/lock(%3) unsafe(%4)")
-                        .arg(DebugString(frame)).arg(GetStatus())
-                        .arg(with_lock).arg(allow_unsafe));
-                DiscardFrame(frame);
-            }
-            ++tries;
-            if (tries<TRY_LOCK_SPINS)
-            {
-                if (tries && !(tries % TRY_LOCK_SPINS_BEFORE_WARNING))
-                    VERBOSE(VB_PLAYBACK, "GetNextFreeFrame() TryLock has "
-                            "spun "<<tries<<" times, this is a lot.");
-                usleep(TRY_LOCK_SPIN_WAIT);
-            }
-            else
-            {
-                VERBOSE(VB_IMPORTANT, 
-                        QString("GetNextFreeFrame() unable to "
-                        "lock frame %1 times. Discarding Frames.")
-                        .arg(TRY_LOCK_SPINS));
-                DiscardFrames(true);
-            }
-        }
+        usleep(TRY_LOCK_SPIN_WAIT);
     }
 
-    return frame;
+    return NULL;
 }
 
 /**
@@ -310 +308 @@
  */
 void VideoBuffers::ReleaseFrame(VideoFrame *frame)
 {
-    global_lock.lock();
+    QMutexLocker locker(&global_lock);
+
     vpos = vbufferMap[frame];
     limbo.remove(frame);
     decode.enqueue(frame);
     used.enqueue(frame);
-    global_lock.unlock();
 }
 
 /**
@@ -325 +323 @@
  */
 void VideoBuffers::DeLimboFrame(VideoFrame *frame)
 {
-    global_lock.lock();
+    QMutexLocker locker(&global_lock);
     if (limbo.contains(frame))
     {
         limbo.remove(frame);
         available.enqueue(frame);
     }
     decode.remove(frame);
-    global_lock.unlock();
 }
 
 /**
@@ -341 +338 @@
  */
 void VideoBuffers::StartDisplayingFrame(void)
 {
-    global_lock.lock();
+    QMutexLocker locker(&global_lock);
     rpos = vbufferMap[used.head()];
-    global_lock.unlock();
 }
 
 /**
@@ -352 +348 @@
  */
 void VideoBuffers::DoneDisplayingFrame(void)
 {
-    global_lock.lock();
+    QMutexLocker locker(&global_lock);
 
     VideoFrame *buf = used.dequeue();
     if (buf)
@@ -361 +357 @@
         if (EnoughFreeFrames())
             available_wait.wakeAll();
     }
-
-    global_lock.unlock();
 }
 
 /**
@@ -372 +366 @@
  */
 void VideoBuffers::DiscardFrame(VideoFrame *frame)
 {
-    global_lock.lock();
+    QMutexLocker locker(&global_lock);
+
     bool ok = TryLockFrame(frame, "DiscardFrame A");
     for (uint i=0; i<5 && !ok; i++)
     {
@@ -393 +388 @@
                                       "Unable to obtain lock on %1, %2")
                 .arg(DebugString(frame, true)).arg(GetStatus()));
     }
-    global_lock.unlock();
 }
 
 frame_queue_t *VideoBuffers::queue(BufferType type)
 {
+    QMutexLocker locker(&global_lock);
+
     frame_queue_t *q = NULL;
 
-    global_lock.lock();
     if (type == kVideoBuffer_avail)
         q = &available;
     else if (type == kVideoBuffer_used)
@@ -413 +408 @@
         q = &pause;
     else if (type == kVideoBuffer_decode)
         q = &decode;
-    global_lock.unlock();
 
     return q;
 }
 
 const frame_queue_t *VideoBuffers::queue(BufferType type) const
 {
+    QMutexLocker locker(&global_lock);
+
     const frame_queue_t *q = NULL;
 
-    global_lock.lock();
     if (type == kVideoBuffer_avail)
         q = &available;
     else if (type == kVideoBuffer_used)
@@ -435 +430 @@
         q = &pause;
     else if (type == kVideoBuffer_decode)
         q = &decode;
-    global_lock.unlock();
 
     return q;
 }
 
 VideoFrame *VideoBuffers::dequeue(BufferType type)
 {
+    QMutexLocker locker(&global_lock);
+
     frame_queue_t *q = queue(type);
-    if (q)
-    {
-        global_lock.lock();
-        VideoFrame *frame = q->dequeue();
-        global_lock.unlock();
-        return frame;
-    }
-    return NULL;
+
+    if (!q)
+        return NULL;
+
+    return q->dequeue();
 }
 
 VideoFrame *VideoBuffers::head(BufferType type)
 {
+    QMutexLocker locker(&global_lock);
+
     frame_queue_t *q = queue(type);
-    if (q)
-    {
-        QMutexLocker locker(&global_lock);
-        if (q->size())
-            return q->head();
-    }
+
+    if (!q)
+        return NULL;
+
+    if (q->size())
+        return q->head();
+
     return NULL;
 }
 
 VideoFrame *VideoBuffers::tail(BufferType type)
 {
+    QMutexLocker locker(&global_lock);
+
     frame_queue_t *q = queue(type);
-    if (q)
-    {
-        QMutexLocker locker(&global_lock);
-        if (q->size())
-            return q->tail();
-    }
+
+    if (!q)
+        return NULL;
+
+    if (q->size())
+        return q->tail();
+
     return NULL;
 }
 
 void VideoBuffers::enqueue(BufferType type, VideoFrame *frame)
 {
-    if (frame)
-    {
-        frame_queue_t *q = queue(type);
-        if (q)
-        {
-            global_lock.lock();
-            q->remove(frame);
-            q->enqueue(frame);
-            global_lock.unlock();
-            if (q == &available && EnoughFreeFrames())
-                available_wait.wakeAll();
-        }
-    }
+    if (!frame)
+        return;
+
+    frame_queue_t *q = queue(type);
+    if (!q)
+        return;
+
+    global_lock.lock();
+    q->remove(frame);
+    q->enqueue(frame);
+    global_lock.unlock();
+
+    if (q == &available && EnoughFreeFrames())
+        available_wait.wakeAll();
+
+    return;
 }
 
 void VideoBuffers::remove(BufferType type, VideoFrame *frame)
 {
-    if (frame)
-    {
-        global_lock.lock();
-        if ((type & kVideoBuffer_avail) == kVideoBuffer_avail)
-            available.remove(frame);
-        if ((type & kVideoBuffer_used) == kVideoBuffer_used)
-            used.remove(frame);
-        if ((type & kVideoBuffer_displayed) == kVideoBuffer_displayed)
-            displayed.remove(frame);
-        if ((type & kVideoBuffer_limbo) == kVideoBuffer_limbo)
-            limbo.remove(frame);
-        if ((type & kVideoBuffer_pause) == kVideoBuffer_pause)
-            pause.remove(frame);
-        if ((type & kVideoBuffer_decode) == kVideoBuffer_decode)
-            decode.remove(frame);
-        global_lock.unlock();
-    }
+    if (!frame)
+        return;
+
+    QMutexLocker locker(&global_lock);
+
+    if ((type & kVideoBuffer_avail) == kVideoBuffer_avail)
+        available.remove(frame);
+    if ((type & kVideoBuffer_used) == kVideoBuffer_used)
+        used.remove(frame);
+    if ((type & kVideoBuffer_displayed) == kVideoBuffer_displayed)
+        displayed.remove(frame);
+    if ((type & kVideoBuffer_limbo) == kVideoBuffer_limbo)
+        limbo.remove(frame);
+    if ((type & kVideoBuffer_pause) == kVideoBuffer_pause)
+        pause.remove(frame);
+    if ((type & kVideoBuffer_decode) == kVideoBuffer_decode)
+        decode.remove(frame);
 }
 
 void VideoBuffers::requeue(BufferType dst, BufferType src, int num)
 {
-    global_lock.lock();
+    QMutexLocker locker(&global_lock);
+
     num = (num <= 0) ? size(src) : num;
     for (uint i=0; i<(uint)num; i++)
     {
@@ -525 +528 @@
         if (frame)
             enqueue(dst, frame);
     }
-    global_lock.unlock();
 }
 
 void VideoBuffers::safeEnqueue(BufferType dst, VideoFrame* frame)
 {
-    if (frame)
-    {
-        global_lock.lock();
-        remove(kVideoBuffer_all, frame);
-        enqueue(dst, frame);
-        global_lock.unlock();
-    }
+    if (!frame)
+        return;
+
+    QMutexLocker locker(&global_lock);
+
+    remove(kVideoBuffer_all, frame);
+    enqueue(dst, frame);
 }
 
 frame_queue_t::iterator VideoBuffers::begin_lock(BufferType type)
@@ -551 +553 @@
 
 frame_queue_t::iterator VideoBuffers::end(BufferType type)
 {
-    global_lock.lock();
+    QMutexLocker locker(&global_lock);
+
     frame_queue_t::iterator it;
     frame_queue_t *q = queue(type);
     if (q)
         it = q->end();
     else
         it = available.end();
-    global_lock.unlock();
+
     return it;
 }
 
 uint VideoBuffers::size(BufferType type) const
 {
-    uint s = 0;
+    QMutexLocker locker(&global_lock);
+
     const frame_queue_t *q = queue(type);
     if (q)
-    {
-        global_lock.lock();
-        s = q->size();
-        global_lock.unlock();
-    }
-    return s;
+        return q->size();
+
+    return 0;
 }
 
 bool VideoBuffers::contains(BufferType type, VideoFrame *frame) const
 {
-    bool c = false;
+    QMutexLocker locker(&global_lock);
+
     const frame_queue_t *q = queue(type);
     if (q)
-    {
-        global_lock.lock();
-        c = q->contains(frame);
-        global_lock.unlock();
-    }
-    return c;
+        return q->contains(frame);
+
+    return false;
 }
 
 VideoFrame *VideoBuffers::GetScratchFrame(void)
@@ -595 +594 @@
         VERBOSE(VB_IMPORTANT,
                 "GetScratchFrame() called, but not allocated");
     }
+
+    QMutexLocker locker(&global_lock);
     return at(allocSize()-1);
 }
 
@@ -605 +606 @@
         VERBOSE(VB_IMPORTANT,
                 "GetScratchFrame() called, but not allocated");
     }
+
+    QMutexLocker locker(&global_lock);
     return at(allocSize()-1);
 }
 
@@ -692 +695 @@
 
 void VideoBuffers::ClearAfterSeek(void)
 {
-    global_lock.lock();
+    {
+        QMutexLocker locker(&global_lock);
 
-    for (uint i = 0; i < size(); i++)
-        at(i)->timecode = 0;
+        for (uint i = 0; i < size(); i++)
+            at(i)->timecode = 0;
 
-    while (used.count() > 1)
-    {
-        VideoFrame *buffer = used.dequeue();
-        available.enqueue(buffer);
-    }
+        while (used.count() > 1)
+        {
+            VideoFrame *buffer = used.dequeue();
+            available.enqueue(buffer);
+        }
 
-    if (used.count() > 0)
-    {
-        VideoFrame *buffer = used.dequeue();
-        available.enqueue(buffer);
-        vpos = vbufferMap[buffer];
-        rpos = vpos;
+        if (used.count() > 0)
+        {
+            VideoFrame *buffer = used.dequeue();
+            available.enqueue(buffer);
+            vpos = vbufferMap[buffer];
+            rpos = vpos;
+        }
+        else
+        {
+            vpos = rpos = 0;
+        }
     }
-    else
-    {
-        vpos = rpos = 0;
-    }
 
     if (EnoughFreeFrames())
         available_wait.wakeAll();
 
-    global_lock.unlock();
 }
 
 void VideoBuffers::LockFrame(const VideoFrame *frame, const char* owner)
@@ -865 +869 @@
 {
     (void)frame;
 #ifdef USING_XVMC
-    inheritence_lock.lock();
+    QMutexLocker locker(&global_lock);
     
     frame_map_t::iterator it = parents.find(frame);
     if (it == parents.end())
@@ -909 +913 @@
         for (; it != new_parents.end(); ++it)
             children[*it].push_back((VideoFrame*)frame);
     }
-
-    inheritence_lock.unlock();
 #endif // USING_XVMC
 }
 
 void VideoBuffers::RemoveInheritence(const VideoFrame *frame)
 {
-    inheritence_lock.lock();
+    QMutexLocker locker(&global_lock);
 
     frame_map_t::iterator it = parents.find(frame);
     if (it == parents.end())
-    {
-        inheritence_lock.unlock();
         return;
-    }
 
     frame_queue_t new_parents;
     frame_queue_t &p = it->second;
@@ -952 +951 @@
             VERBOSE(VB_IMPORTANT, QString("Parent #%1: %2")
                     .arg(i).arg(DebugString(*pit)));
     }
-
-    inheritence_lock.unlock();
 }
 
 frame_queue_t VideoBuffers::Children(const VideoFrame *frame)
 {
+    QMutexLocker locker(&global_lock);
+
     frame_queue_t c;
     frame_map_t::iterator it = children.find(frame);
     if (it != children.end())
@@ -967 +966 @@
 
 bool VideoBuffers::HasChildren(const VideoFrame *frame)
 {
+    QMutexLocker locker(&global_lock);
+
     frame_map_t::iterator it = children.find(frame);
     if (it != children.end())
         return !(it->second.empty());
@@ -1033 +1034 @@
 
 void VideoBuffers::SetOSDFrame(VideoFrame *frame, VideoFrame *osd)
 {
-    if (frame==osd)
+    if (frame == osd)
     {
         VERBOSE(VB_IMPORTANT, QString("SetOSDFrame() -- frame==osd %1")
                 .arg(GetStatus()));
@@ -1044 +1045 @@
     xvmc_render_state_t* r = GetRender(frame);
     if (r)
     {
-        global_lock.lock();
+        QMutexLocker locker(&global_lock);
 
         VideoFrame* old_osd = (VideoFrame*) r->p_osd_target_surface_render;
         if (old_osd)
@@ -1054 +1055 @@
 
         if (osd)
             xvmc_osd_parent[osd] = frame;
-
-        global_lock.unlock();
     }
+
     UnlockFrame(frame, "SetOSDFrame");
 }
 
 VideoFrame* VideoBuffers::GetOSDParent(const VideoFrame *osd)
 {
-    global_lock.lock();
-    VideoFrame *parent = xvmc_osd_parent[osd];
-    global_lock.unlock();
-    return parent;
+    QMutexLocker locker(&global_lock);
+    return xvmc_osd_parent[osd];
 }
 
 #endif