GNOME Bugzilla – Bug 678257
cdda backend not compatible with cdparanoia 0.83 API
Last modified: 2012-08-15 17:23:13 UTC
libcdio split out the paranoia part (which gvfs cdda backend ultimately uses). While doing so, the API was changed slightly and as a result, gvfs does not build against it: CC gvfsd_cdda-gvfsbackendcdda.o gvfsbackendcdda.c: In function 'fetch_metadata': gvfsbackendcdda.c:172:3: error: too many arguments to function 'cdio_get_cdtext' In file included from /usr/include/cdio/cdio.h:61:0, from /usr/include/cdio/cdda.h:18, from /usr/include/cdio/paranoia.h:18, from gvfsbackendcdda.c:58: /usr/include/cdio/disc.h:77:13: note: declared here gvfsbackendcdda.c:174:55: error: 'CDTEXT_TITLE' undeclared (first use in this function) gvfsbackendcdda.c:174:55: note: each undeclared identifier is reported only once for each function it appears in gvfsbackendcdda.c:174:5: error: incompatible type for argument 2 of 'cdtext_get' In file included from /usr/include/cdio/cdio.h:58:0, from /usr/include/cdio/cdda.h:18, from /usr/include/cdio/paranoia.h:18, from gvfsbackendcdda.c:58: /usr/include/cdio/cdtext.h:249:7: note: expected 'cdtext_field_t' but argument is of type 'const struct cdtext_t *' gvfsbackendcdda.c:174:5: error: too few arguments to function 'cdtext_get' In file included from /usr/include/cdio/cdio.h:58:0, from /usr/include/cdio/cdda.h:18, from /usr/include/cdio/paranoia.h:18, from gvfsbackendcdda.c:58: /usr/include/cdio/cdtext.h:249:7: note: declared here gvfsbackendcdda.c:175:56: error: 'CDTEXT_PERFORMER' undeclared (first use in this function) gvfsbackendcdda.c:175:5: error: incompatible type for argument 2 of 'cdtext_get' In file included from /usr/include/cdio/cdio.h:58:0, from /usr/include/cdio/cdda.h:18, from /usr/include/cdio/paranoia.h:18, from gvfsbackendcdda.c:58: /usr/include/cdio/cdtext.h:249:7: note: expected 'cdtext_field_t' but argument is of type 'const struct cdtext_t *' gvfsbackendcdda.c:175:5: error: too few arguments to function 'cdtext_get' In file included from /usr/include/cdio/cdio.h:58:0, from /usr/include/cdio/cdda.h:18, from /usr/include/cdio/paranoia.h:18, from gvfsbackendcdda.c:58: /usr/include/cdio/cdtext.h:249:7: note: declared here gvfsbackendcdda.c:176:49: error: 'CDTEXT_GENRE' undeclared (first use in this function) gvfsbackendcdda.c:176:5: error: incompatible type for argument 2 of 'cdtext_get' In file included from /usr/include/cdio/cdio.h:58:0, from /usr/include/cdio/cdda.h:18, from /usr/include/cdio/paranoia.h:18, from gvfsbackendcdda.c:58: /usr/include/cdio/cdtext.h:249:7: note: expected 'cdtext_field_t' but argument is of type 'const struct cdtext_t *' gvfsbackendcdda.c:176:5: error: too few arguments to function 'cdtext_get' In file included from /usr/include/cdio/cdio.h:58:0, from /usr/include/cdio/cdda.h:18, from /usr/include/cdio/paranoia.h:18, from gvfsbackendcdda.c:58: /usr/include/cdio/cdtext.h:249:7: note: declared here gvfsbackendcdda.c:185:5: error: too many arguments to function 'cdio_get_cdtext' In file included from /usr/include/cdio/cdio.h:61:0, from /usr/include/cdio/cdda.h:18, from /usr/include/cdio/paranoia.h:18, from gvfsbackendcdda.c:58: /usr/include/cdio/disc.h:77:13: note: declared here gvfsbackendcdda.c:187:7: error: incompatible type for argument 2 of 'cdtext_get' In file included from /usr/include/cdio/cdio.h:58:0, from /usr/include/cdio/cdda.h:18, from /usr/include/cdio/paranoia.h:18, from gvfsbackendcdda.c:58: /usr/include/cdio/cdtext.h:249:7: note: expected 'cdtext_field_t' but argument is of type 'const struct cdtext_t *' gvfsbackendcdda.c:187:7: error: too few arguments to function 'cdtext_get' In file included from /usr/include/cdio/cdio.h:58:0, from /usr/include/cdio/cdda.h:18, from /usr/include/cdio/paranoia.h:18, from gvfsbackendcdda.c:58: /usr/include/cdio/cdtext.h:249:7: note: declared here gvfsbackendcdda.c:188:7: error: incompatible type for argument 2 of 'cdtext_get' In file included from /usr/include/cdio/cdio.h:58:0, from /usr/include/cdio/cdda.h:18, from /usr/include/cdio/paranoia.h:18, from gvfsbackendcdda.c:58: /usr/include/cdio/cdtext.h:249:7: note: expected 'cdtext_field_t' but argument is of type 'const struct cdtext_t *' gvfsbackendcdda.c:188:7: error: too few arguments to function 'cdtext_get' In file included from /usr/include/cdio/cdio.h:58:0, from /usr/include/cdio/cdda.h:18, from /usr/include/cdio/paranoia.h:18, from gvfsbackendcdda.c:58: /usr/include/cdio/cdtext.h:249:7: note: declared here make[4]: *** [gvfsd_cdda-gvfsbackendcdda.o] Error 1 make[4]: Leaving directory `/home/abuild/rpmbuild/BUILD/gvfs-1.12.3/daemon
some background on the mailing list regarding that change: http://comments.gmane.org/gmane.comp.audio.libcdio.devel/885
Not sure if it's clear from above what exactly that means, that this cdda is disabled: - nautilus can't mount audio disks - totem can't play audio CDs - Rhythmbox can't play Audio CD's
Created attachment 220497 [details] [review] My attempt to fix it This is my understanding of how the new API should work / behave. It builds without issues, but I lack CD's with CDText information, so I can't test. Also, I'm not sure if you'd prefer having backwards compatibility with older libcdio versions, but that would likely become a bit messier)
Actually, I even just found a CD with Text information, which now shows up correctly again, with libcdio_paranoia 0.83 installed.
The patch committed bug 671259 clashes with the patch here. Could you please update it? Also, we actually need to keep on supporting older versions of libcdio. Furthermore, I have libcdio 0.83 installed here (on Fedora 17) and the headers still use CDTEXT_PERFORMER rather than CDTEXT_FIELD_PERFORMER. Is the change only in 0.84? Has 0.84 been released?
Thanks for your input. This seems to indeed to happen somewhen between 0.83 and 0.84 (not realesed). I did not realize this, as openSUSE 12.2 moved to the git snapshot due to licensing issues in upstream code base it seems (which is the reason why cdparanoia split out there after all). So, likely, the issue is indeed in 0.84, where all the APIs are being switched. As for 'old API support': I'll add support to identify the 'right thing to be done'.
The cdio commit changing the CDTEXT_TITLE (et.al) to CDTEXT_FIELD_TITLE was http://git.savannah.gnu.org/gitweb/?p=libcdio.git;a=commitdiff;h=abe5b8429573c9ac796c274c7abe7f76ef52dc76
Created attachment 220579 [details] [review] Patch to switch API based on LIBCDIO_VERSION_NUM being >= 84 Builds fine with my version of libcdio (being 0.83git, close to git master I think) untested with 0.83 and older though, please help test.
For reference: we're shipping this patch on openSUSE 12.2, which includes libcdio 0.83+git
Review of attachment 220579 [details] [review]: ::: daemon/gvfsbackendcdda.c @@ +182,3 @@ if (cdtext) { + cdda_backend->album_title = cdtext_string_to_utf8 (cdtext_get_const ( +#if LIBCDIO_VERSION_NUM >= 84 Really don't like having #ifdef's in the middle of function calls.
Created attachment 221290 [details] [review] cdda: Support libcdio 0.84 with changed API following CDIO changeset http://git.savannah.gnu.org/gitweb/?p=libcdio.git;a=commitdiff;h=abe5b8429573c9ac796c274c7abe7f76ef52dc76
Let me know whether that patch works. It's yours, but cleaned up for the above.
Created attachment 221291 [details] [review] New version with ifdef's around functions Then let's try this variant...
Attachment 221290 [details] pushed as fe9e82d - cdda: Support libcdio 0.84 with changed API