GNOME Bugzilla – Bug 353870
Save/Load project from/to file
Last modified: 2008-10-11 10:40:35 UTC
We still don't save projects to file, this is getting urgent.
Has a project file format been determined? SMIL is an option however more advanced features may not be handled well. An XML format would be common way of doing things. I would like to see a format which is friendly to versioning programs similar to CVS and SVN. Having the ability to collaborate with these established tools would be very useful.
It is going to be XML, maybe compressed.
I have the beginnings of a patch for this: it contains most (perhaps all) of the UI work required to handle file saving and loading (for example, what to do if the filepath to save to already exists), and the connecting of signals to new projects (required when a new project is loaded). It doesn't include the actual code used to dump the project into or out of files (though it does have a stub for each of these actions - the save stub writes some of the data out, and the load stub raises an exception). I wrote this a while ago, and hoped to get it finished off, but then ran out of spare time. I've just updated it to apply to current CVS head, and still hope to get it finished off. However, I may well run out of time again, so I'm adding the patch as it stands to this issue. The patch which I will shortly attach is therefore messy and a work in progress, and also addresses a couple of other issues I found along the way (eg, bug 381959). I'll try and hang around on IRC (#pitivi) over the next few days - if anyone is wanting to work on this, try giving me a prod first to check if I've made any further progress. Hopefully I'll manage to get a cleaner patch together soon, but no promises. :(
Created attachment 77595 [details] [review] Work-In-Progress patch which addresses UI side of load/save
wow, that's one big hefty patch. I hope to have some time to review it some time soon. The code looks nice ! Maybe it would be better to split up your work in smaller patches. There's more in this patch than just save/load functionnality.
Several comments after having gone through the patch more thoroughly. I've extracted some modfications already in cvs : the svn/cvs uninstalled patch, and cleanups of the .cvsignore. The following functionalities should be separated in different modules/classes/interfaces and a separate patch: _ Undo/Redo . For this we need an interface and classes for removeable actions, with things like their name, their opposite, methods to translate the arguments into their opposites, etc... Where you want to use them is the good place nonetheless. _ Project load/save logic (not the actual writing) is quite nice. Not having the project load automatically at __init__ is a point I hadn't thought about and makes sense. _ (de)serialization. We need to have an interface for serializable objects (The info we are going to save in the files). It would be interesting to make it generic enough for some other ideas which are being though about, like collaborative editing via network. Maybe they could support different serialization protocols (but we only need xml for todays' case which is file save/load). _ Using functions as callbacks for dialogbox signals instead of class methods. Makes the code cleaner. _ Putting the glade-based classes in separates files It seems you forgot to add some new files to the patch :) Like filelisterrordialog and confirmclosedialog. It would be nice to have a new patch with those files so I can have a deeper look and maybe start splitting up the patch and commit some stuff. And I really hate the fact that we don't have cvs at gnome to ease this process... As you said on IRC, this is a diff of your current work, not just file save/load. I'll start implementing/applying some of the points above as soon as I get time. Nice work !
Created attachment 77683 [details] [review] Updated work-in-progress patch Redid the diff, with updates from cvs, and remembering the "-N" option to diff so I should have the contents of the new files in the patch this time.
Created attachment 77697 [details] [review] Patch to separate DiscovererErrorDialog out Extracted part of my mega-patch which concerns separating DiscovererErrorDialog out into a separate file (filelisterrordialog.py), and renaming to FileListErrorDialog.
Created attachment 77702 [details] [review] Patch to add signal groups and handle new project signals This patch creates a new file "signalgroup.py" which defines a SignalGroup class which can be used to manage a set of signals allowing them to be easily reconnected. It then uses this class in several ui/ files to allow the new-project signal to trigger reconnecting of signals to the project or timeline. It also refactors some duplicated signal connecting code, fixes a missed reconnection to the "not_media_file" signal in sourcefactories.py and a couple of typos in timelineobjects.py.
Created attachment 77703 [details] [review] Some miscellaneous code tidyups Sort files listed in pitivi/ui/Makefile.am into alphabetical order for easier maintenance. Add assert in sourcefactories.py (I wasn't convinced that the logic guaranteed that _newProjectCb() was always called before the current project had been replaced with a new one), and remove classic pointless comment (storemodel.clear() #clear the storemodel). Remove import in timeline.py of SimpleSourceWidget symbol, which isn't used.
Created attachment 77704 [details] [review] Updated work-in-progress patch This patch contains the changes which were contained in patch #77683, except for those covered by patches #77696, #77702, #77703. In other words, everything which I haven't yet separated into separate patches. I probably won't get a chance to do further work on this tomorrow, so thought I'd better submit this update of my current state.
Commited signal group and DiscovererErrorDialog separation patches.
Created attachment 92220 [details] [review] new patch which incorporates portions of Richard's changes but patched against SVN Split out the portion of the patch which applies to pitivi/pitivi.py and pitivi/project.py did not consider other portions of the patch, and this patch will break pitivi -- it removes signals from the Pitivi and Project objects.
marking last patches as obsolete since we already have the code in svn.
fixed and activated in 0.11.2