Amarok Blog

Don’t worry all you people, I wasn’t killed in a bus accident yet; you are lucky, I actually go out quite rarely so I don’t suffer from this kind of problems very often, the worst that can happen would be a health problem, but I’m lucky enough to feel decently well at the moment Of course there is too much heat for my liking and I start to feel how annoying it is at this temperature, especially staying in my home office with two/three boxes online.

Anyway, as I promised, I started looking at the fork of libdvdnav lead by Nico, to import a newer version of libdvdnav in xine, and cleaning up the patches applied.

Unfortunately it wasn’t exactly straightforward to update the libdvdnav version; beside my will to move the sources into the contrib/ subdirectory as I already did for libmpcdec and ffmpeg, Nico started to replace autotools with a build system alike to FFmpeg’s (which is quite fine by me, as I can integrate it nicely enough into xine-lib), so I had to make sure that the build system was up to a minimum support for the features I needed. The main showstopper was the lack of a way to build in a directory different from the original sources directory, but that was quite easy to handle.

But luckily working with Nico is quite enjoyable All the patch I’ve sent for inclusion up to tonight (so excluding the one I sent less than an hour ago) were accepted and committed to the Subversion repository, which means that the build system is usable from xine-lib’s custom base system, that the compiler flags we inject are respected, that the library builds with -Werror-implicit-declarations (the change was needed to libdvdread, Nico took care of sending it up there).

Tonight I rebuilt xine-lib-1.2 with the new libdvdnav, and it’s working nicely. I suppose the reason why external libdvdnav is not working on seeking is that the code was present in cvs (for sf.net’s dvd project) but not released yet. I should probably also add support for .pc files for libdvdnav and then use those to check for its presence, so to require at least a new enough version that doesn’t have seeking problems, but that is not a priority until a release is done.

There is only one problem that has to be considered: the libdvdnav copy in xine-lib is patched with a patch from Bastien Nocera (Totem’s author) which is used to play invalid DVDs (non-encrypted ISO filesystem DVDs; proper DVDs have UDF as file system). That patch as it is won’t be accepted by libdvdnav authors, and sincerely I wouldn’t have accepted it for xine-lib either.

Why this? Well, the patch takes an opaque type, and makes it transparent, copying out of libdvdcss the structure definition. This is fine as long as the structure is not changed on libdvdcss, but the reason why a type is opaque, is just so that you don’t have to put its definition in the ABI, so you can change it without having to deal with software crashing because it was compiled against the previous definition. This patch breaks this assumption with xine, so it’s bad from a good practices point of view.

I’m not sure myself how much sense does it make to use such a stupid solution (sorry Bastien, but re-declaring an opaque type is a stupid solution) to consider invalid media. A better solution would be to always use the files if they are found, rather than using UDF access, with fallback to raw UDF instead. This should work, but I need first some media to test with, and then I have to understand libdvdnav code.

Anyway, the build framework changes are now committed to the xine-lib-1.2-newdvdnav branch, the “only” thing missing there is the libdvdnav code itself; the reason for this is that up to tonight I was still working with patched sources, not with upstream sources, and I wanted to avoid recommitting everything every time. I will probably wait as much as I can before committing the sources themselves, with this “as much as I can” representing ideally the time till I get to have committed to libdvdnav all the changes that make sense for xine-lib (revised Bastien’s patch, and the file descriptor leak patch).

I also want to thank Nico for the work being done toward adding an option to use external libdvdread in libdvdnav, which will certainly help Gentoo and other distributions: you wouldn’t have to duplicate the code between libdvdnav and libdvdread; you’d have one more dependency in the libdvdnav library, but that shouldn’t be much overhead.

And I want to say it officially: there is a lot of code in xine-lib that scares the hell out of me, and I would like to see it killed before it propagates too much; unfortunately overhauling all of it is difficult alone, especially since I might end up having repercussions that I don’t see (like the CDDA failure that didn’t fail for me when I tested – I was using by-extension detection in Amarok – and the DVD failure that still I was unable to reproduce till at least since Darren fixed it).

I start hating spending my time on xine, I hope to be able to continue working on it till it’s enjoyable to do so.