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 665244 - new cheese camera device monitor crashes Empathy
new cheese camera device monitor crashes Empathy
Status: RESOLVED FIXED
Product: empathy
Classification: Core
Component: VoIP
unspecified
Other Linux
: Low minor
: ---
Assigned To: empathy-maint
empathy-maint
Depends on:
Blocks:
 
 
Reported: 2011-11-30 20:18 UTC by Raluca-Elena Podiuc
Modified: 2011-12-02 08:21 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Backtrace (3.21 KB, application/octet-stream)
2011-12-01 04:44 UTC, Nicolas Dufresne (ndufresne)
  Details
renamespace CheeseCameraDeviceMonitor (19.53 KB, patch)
2011-12-01 10:29 UTC, Guillaume Desmottes
committed Details | Review

Description Raluca-Elena Podiuc 2011-11-30 20:18:17 UTC
This cheese commit breaks Empathy:
http://git.gnome.org/browse/cheese/commit/?id=054a929303468353537442b4ba77590449c90a73

Cheese used to emit signals like this:
  g_signal_emit (monitor, monitor_signals[ADDED], 0,
                 devpath,
                 device_file,
                 product_name,
                 v4l_version);

but now it does this:

   CheeseCameraDevice *device = cheese_camera_device_monitor_set_up_device (udevice);
   g_signal_emit (monitor, monitor_signals[ADDED], 0, device);


where cheese_camera_device_monitor_set_up_device does
  CheeseCameraDevice *device = cheese_camera_device_new (devpath,
                                                         device_file,
                                                         product_name,
                                                         v4l_version,
                                                         &error);


Empathy could get all the required fields from the CheeseCameraDevice structure, but this change breaks the ABI.


I'm not sure who should address the issue:
- Empathy can from now on depend on a new libcheese and update itself
- Cheese rewrite the patch to support bot old-style signals and add new signals to address the new-style


I'm also worried about existing Empathy clients running on systems that update only libcheese. On those systems old Empaty clients will crash, won't they?
Unfortunately Cheese released 3.3.2 containing this patch.
Comment 1 Raluca-Elena Podiuc 2011-11-30 20:32:13 UTC
Similarly this patch

http://git.gnome.org/browse/cheese/commit/?id=9e2f1e18387e2d492715c6e6ae9aac1829751531

commit 9e2f1e18387e2d492715c6e6ae9aac1829751531
Author: David King <amigadave@amigadave.com>
Date:   Sun Nov 6 16:13:10 2011 +0100

    Improve CheeseEffect documentation
    
    Document the private methods in CheeseEffect. Simplify effect
    construction, by making the name and pipeline-desc properties
    construct-only. Improve some variables names.

Does this:

 CheeseEffect *
-cheese_effect_new (void)
+cheese_effect_new (const gchar *name, const gchar *pipeline_desc)

breaking Empathy compilation with new Cheese. The fix is very simple for Empathy, but is the ABI breakage allowed?
Comment 2 David King 2011-11-30 21:31:16 UTC
(In reply to comment #1)
> breaking Empathy compilation with new Cheese. The fix is very simple for
> Empathy, but is the ABI breakage allowed?

During development releases, yes. For guidance on this, see the release planning wiki page:

http://live.gnome.org/ReleasePlanning/Freezes#API.2BAC8-ABI_Freeze

which for the 3.3 cycle corresponds to a API/ABI freeze date of 6th February 2012:

http://live.gnome.org/ThreePointThree#Schedule

Cheese is not strictly part of the development platform, but I intend to stick to the freeze nonetheless. For Cheese 3.3.2, the libtool library version was bumped to indicate an ABI-incompatible change for both libcheese and libcheese-gtk:

http://git.gnome.org/browse/cheese/commit/?id=73975f712b4161a44ec98ef670093d23e8b665db

This means that an Empathy version linked against an old libcheese will not run with a new libcheese version (in response to comment #0). I also updated the Cheese API reference to indicate that the API is currently unstable, and is likely to change again in the future:

http://developer.gnome.org/cheese/unstable/CheeseCameraDeviceMonitor.html
Comment 3 Raluca-Elena Podiuc 2011-11-30 23:58:48 UTC
Actually, I was mistaking: Empathy has copy-pasted camera detection from cheese.

I guess I was getting segfaults because I was building it with libcheese and I think the linker gave priority to libcheese rather than libempathy to some symbols.

But should Empathy keep it's own copy-pasted sources for cheese-camera-detection?
Or at least when WITH_CHEESE is true, shouldn't it drop it's own copy-pasted ones from the tree?
Comment 4 Nicolas Dufresne (ndufresne) 2011-12-01 04:38:07 UTC
I'm having crashes too when adding/removing USB camera. I've built a Call enable empathy for Fedora.
Comment 5 Nicolas Dufresne (ndufresne) 2011-12-01 04:44:27 UTC
Created attachment 202498 [details]
Backtrace
Comment 6 Nicolas Dufresne (ndufresne) 2011-12-01 05:08:31 UTC
Build with --with-cheese=no workaround the issue. I see three solutions to this, 1) rename the internal copy 2) don't link empathy-call to libcheese, as this process does not really need it 3) Simply use cheese library.
Comment 7 Guillaume Desmottes 2011-12-01 09:51:49 UTC
3) isn't really an option, Ubuntu does't build with Cheese (it's not on the CD) and I want them to have  camera detection anyway.

I think that 1) is the proper solution and 2) is good to do that anyway.
Comment 8 Guillaume Desmottes 2011-12-01 10:04:23 UTC
Humm actually only libempathy-gtk is linking on libcheese (used in the avatar code) but that doesn't solve our problem as empathy-call needs to link on libempathy and libempathy-gtk anyway.

So 2) it is.
Comment 9 Raluca-Elena Podiuc 2011-12-01 10:23:23 UTC
@Guillaume: I'm working on my gsoc patches to bring effects to empathy-call. They will need libcheese linked to emapthy-call.

I think there's room for a 4) if with-cheese=yes don't use the internal copy, but the one from libcheese.
Comment 10 Guillaume Desmottes 2011-12-01 10:29:39 UTC
Created attachment 202507 [details] [review]
renamespace CheeseCameraDeviceMonitor

This ensures empathy will always use our version, even when linking on
libcheese, and so avoid incompatibliy when libcheese breaks its ABI.
Comment 11 Guillaume Desmottes 2011-12-01 10:30:49 UTC
Raluca, Nicolas: could one of you test and review this patch please?
Comment 12 Guillaume Desmottes 2011-12-01 10:35:21 UTC
(In reply to comment #9)
> @Guillaume: I'm working on my gsoc patches to bring effects to empathy-call.
> They will need libcheese linked to emapthy-call.

Great! So yeah we need to be able to link on it anyway.

> I think there's room for a 4) if with-cheese=yes don't use the internal copy,
> but the one from libcheese.

That won't work as libcheese may have a different API/ABI than our internal copy.
Comment 13 Raluca-Elena Podiuc 2011-12-01 10:50:14 UTC
(In reply to comment #12)
> That won't work as libcheese may have a different API/ABI than our internal
> copy.

That can be address by updating Empathy to the current Cheese version, can't it?
Eh, I'm sure there are other things at stake and that you know better :)
Comment 14 Guillaume Desmottes 2011-12-01 11:23:43 UTC
Yeah that would work but means we'll have this problem back as soon as libcheese's API/ABI changes again.

Another argument is that I prefer to have all versions using the same code (much easier to debug issues).

Note that I'd be happy to update our own copy of this code but ideally I'd prefer to wait that the API is frozen for 3.4 first.
Comment 15 Nicolas Dufresne (ndufresne) 2011-12-01 15:56:29 UTC
Review of attachment 202507 [details] [review]:

Looks good.
Comment 16 Nicolas Dufresne (ndufresne) 2011-12-01 16:02:41 UTC
(In reply to comment #9)
> @Guillaume: I'm working on my gsoc patches to bring effects to empathy-call.
> They will need libcheese linked to emapthy-call.
> 
> I think there's room for a 4) if with-cheese=yes don't use the internal copy,
> but the one from libcheese.

Honestly I would not use cheese effect, but ratter something faster like GstFilter (not yet merge though). The idea is that to add/remove effect with cheese you need to turn off/on the camera, which take a lot of time with certain USB UVC camera. See http://gstconf.ubicast.tv/videos/gstreamer-and-farsight/ for more info.

Guillaume, I didn't tested it yet, but I'm not worried, that will definitely work.
Comment 17 Guillaume Desmottes 2011-12-02 08:21:08 UTC
Attachment 202507 [details] pushed as 0fdc280 - renamespace CheeseCameraDeviceMonitor