Source/WebCore/manual-tests/video-rtsp.html loads but no video is displayed and I hear sound-cracks, the rtsp:// link plays fine with gst-launch. Needs some further investigation
Created attachment 88462 [details] proposed patch
Attachment 88462 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style']" exit_code: 1 Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h:54: The parameter name "player" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h:55: The parameter name "player" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 2 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 88592 [details] proposed patch
Comment on attachment 88592 [details] proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=88592&action=review Looks good, though please make callback static, unless there is a reason they should be exposed in the header. > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:215 > +void mediaPlayerPrivateVideoChangedCallback(GObject* element, MediaPlayerPrivateGStreamer* player) You should omit "element" here. Why aren't all of these callbacks defined statically inside this file? Are they shared anywhere? > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:220 > +void mediaPlayerPrivateAudioChangedCallback(GObject* element, MediaPlayerPrivateGStreamer* player) Ditto. > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h:55 > +void mediaPlayerPrivateVideoChangedCallback(GObject* element, MediaPlayerPrivateGStreamer*); > +void mediaPlayerPrivateAudioChangedCallback(GObject* element, MediaPlayerPrivateGStreamer*); > +gboolean mediaPlayerPrivateAudioChangeTimeoutCallback(MediaPlayerPrivateGStreamer*); > +gboolean mediaPlayerPrivateVideoChangeTimeoutCallback(MediaPlayerPrivateGStreamer*); As above, couldn't these just be defined statically in the C++ file? Why make them public? Also, you should omit the "element" variable.
(In reply to comment #4) > (From update of attachment 88592 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=88592&action=review > > Looks good, though please make callback static, unless there is a reason they should be exposed in the header. Making the callbacks static leads to: Program received signal SIGSEGV, Segmentation fault. 0x00007ffff23102b7 in g_object_get (_object=<value optimized out>, first_property_name= 0x7ffff606b9a8 "n-video") at gobject.c:1867 1867 g_return_if_fail (G_IS_OBJECT (object)); (gdb) bt #0 0x00007ffff23102b7 in g_object_get (_object=<value optimized out>, first_property_name= 0x7ffff606b9a8 "n-video") at gobject.c:1867 #1 0x00007ffff505e527 in WebCore::MediaPlayerPrivateGStreamer::notifyPlayerOfVideo (this=0xcc0000) at ../../../../Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:635 #2 0x00007ffff505cd74 in WebCore::mediaPlayerPrivateVideoChangeTimeoutCallback (player=0xcc0000) at ../../../../Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:235 #3 0x00007ffff1a17edb in g_timeout_dispatch (source=0x7fffe00e01a0, callback=<value optimized out>, user_data=<value optimized out>) at gmain.c:3882 #4 0x00007ffff1a165a3 in g_main_dispatch (context=0x6417e0) at gmain.c:2440 #5 g_main_context_dispatch (context=0x6417e0) at gmain.c:3013 #6 0x00007ffff1a16d80 in g_main_context_iterate (context=0x6417e0, block=1, dispatch=1, self=<value optimized out>) at gmain.c:3091 #7 0x00007ffff1a173f2 in g_main_loop_run (loop=0xce5780) at gmain.c:3299 #8 0x00007ffff4199557 in IA__gtk_main () at gtkmain.c:1255 #9 0x0000000000402b09 in main (argc=2, argv=0x7fffffffd6c8) at ../../../../Tools/GtkLauncher/main.c:274 (gdb) f 1 #1 0x00007ffff505e527 in WebCore::MediaPlayerPrivateGStreamer::notifyPlayerOfVideo (this=0xcc0000) at ../../../../Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:635 635 g_object_get(m_playBin, "n-video", &videoTracks, NULL); (gdb) p m_playBin $1 = (GstElement *) 0xe1
Sorry Martin, my initial patch was wrong in the first place... The notify::caps signal should not reuse mediaPlayerPrivateVideoChangedCallback(). We need a new handler with a different signature.
Created attachment 88985 [details] proposed patch
Created attachment 88987 [details] proposed patch
Comment on attachment 88987 [details] proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=88987&action=review Thanks for fixing this up! > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h:49 > gboolean mediaPlayerPrivateMessageCallback(GstBus* bus, GstMessage* message, gpointer data); > -void mediaPlayerPrivateVolumeChangedCallback(GObject* element, GParamSpec* pspec, gpointer data); > -void mediaPlayerPrivateMuteChangedCallback(GObject* element, GParamSpec* pspec, gpointer data); > void mediaPlayerPrivateSourceChangedCallback(GObject* element, GParamSpec* pspec, gpointer data); It'd be great to remove to remov the remaining two callback in a followup patch. :)
Committed r83566: <http://trac.webkit.org/changeset/83566>