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 752800 - basetransform: may return not-negotiation on shutdown
basetransform: may return not-negotiation on shutdown
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other Linux
: Normal major
: 1.5.90
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-07-23 22:16 UTC by Olivier Crête
Modified: 2015-08-16 13:39 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
basetransform: Return FLOW_FLUSHING if negotiation fails during shutdown (845 bytes, patch)
2015-07-23 22:16 UTC, Olivier Crête
committed Details | Review
avviddec: Ignore negotiation error on shutdown (838 bytes, patch)
2015-07-23 23:16 UTC, Olivier Crête
committed Details | Review

Description Olivier Crête 2015-07-23 22:16:19 UTC
Created attachment 308039 [details] [review]
basetransform: Return FLOW_FLUSHING if negotiation fails during shutdown

There is a race in basetransform, where, if setcaps is called from default_submit_input_buffer() while it is shutting down, the setcaps may fail because the srcpad has already been set to flushing, this will then "goto not_negotiated" and return flow-not-negotiated , instead of flow-flushing.

Possible (not so nice) patch is attached.

I can reproduce it quite reliably with validate.file.playback.change_state_intensive.raw_video_mkv:

GST_GL_XINITTHREADS=1 DISPLAY=:1 GST_VALIDATE_SCENARIO=change_state_intensive gst-validate-1.0  playbin uri=file:///home/ocrete/gst-validate/gst-integration-testsuites/medias/defaults/matroska/raw_video.mkv audio-sink='fakesink sync=true' video-sink='fakesink sync=true' --set-media-info "/home/ocrete/gst-validate/gst-integration-testsuites/medias/defaults/matroska/raw_video.mkv.media_info"
Comment 1 Tim-Philipp Müller 2015-07-23 22:33:03 UTC
Could swear the exact same thing was fixed some time ago already, maybe it was elsewhere.
Comment 2 Olivier Crête 2015-07-23 22:45:23 UTC
And I'm seeing the same thing in gst_ffmpegviddec_video_frame()
Comment 3 Olivier Crête 2015-07-23 23:16:16 UTC
Created attachment 308043 [details] [review]
avviddec: Ignore negotiation error on shutdown
Comment 4 Nicolas Dufresne (ndufresne) 2015-07-24 22:30:57 UTC
Review of attachment 308039 [details] [review]:

Yes, I think we fixed similar issues in many many places already, so I'd say make sense.
Comment 5 Thibault Saunier 2015-07-24 22:37:07 UTC
Indeed we did the exact same thing in many places already... and yes it is not so nice... Can't we think of a way of doing that in the pad generically? (just wondering here)
Comment 6 Nicolas Dufresne (ndufresne) 2015-07-24 23:16:00 UTC
I think the problem is that we have function returning gboolean, but running function that return FLOW_RETURN internally, and we loose the notion of FLUSHING. Fortunatly, all thread stops when that happen, and the pad is left with the flushing flag, that we check.

We could duplicate the API function that are affected and start using it ?
Comment 7 Olivier Crête 2015-07-27 18:00:18 UTC
Merged:

commit 5e5a14028a8605f957b5acf2c2170850d02a7b51
Author: Olivier Crête <olivier.crete@collabora.com>
Date:   Thu Jul 23 18:15:05 2015 -0400

    basetransform: Return FLOW_FLUSHING if negotiation fails during shutdown
    
    https://bugzilla.gnome.org/show_bug.cgi?id=752800


commit 6bfe79d903d8d855b16a8136d15b631aaf01de5b
Author: Olivier Crête <olivier.crete@collabora.com>
Date:   Thu Jul 23 19:15:43 2015 -0400

    avviddec: Ignore negotiation error on shutdown
    
    https://bugzilla.gnome.org/show_bug.cgi?id=752800