GNOME Bugzilla – Bug 571989
"Video preview" menu item doesn't do anything
Last modified: 2009-08-03 15:22:29 UTC
View -> Video preview doesn't do anything. It should turn on/off the preview widget and get automatically enabled when "Sending Video" is activated.
Bug is still present in 2.26.0
Created attachment 135453 [details] [review] Branch containing the fix: http://git.collabora.co.uk/?p=user/jtellier/empathy.git;a=shortlog;h=refs/heads/call-window-video-preview. Also happens to fix Bug 583936 libempathy/empathy-call-handler.c | 22 ++++++++ libempathy/empathy-tp-call.c | 37 ++++++++++++++ libempathy/empathy-tp-call.h | 2 + src/empathy-call-window.c | 100 ++++++++++++++++++++++++++++++++++--- src/empathy-call-window.ui | 1 + 5 files changed, 155 insertions(+), 7 deletions(-)
I'm not convinced that the window preview should be enabled by default if the call is initiated as audio only. Your patch just show/hides the preview widget. I think we shouldn't construct the gst pipeline when preview is disabled, there is no point to start the webcam if we are not using it.
bug 583936 is a dup of bug 582598
You didn't fix this comment: http://bugzilla.gnome.org/show_bug.cgi?id=582810#c3 I'm wondering if it's worth to add a "video-stream-changed" signal on EmpathyCallHandler which just relay the "notify::video-stream" from the EmpathyTpCall. Maybe the call window could connect directly this signal? Didn't review call-window changes yet.
+ * Determines if the call managed by this #EmpathyCallHandler was created as + * a video conversation. I'd say "Return %TRUE if the call managed by this #EmpathyCallHandler was created with video enabled". (you have to use %TRUE and %FALSE in gtk-doc comments) empathy_call_window_setup_video_preview I'd check if priv->video_preview and priv->video_tee are NULL in that function so caller doesn't have to check it themself. empathy_call_window_sink_added_cb I'm not sure to understand this change. Does it automatically start our preview when we receive video from the other contact?
Created attachment 136300 [details] [review] Fixed libempathy/empathy-call-handler.c | 17 +++ libempathy/empathy-call-handler.h | 2 + libempathy/empathy-tp-call.c | 35 ++++++ libempathy/empathy-tp-call.h | 2 + src/empathy-call-window.c | 217 +++++++++++++++++++++++++++++++------ src/empathy-call-window.ui | 1 + 6 files changed, 238 insertions(+), 36 deletions(-)
empathy_call_window_setup_video_preview: I'd use an early return instead of a huge if block containing all the code. Are priv->video_tee and priv->video_preview garanteed to be always NULL/not-NULL at the same time? If yes, I'd test only once and assert == NULL on the second. empathy_call_window_setup_video_preview_visibility If the "if" block has { }, the "else" block should have to (style convention). Did you remove the change in empathy_call_window_sink_added_cb? What's the rationnal of this?
Created attachment 136362 [details] [review] patch v3 libempathy-gtk/empathy-video-widget.c | 3 + libempathy/empathy-call-handler.c | 17 ++ libempathy/empathy-call-handler.h | 2 + libempathy/empathy-tp-call.c | 35 ++++ libempathy/empathy-tp-call.h | 2 + src/empathy-call-window.c | 330 ++++++++++++++++++++++++--------- src/empathy-call-window.ui | 1 + 7 files changed, 302 insertions(+), 88 deletions(-)
Some more comments (sorry... :) empathy_call_window_setup_video_preview: - all the variable definitions should be done at the begin of the function - identation is wrong - I'd assert g_assert (priv->video_tee == NULL) after the if. I observed the following problem with this branch: - I start an audio call - I enable the video preview - The widget appears but with my avatar (my webcam is not started) - I enable sending video - Now the webcam is started and the preview displays it
If have this crash with latest version of your branch: - A starts a *video* call with B (using the webcam icon) - B accepts and crashes I don't have any problem if I start an audio call and then add video after. ** (empathy:16852): DEBUG: stream 2 0x2c05640 (audio) _tf_stream_try_sending_codecs: called (send_local:1 send_supported:0) ** (empathy:16852): DEBUG: stream 2 0x2c05640 (audio) _tf_stream_try_sending_codecs: 100: audio SPEEX clock:16000 channels:1 ** (empathy:16852): DEBUG: stream 2 0x2c05640 (audio) _tf_stream_try_sending_codecs: 101: audio SPEEX clock:8000 channels:1 ** (empathy:16852): DEBUG: stream 2 0x2c05640 (audio) _tf_stream_try_sending_codecs: 0: audio PCMU clock:8000 channels:0 ** (empathy:16852): DEBUG: stream 2 0x2c05640 (audio) _tf_stream_try_sending_codecs: 8: audio PCMA clock:8000 channels:0 ** (empathy:16852): DEBUG: stream 2 0x2c05640 (audio) _tf_stream_try_sending_codecs: 3: audio GSM clock:8000 channels:0 ** (empathy:16852): DEBUG: stream 2 0x2c05640 (audio) _tf_stream_try_sending_codecs: 98: audio telephone-event clock:8000 channels:0 events=0-15 ** (empathy:16852): DEBUG: stream 2 0x2c05640 (audio) fs_codecs_to_tp: adding codec SPEEX [100] ** (empathy:16852): DEBUG: stream 2 0x2c05640 (audio) fs_codecs_to_tp: adding codec SPEEX [101] ** (empathy:16852): DEBUG: stream 2 0x2c05640 (audio) fs_codecs_to_tp: adding codec PCMU [0] ** (empathy:16852): DEBUG: stream 2 0x2c05640 (audio) fs_codecs_to_tp: adding codec PCMA [8] ** (empathy:16852): DEBUG: stream 2 0x2c05640 (audio) fs_codecs_to_tp: adding codec GSM [3] ** (empathy:16852): DEBUG: stream 2 0x2c05640 (audio) fs_codecs_to_tp: adding codec telephone-event [98] ** (empathy:16852): DEBUG: stream 2 0x2c05640 (audio) _tf_stream_try_sending_codecs: calling MediaStreamHandler::Ready ** (empathy:16852): DEBUG: stream 1 0x2c05590 (video) set_remote_codecs: called ** (empathy:16852): DEBUG: stream 1 0x2c05590 (video) set_remote_codecs: adding remote codec H264 [97] ** (empathy:16852): DEBUG: stream 1 0x2c05590 (video) set_remote_codecs: adding remote codec THEORA [96] ** (empathy:16852): DEBUG: stream 1 0x2c05590 (video) set_remote_codecs: adding remote codec H263 [34] ** (empathy:16852): DEBUG: stream 1 0x2c05590 (video) set_remote_codecs: adding remote codec H263-1998 [98] ** (empathy:16852): DEBUG: stream 1 0x2c05590 (video) set_remote_codecs: adding remote codec H263-N800 [99] ** (empathy:16852): DEBUG: stream 1 0x2c05590 (video) _tf_stream_try_sending_codecs: called (send_local:0 send_supported:1) ** (empathy:16852): DEBUG: stream 1 0x2c05590 (video) _tf_stream_try_sending_codecs: 97: video H264 clock:90000 channels:0 ** (empathy:16852): DEBUG: stream 1 0x2c05590 (video) _tf_stream_try_sending_codecs: 96: video THEORA clock:90000 channels:0 delivery-method=inline ** (empathy:16852): DEBUG: stream 1 0x2c05590 (video) _tf_stream_try_sending_codecs: 34: video H263 clock:90000 channels:0 ** (empathy:16852): DEBUG: stream 1 0x2c05590 (video) _tf_stream_try_sending_codecs: 98: video H263-1998 clock:90000 channels:0 ** (empathy:16852): DEBUG: stream 1 0x2c05590 (video) _tf_stream_try_sending_codecs: 99: video H263-N800 clock:90000 channels:0 ** (empathy:16852): DEBUG: stream 1 0x2c05590 (video) fs_codecs_to_tp: adding codec H264 [97] ** (empathy:16852): DEBUG: stream 1 0x2c05590 (video) fs_codecs_to_tp: adding codec THEORA [96] ** (empathy:16852): DEBUG: stream 1 0x2c05590 (video) fs_codecs_to_tp: adding codec H263 [34] ** (empathy:16852): DEBUG: stream 1 0x2c05590 (video) fs_codecs_to_tp: adding codec H263-1998 [98] ** (empathy:16852): DEBUG: stream 1 0x2c05590 (video) fs_codecs_to_tp: adding codec H263-N800 [99] ** (empathy:16852): DEBUG: stream 1 0x2c05590 (video) _tf_stream_try_sending_codecs: calling MediaStreamHandler::SupportedCodecs ** (empathy:16852): DEBUG: stream 1 0x2c05590 (video) set_stream_playing: 0 ** (empathy:16852): DEBUG: stream 1 0x2c05590 (video) set_stream_sending: 0 ** (empathy:16852): DEBUG: stream 2 0x2c05640 (audio) set_remote_codecs: called ** (empathy:16852): DEBUG: stream 2 0x2c05640 (audio) set_remote_codecs: adding remote codec SPEEX [100] ** (empathy:16852): DEBUG: stream 2 0x2c05640 (audio) set_remote_codecs: adding remote codec SPEEX [101] ** (empathy:16852): DEBUG: stream 2 0x2c05640 (audio) set_remote_codecs: adding remote codec PCMU [0] ** (empathy:16852): DEBUG: stream 2 0x2c05640 (audio) set_remote_codecs: adding remote codec PCMA [8] ** (empathy:16852): DEBUG: stream 2 0x2c05640 (audio) set_remote_codecs: adding remote codec GSM [3] ** (empathy:16852): DEBUG: stream 2 0x2c05640 (audio) set_remote_codecs: adding remote codec telephone-event [98] ** (empathy:16852): DEBUG: stream 2 0x2c05640 (audio) _tf_stream_try_sending_codecs: called (send_local:0 send_supported:1) ** (empathy:16852): DEBUG: stream 2 0x2c05640 (audio) _tf_stream_try_sending_codecs: 100: audio SPEEX clock:16000 channels:1 ** (empathy:16852): DEBUG: stream 2 0x2c05640 (audio) _tf_stream_try_sending_codecs: 101: audio SPEEX clock:8000 channels:1 ** (empathy:16852): DEBUG: stream 2 0x2c05640 (audio) _tf_stream_try_sending_codecs: 0: audio PCMU clock:8000 channels:0 ** (empathy:16852): DEBUG: stream 2 0x2c05640 (audio) _tf_stream_try_sending_codecs: 8: audio PCMA clock:8000 channels:0 ** (empathy:16852): DEBUG: stream 2 0x2c05640 (audio) _tf_stream_try_sending_codecs: 3: audio GSM clock:8000 channels:0 ** (empathy:16852): DEBUG: stream 2 0x2c05640 (audio) _tf_stream_try_sending_codecs: 98: audio telephone-event clock:8000 channels:0 events=0-15 ** (empathy:16852): DEBUG: stream 2 0x2c05640 (audio) fs_codecs_to_tp: adding codec SPEEX [100] ** (empathy:16852): DEBUG: stream 2 0x2c05640 (audio) fs_codecs_to_tp: adding codec SPEEX [101] ** (empathy:16852): DEBUG: stream 2 0x2c05640 (audio) fs_codecs_to_tp: adding codec PCMU [0] ** (empathy:16852): DEBUG: stream 2 0x2c05640 (audio) fs_codecs_to_tp: adding codec PCMA [8] ** (empathy:16852): DEBUG: stream 2 0x2c05640 (audio) fs_codecs_to_tp: adding codec GSM [3] ** (empathy:16852): DEBUG: stream 2 0x2c05640 (audio) fs_codecs_to_tp: adding codec telephone-event [98] ** (empathy:16852): DEBUG: stream 2 0x2c05640 (audio) _tf_stream_try_sending_codecs: calling MediaStreamHandler::SupportedCodecs ** (empathy:16852): DEBUG: stream 2 0x2c05640 (audio) set_stream_playing: 0 ** (empathy:16852): DEBUG: stream 2 0x2c05640 (audio) set_stream_sending: 0 ** (empathy:16852): DEBUG: stream 1 0x2c05590 (video) _tf_stream_bus_message: Codecs changed ** (empathy:16852): DEBUG: stream 1 0x2c05590 (video) _tf_stream_try_sending_codecs: called (send_local:0 send_supported:0) ** (empathy:16852): DEBUG: stream 1 0x2c05590 (video) _tf_stream_try_sending_codecs: 97: video H264 clock:90000 channels:0 ** (empathy:16852): DEBUG: stream 1 0x2c05590 (video) _tf_stream_try_sending_codecs: 96: video THEORA clock:90000 channels:0 delivery-method=inline ** (empathy:16852): DEBUG: stream 1 0x2c05590 (video) _tf_stream_try_sending_codecs: 34: video H263 clock:90000 channels:0 ** (empathy:16852): DEBUG: stream 1 0x2c05590 (video) _tf_stream_try_sending_codecs: 98: video H263-1998 clock:90000 channels:0 ** (empathy:16852): DEBUG: stream 1 0x2c05590 (video) _tf_stream_try_sending_codecs: 99: video H263-N800 clock:90000 channels:0 ** (empathy:16852): DEBUG: stream 2 0x2c05640 (audio) _tf_stream_bus_message: Codecs changed ** (empathy:16852): DEBUG: stream 2 0x2c05640 (audio) _tf_stream_try_sending_codecs: called (send_local:0 send_supported:0) ** (empathy:16852): DEBUG: stream 2 0x2c05640 (audio) _tf_stream_try_sending_codecs: 100: audio SPEEX clock:16000 channels:1 ** (empathy:16852): DEBUG: stream 2 0x2c05640 (audio) _tf_stream_try_sending_codecs: 101: audio SPEEX clock:8000 channels:1 ** (empathy:16852): DEBUG: stream 2 0x2c05640 (audio) _tf_stream_try_sending_codecs: 0: audio PCMU clock:8000 channels:0 ** (empathy:16852): DEBUG: stream 2 0x2c05640 (audio) _tf_stream_try_sending_codecs: 8: audio PCMA clock:8000 channels:0 ** (empathy:16852): DEBUG: stream 2 0x2c05640 (audio) _tf_stream_try_sending_codecs: 3: audio GSM clock:8000 channels:0 ** (empathy:16852): DEBUG: stream 2 0x2c05640 (audio) _tf_stream_try_sending_codecs: 98: audio telephone-event clock:8000 channels:0 events=0-15 ** (empathy:16852): DEBUG: stream 2 0x2c05640 (audio) _tf_stream_bus_message: Codecs changed ** (empathy:16852): DEBUG: stream 2 0x2c05640 (audio) _tf_stream_try_sending_codecs: called (send_local:0 send_supported:0) ** (empathy:16852): DEBUG: stream 2 0x2c05640 (audio) _tf_stream_try_sending_codecs: 100: audio SPEEX clock:16000 channels:1 ** (empathy:16852): DEBUG: stream 2 0x2c05640 (audio) _tf_stream_try_sending_codecs: 101: audio SPEEX clock:8000 channels:1 ** (empathy:16852): DEBUG: stream 2 0x2c05640 (audio) _tf_stream_try_sending_codecs: 0: audio PCMU clock:8000 channels:0 ** (empathy:16852): DEBUG: stream 2 0x2c05640 (audio) _tf_stream_try_sending_codecs: 8: audio PCMA clock:8000 channels:0 ** (empathy:16852): DEBUG: stream 2 0x2c05640 (audio) _tf_stream_try_sending_codecs: 3: audio GSM clock:8000 channels:0 ** (empathy:16852): DEBUG: stream 2 0x2c05640 (audio) _tf_stream_try_sending_codecs: 98: audio telephone-event clock:8000 channels:0 events=0-15 ** (empathy:16852): DEBUG: stream 2 0x2c05640 (audio) _tf_stream_bus_message: Send codec changed: 100: audio SPEEX clock:16000 channels:1 params:(nil) ** (empathy:16852): DEBUG: stream 1 0x2c05590 (video) _tf_stream_bus_message: Send codec changed: 97: video H264 clock:90000 channels:0 params:(nil) ** WARNING **: stream 1 0x2c05590 (video) _tf_stream_bus_message: error (internal (2)): Internal error while discovering codecs configurations : Got notify::caps signal on the discovery codecs whith no codecs being discovered aborting... Program received signal SIGABRT, Aborted.
+ Trace 215981
Thread 140195661416432 (LWP 16852)
Another bug I just noticed: - A starts an audio call with B - A enables video preview - B accepts the call - call is connected and A's video preview disappears
the error in comment #11 is mostly likely fixed in farsight 0.0.12+
This particular error seems fixed, but I get a segfault in libx264.so. I'm still investigating to see if my code could be the cause. As for the other problem mentioned it has been fixed.
I've got a similar segfault in both "master" and "2.26.2". Maybe the problem is not related to this branch after all.
Olivier: indeed, I got this error with master as well. Jonathan: Branch seems ok. I finally merged it. Thanks for your hard work!
Comment on attachment 136362 [details] [review] patch v3 Original description (too long for the soon-to-be-upgraded Bugzilla): Just fixed what you've pointed out and corrected a bug which showed up when the branch got merged with the changed regarding the Redial button. Regarding the changes to empathy_call_window_sink_added_cb, what I had initially done did have the side effect of automatically starting the video preview when started receiving video from the remote contact. It has been fixed.