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 656884 - Give more feedback when changing camera
Give more feedback when changing camera
Status: RESOLVED FIXED
Product: empathy
Classification: Core
Component: VoIP
unspecified
Other Linux
: Normal normal
: ---
Assigned To: empathy-maint
Depends on:
Blocks:
 
 
Reported: 2011-08-19 10:05 UTC by Jonny Lamb
Modified: 2011-09-26 10:19 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
RoundedActor: allow to set a different round factor (3.09 KB, patch)
2011-09-21 13:47 UTC, Emilio Pozuelo Monfort
committed Details | Review
CallWindow: show a spinner when switching cameras (5.16 KB, patch)
2011-09-21 13:47 UTC, Emilio Pozuelo Monfort
committed Details | Review
screenshot (91.84 KB, image/png)
2011-09-21 13:48 UTC, Emilio Pozuelo Monfort
  Details

Description Jonny Lamb 2011-08-19 10:05:21 UTC
When you change camera your preview just freezes as if it's crashed. It'd be awesome if you could show a spinner somewhere? Perhaps dim the frozen preview and show a spinner on top or something? Not sure how hard this is, but would make it less bad given v4l2 making all device changes take ten minutes.
Comment 1 Emilio Pozuelo Monfort 2011-08-19 19:17:21 UTC
Ideally v4l2 wouldn't take so long :-)  But since we can't do that much about it from an Empathy POV...

Nick, what do you think?
Comment 2 Nick Richards 2011-08-23 16:39:31 UTC
Yes, If we don't have any feedback to give the user we should use an indeterminate feedback device (i.e. a spinner ;-)

I'll add this to the next batch of wireframes.
Comment 3 Nick Richards 2011-08-24 11:26:49 UTC
This has been done, it's section 1.3a (p8) of the 24th July wireframes.
Comment 4 Emilio Pozuelo Monfort 2011-09-21 13:47:07 UTC
Created attachment 197153 [details] [review]
RoundedActor: allow to set a different round factor
Comment 5 Emilio Pozuelo Monfort 2011-09-21 13:47:12 UTC
Created attachment 197154 [details] [review]
CallWindow: show a spinner when switching cameras
Comment 6 Emilio Pozuelo Monfort 2011-09-21 13:48:09 UTC
Created attachment 197155 [details]
screenshot
Comment 7 Guillaume Desmottes 2011-09-21 14:05:10 UTC
Review of attachment 197153 [details] [review]:

++
Comment 8 Guillaume Desmottes 2011-09-21 14:09:30 UTC
Review of attachment 197154 [details] [review]:

::: src/empathy-call-window.c
@@ +3390,3 @@
+            if (newstate == GST_STATE_PLAYING &&
+                pending == GST_STATE_VOID_PENDING)
+              empathy_call_window_stop_camera_spinning (self);

Shouldn't we stop it if there is an error opening the camera as well?
Comment 9 Jonny Lamb 2011-09-21 14:17:33 UTC
Why is the spinner the size of the preview? It looks stupid. I think you should make it the same size as every other spinner. Didn't we have the same problem with the log viewer spinner?
Comment 10 Jonny Lamb 2011-09-21 14:19:42 UTC
Also, does it dim the old video actor before showing the spinner instead of just throwing the spinner right on top of your face? That'd look better, wouldn't it?
Comment 11 Emilio Pozuelo Monfort 2011-09-22 11:09:10 UTC
(In reply to comment #8)
> Review of attachment 197154 [details] [review]:
> 
> ::: src/empathy-call-window.c
> @@ +3390,3 @@
> +            if (newstate == GST_STATE_PLAYING &&
> +                pending == GST_STATE_VOID_PENDING)
> +              empathy_call_window_stop_camera_spinning (self);
> 
> Shouldn't we stop it if there is an error opening the camera as well?

I hadn't thought about that, but looking at the code it seems that'd be covered by:

          if (g_str_has_prefix (gst_element_get_name (gst_error),
                VIDEO_INPUT_ERROR_PREFIX))
            {
              /* Remove the video input and continue */
              if (priv->video_input != NULL)
                empathy_call_window_remove_video_input (self);
              gst_element_set_state (priv->pipeline, GST_STATE_PLAYING);
            }

And empathy_call_window_remove_video_input() destroys the video preview (and that includes the spinner) so this is handled.
Comment 12 Emilio Pozuelo Monfort 2011-09-22 11:12:43 UTC
(In reply to comment #9)
> Why is the spinner the size of the preview? It looks stupid. I think you should
> make it the same size as every other spinner. Didn't we have the same problem
> with the log viewer spinner?

It's kinda big because I was following the wireframes, and a normal spinner (which I tried first) looks just too small to me. The log viewer has a bigger spinner than normal (not as big as this one though).  It's trivial to change anyway. Nick?

(In reply to comment #10)
> Also, does it dim the old video actor before showing the spinner instead of
> just throwing the spinner right on top of your face? That'd look better,
> wouldn't it?

The GtkClutterActor that contains the spinner is set to be half transparent, so the old video looks dimmed, yes.
Comment 13 Guillaume Desmottes 2011-09-22 11:47:05 UTC
Comment on attachment 197154 [details] [review]
CallWindow: show a spinner when switching cameras

Cool, good to go once we branch then.
Comment 14 Guillaume Desmottes 2011-09-26 10:19:03 UTC
Merged to master; thanks!

Attachment 197153 [details] pushed as 6e0b062 - RoundedActor: allow to set a different round factor
Attachment 197154 [details] pushed as 288a8d8 - CallWindow: show a spinner when switching cameras