[Bug 715192] New: STREAM_LOCK is locked excessively during finish_frame() function of GstVideoDecoder

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=715192
  GStreamer | gst-plugins-base | git

           Summary: STREAM_LOCK is locked excessively during
                    finish_frame() function of GstVideoDecoder
    Classification: Platform
           Product: GStreamer
           Version: git
        OS/Version: Linux
            Status: UNCONFIRMED
          Severity: normal
          Priority: Normal
         Component: gst-plugins-base
        AssignedTo: gstreamer-bugs at lists.freedesktop.org
        ReportedBy: 4ernov at gmail.com
         QAContact: gstreamer-bugs at lists.freedesktop.org
     GNOME version: ---


Created an attachment (id=261479)
 View: https://bugzilla.gnome.org/attachment.cgi?id=261479
 Review: https://bugzilla.gnome.org/review?bug=715192&attachment=261479

Patch with release of STREAM_LOCK during gst_pad_push for GstVideoDecoder

STREAM_LOCK is locked too much inside finish_frame() function of
GstVideoDecoder as it remains locked during the whole period of gst_pad_push()
call. When there's no queue after decoder or this queue is full, the period of
time when STREAM_LOCK is actually locked by finish_frame() can be near to frame
duration as gst_pad_push() waits for frame to be displayed. That means that
upstream thread (i.e. handle_frame()) should wait for STREAM_LOCK been
unlocked, but given that it doesn't have corresponding wait condition there's
no way to wake up upstream thread and in most cases it simply misses its turn
to occupy STREAM_LOCK.

While writing GstVideoDecoder-based decoder we found that there's no harm to
unlock STREAM_LOCK before calling gst_pad_push() and lock it again afterwards.
It's possible for the following reasons:
1. In single-threaded decoders where finish_frame() is effectively called
inside handle_frame() there's no problem as a) STREAM_LOCK is still locked at
least once and b) the thread is blocked by gst_pad_push() naturally
2. In multi-threaded case synchronization requires more intelligent approach
than blocking input thread out of output thread. This model is often used when
decoder isn't capable to deliver every single frame during its duration but
could provide enough average framerate, so it would like to consume some input
frames even when output frame is being displayed. It surely requires some
overfill protection but it can be done using queues with mutexes and wait
conditions while it's too dangerous to use locked STREAM_LOCK during
gst_pad_push() for it.

We prepared a couple of patches for review, one for GstVideoDecoder and another
one for GstVideoEncoder. We release STREAM_LOCK only for gst_pad_push() as
gst_pad_push_event() is already called outside STREAM_LOCK so everything's fine
with them.

-- 
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.