Skip to content

appsink: Fix race condition on caps handling

Alicia Boya García requested to merge ntrrgc/gstreamer:appsink-race-condition into main

Background:

Whenever a caps event is received by appsink, the caps are stored in the same internal queue as buffers. Only when enough buffers have been popped from the queue to reach the caps, priv->sample gets its caps updated to match, so that they are correct for the following buffers.

Note that as far as upstream elements are concerned, the caps of appsink are updated immediately when the CAPS event is sent. Samples pulled from appsink retain the old caps until a later buffer -- one that was sent by upstream elements after the new caps -- is pulled.

The race condition:

When a flush is received, appsink clears the entire internal queue. The caps of priv->sample are not updated as part of this process, and instead remain as those of the sample that was last pulled by the user.

This leaves open a race condition where:

  1. Upstream sends a new caps event, and possibly some buffers for the new caps.
  2. Upstream sends a flush (possibly from a different thread).
  3. Upstream sends a new buffer for the new caps. Since as far as upstream is concerned, appsink caps are the new caps already, no new CAPS event is sent.
  4. The appsink user pulls a sample, having not pulled before enough samples to reach the buffers sent in step 1.

Bug: the pulled sample has the old caps instead of the new caps.

Fixing the race condition:

To avoid this problem, when a buffer is received after a flush, priv->sample's caps should be updated with the current caps before the buffer is added to the internal queue.

Interestingly, before this patch, appsink already had code for this, in gst_app_sink_render_common():

/* queue holding caps event might have been FLUSHed,
 * but caps state still present in pad caps */
if (G_UNLIKELY (!priv->last_caps &&
        gst_pad_has_current_caps (GST_BASE_SINK_PAD (psink)))) {
  priv->last_caps = gst_pad_get_current_caps (GST_BASE_SINK_PAD (psink));
  gst_sample_set_caps (priv->sample, priv->last_caps);
  GST_DEBUG_OBJECT (appsink, "activating pad caps %" GST_PTR_FORMAT,
      priv->last_caps);
}

This code assumes priv->last_caps is reset when a flush is received, which makes sense, but unfortunately, there was no code in the flush code path resetting it.

This patch adds such code, therefore fixing the race condition. A unit test demonstrating the bug and testing its behavior with the fix has also been added.

Merge request reports