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 740305 - glimagesink: critical error while seeking using playback-test
glimagesink: critical error while seeking using playback-test
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
unspecified
Other Linux
: Normal minor
: 1.5.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-11-18 11:35 UTC by Vineeth
Modified: 2014-11-20 04:41 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
playback-test patch (1.24 KB, patch)
2014-11-19 05:38 UTC, Vineeth
rejected Details | Review
glimagesink patch (1.32 KB, patch)
2014-11-20 03:46 UTC, Vineeth
committed Details | Review

Description Vineeth 2014-11-18 11:35:41 UTC
When i use command ./playback-test 0 test.mp4 or any media file,

on seeking the media when it is in stop state, i get the below error.
This happens because context of glimagesink has been destroyed on stop, but still navigation events are being called and tries to use context.

** (lt-playback-test:29935): CRITICAL **: gst_gl_context_get_window: assertion 'GST_GL_IS_CONTEXT (context)' failed

Program received signal SIGTRAP, Trace/breakpoint trap.
0xb725efb9 in g_logv () from /lib/i386-linux-gnu/libglib-2.0.so.0
(gdb) backtrace
  • #0 g_logv
    from /lib/i386-linux-gnu/libglib-2.0.so.0
  • #1 g_log
    from /lib/i386-linux-gnu/libglib-2.0.so.0
  • #2 g_return_if_fail_warning
    from /lib/i386-linux-gnu/libglib-2.0.so.0
  • #3 gst_gl_context_get_window
    at gstglcontext.c line 784
  • #4 gst_glimage_sink_navigation_send_event
    at gstglimagesink.c line 201
  • #5 gst_navigation_send_event
    at navigation.c line 117
  • #6 gst_play_sink_navigation_send_event
    at gstplaysink.c line 5201
  • #7 gst_navigation_send_event
    at navigation.c line 117
  • #8 gst_play_bin_navigation_send_event
    at gstplaybin2.c line 5753
  • #9 gst_navigation_send_event
    at navigation.c line 117
  • #10 gst_navigation_send_mouse_event
    at navigation.c line 158
  • #11 motion_notify_cb
    at playback-test.c line 2193




This may be similar to https://bugzilla.gnome.org/show_bug.cgi?id=740292, but not exactly sure as the stack is different for both.
Comment 1 Vineeth 2014-11-19 05:38:32 UTC
Created attachment 290958 [details] [review]
playback-test patch

If we seek when media is in stop state, playback-test gives
critical error, since context of glimagesink is destroyed during stop.
Hence adding an extra condition in playback-test,
to pass on navigation events only when the playbackapp is live


One other way which i can think of is in by making sure 
gst_glimage_sink_navigation_send_event in glimagesink checks if context is valid before proceeding.
But this does not sound like a proper patch..


Please review and give comments on the same.
Comment 2 Sebastian Dröge (slomo) 2014-11-19 09:31:51 UTC
Comment on attachment 290958 [details] [review]
playback-test patch

The problem is in glimagesink / libgstgl. It should be possible to send navigation events at any time, and any problems should be caught in there.

Not sure why there is no context but the navigation event thread is running. That seems suspicious :)
Comment 3 Tim-Philipp Müller 2014-11-19 09:46:11 UTC
Also, the commit message talks about seeking in some state or another, but the patch is about sending navigation events. And the live-ness of a pipeline really has nothing to do with either.
Comment 4 Matthew Waters (ystreet00) 2014-11-19 11:00:41 UTC
Actually, this is from the playback-test explicitly sending the navigation event.  Now, if navigation events should be able to be sent in any state, then glimagesink needs to account for that in it's implementation and conditionalise the scaling that currently happens only when there's a context.
Comment 5 Tim-Philipp Müller 2014-11-19 11:10:16 UTC
'able to be sent in any state' just means that the app shouldn't have to worry about whether to send them or not, but the sink can just drop/ignore them if state < PAUSED.
Comment 6 Vineeth 2014-11-19 12:38:30 UTC
Hi
how much sense does it make to completely remove navigation_send_event from glimagesink and add the scaling of x and y pos in mouse_event_cb and key_event_cb

Since those key events are attached to th e window, whenever window gets destroyed these won't be called..

I have made some changes and verified and it doesn't seem different with these changes... but i need to verify more..


But i want some advice if this really any good  :)...
Comment 7 Matthew Waters (ystreet00) 2014-11-19 12:56:32 UTC
The navigation_send_event is still required as applications can explicitly inject navigation events using it which is what is happening in the playback-test example.  We would still need to scale these coordinates from window coordinates to stream coordinates in such a case as well.  The {mouse,key}_event_cb functions are called in response to some event from the windowing system.
Comment 8 Matthew Waters (ystreet00) 2014-11-19 12:58:29 UTC
Basically what you want to do is ignore any send_event when we don't have a context/window.  Which is what currently happens, just with the side effect of criticals.
Comment 9 Vineeth 2014-11-19 15:25:45 UTC
Just one last question 
generally critical errors need to be shown when there is really something wrong right.
since we all accept that the behaviour is proper, we might just want to return from the function without showing those critical warnings..
So this will just become a trivial patch to just work around the critical errors being displayed if i am right :)...

Critical error is so misleading at this place :)
Comment 10 Matthew Waters (ystreet00) 2014-11-19 22:34:45 UTC
Correct
Comment 11 Vineeth 2014-11-20 03:46:53 UTC
Created attachment 291053 [details] [review]
glimagesink patch

Added a condition to exit send_event instead of giving a critical error. Since critical error in this scenario is not proper.

Please review
Comment 12 Matthew Waters (ystreet00) 2014-11-20 04:40:49 UTC
commit 47f3a1954ed773b33208772cf9f224612a387beb
Author: Vineeth T M <vineeth.tm@samsung.com>
Date:   Thu Nov 20 09:13:58 2014 +0530

    glimagesink: critical error while seek playback-test(stop state)
    
    If we seek when media is in stop state, playback-test gives
    critical error, since context of glimagesink is destroyed during stop.
    But since context is not present, we need not handle send_event in glimagesink
    Hence adding a condition to check if context is valid.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=740305