After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 655279 - [playbin2] Don't reset sinks when not needed
[playbin2] Don't reset sinks when not needed
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
git master
Other Linux
: Normal normal
: 0.10.36
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2011-07-25 16:43 UTC by Edward Hervey
Modified: 2011-07-26 09:40 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
playbin2: Avoid resetting playsink when not needed (2.48 KB, patch)
2011-07-25 16:44 UTC, Edward Hervey
committed Details | Review

Description Edward Hervey 2011-07-25 16:43:46 UTC
When playing back chained multi-streams, we end up triggering playbin2's no_more_pads_cb during the playback of one file.

When no non-NULL audio/video/text sinks are specified, that code will end up resetting playsink's internal sinks to NULL (instead of the previously auto-selected sinks).

This will cause the previously configured sinks to be set to NULL, resulting in any stream currently pushing in those sinks to return with GST_FLOW_WRONG_STATE... eventually making the upstream elements stop without any warning.
Comment 1 Edward Hervey 2011-07-25 16:44:35 UTC
Created attachment 192622 [details] [review]
playbin2: Avoid resetting playsink when not needed

When we don't have specific {audio|video|text}-sink properties, don't
set them on playsink when reconfiguring.
If we do that, we end up setting the previous configured sink to
GST_STATE_NULL resulting in any potentially pending push being returned
with GST_FLOW_WRONG_STATE which will cause the upstream elements to
silently stop.
Comment 2 Sebastian Dröge (slomo) 2011-07-26 07:15:21 UTC
And we should probably only reset the custom sinks if they changed since the last time. Otherwise there should be the same problems if I'm not missing anything.

Other than that this makes sense, yes.
Comment 3 Edward Hervey 2011-07-26 08:14:26 UTC
(In reply to comment #2)
> And we should probably only reset the custom sinks if they changed since the
> last time. Otherwise there should be the same problems if I'm not missing
> anything.

playsink automatically figures that out:
=== (gst_play_sink_set_sink)
  if (elem) {
    old = *elem;
    if (sink)
      gst_object_ref (sink);
    *elem = sink;
  }
  GST_PLAY_SINK_UNLOCK (playsink);

  if (old) {
    if (old != sink)
      gst_element_set_state (old, GST_STATE_NULL);
    gst_object_unref (old);
  }
===
> 
> Other than that this makes sense, yes.
Comment 4 Edward Hervey 2011-07-26 09:40:01 UTC
commit 059db89633151248f1612edb64de0c9d6208e4b1
Author: Edward Hervey <edward.hervey@collabora.co.uk>
Date:   Mon Jul 25 18:37:15 2011 +0200

    playbin2: Avoid resetting playsink when not needed
    
    When we don't have specific {audio|video|text}-sink properties, don't
    set them on playsink when reconfiguring.
    If we do that, we end up setting the previous configured sink to
    GST_STATE_NULL resulting in any potentially pending push being returned
    with GST_FLOW_WRONG_STATE which will cause the upstream elements to
    silently stop.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=655279