GNOME Bugzilla – Bug 586003
Previewer in the importing file chooser
Last modified: 2011-05-22 17:30:32 UTC
It would be nice to have a small previewer (and a play/pause button and slider) when using the filechooser to add clips to a project.
and also for pictures of course
Please note that Braseor already has preview in the importing file chooser for pictures, songs and video. It may be useful
yes, thank you for pointing it out
see also that brasero bug : Allow user to zoom in/out in media previewer with mouse wheel ( http://bugzilla.gnome.org/show_bug.cgi?id=588406 )
I start working on this... You can find my git branch at http://github.com/m3tr0/pitivi---preview-on-filechooser It's not complete now, but some hints from pitivi developers are much appreciate!
Hi, some quick UI feedback (from a non-dev) on your branch (you're probably already aware of those issues though): - Ideally, it should show the frame (image) of the video while paused - The slider bar doesn't work very well - We should be able to use the slider bar to seek in the video without even using the play button - AFAIK, gstreamer is able to do scrubbing with the video; for example, *while* you drag the slider in totem or pitivi's playhead, the video updates during the drag... your implementation doesn't do that, it changes the playhead position only when clicking and releasing the slider. Unless there's a performance issue at stake, it would be very nice to be able to add live "scrubbing" like that :) - When switching from one clip to another, the slider is not reset (until you click Play) - The "<b>Video</b>" text is redundant, we know it's a video :) but the resolution is nice to have. Or maybe you could add the info about audio channels for videos? (see below) - There's a lot of wasted vertical space. Maybe it could be filled with metadata, like the source list's "list view", or even more. For example, we could also have artist/title/copyright/year information, while we're at it (and this could maybe be reused in a tooltip somewhere in the source list? hmm). This becomes especially important for audio files, too. - For some reason, while it's all top-aligned for video files, it's stretched from top to bottom for audio files. I'd prefer that the two kinds of previewers are kept consistent. Overall it does work, and you did some really great job!
I've upload a new version of the code. This is a major rework that tries to fix bugs/missing features as pointed out from pitivi devs/users (thanks to Jean-François Fortin) List of changes: * Add zoom option on video playback (as in Brasero) * Show metadata informations (tags) * Improve slider usability: you can seek without play the video you can "live scrubbing" * Better use of space in preview panel TODO/BUGFIX * still some problems on pause (the image is displayed but sometimes not completely) * now all tags related to a clip are displayed in alphabetical order. This is not so useful (for example title is usually at the bottom...) but I don't know if is better to have a fix set of tags in a predefined order. Also related to this, the name of tags are defined somewhere? For example should I look for "author" or "artist" or something else?
Created attachment 161752 [details] screenshot Nice work! Big problem with the fact that you automatically use all the tags of a clip though... see this screenshot :) If it's possible to restrict this to a manually-defined set of tags, it would be better. We should only care about a handful of tags for video, and another handful of tags for audio, I think (don't know if all the files provide them consistently though... and if they don't, you need to make sure you have stuff in place to handle the failing scenarios). Also, clicking somewhere in the slider while the clip is playing yields an incorrect result (compared to clicking when it's paused, or scrubbing).
For the tags you can get the human-readable (and translated) names by using gst.tag_get_nick(tag). Would make it cleaner. You could also limit the tags by only showing those that are strings/numericals (sort using gst.tag_get_tag_type(tag)). I'd do it that way, rather than assuming that files will always produce certain tags.
Thanks for feedback! I will fix the tag problem following the Edward hints. On the bug on slider found by Jean-François: Now when you click somewhere on slider, the cursor position is increased/decreased by a page_increment that is a 10% of clip duration (I'm using the standard behaviour for gtk.Scale). I will change the code to have the same behaviour as when the clip is not playng. On play/stop issue: has someone encountered the same issue I mentioned before (i.e. when you pause the preview the image is not completely displayed)? If no maybe is a problem on my installation (I'm on ubuntu 10.4 beta)
New version of the code * Add zoom also for image preview * Better handling of tags * Controls (play/pause button slice zoom buttons) are always visible but set sensivite only when needed * Bugfix: Clicking somewhere in the slider while the clip is playing now yields a correct result Git branch http://github.com/m3tr0/pitivi---preview-on-filechooser
Ok, tested. Some more user-visible bugs: - If you zoom in (2-3x maybe), when the playback slider reaches the end, it will go "off limits". Not only this looks weird, but it also causes error messages in the terminal, such as: ** (pitivi:23122): CRITICAL **: clearlooks_style_draw_box: assertion `width >= -1' failed Also, - There is no ":" between the tags and the tag values... - Ideally, it would be good if the "zoom" level was remembered, at least for the current session (and perhaps forever). - Major bug: if you play a clip and let it finish, then hit the play button again... all the tags are duplicated (and this happens +1 each time you do it). Other than that, it's great and works as advertised (don't know if the handling of tags is perfect, but so far it's relatively satisfactory). Awesome!
Jeff, ASSIGNED means you're going to be merging the work... which I doubt considering you don't have any pitivi.org git account.
* It might be nice to have an option to disable previewing (yes, I know, but still). * Make your classable also subclass from the log.loggable.Loggable class and add some debugging statements for easier debugging. * I'm getting some weird backtraces on some files [1] * If the discovery fails, it might be good to mention the file can't be use in pitivi (and maybe come up with a reason (like the error given by discoverer). [1]: Traceback (most recent call last):
+ Trace 222485
self._finishAnalysis("EOS")
if self._emitResult():
self._emitDone(factory)
self.emit("discovery-done", self.current_uri, factory)
*args, **kwargs)
res = cb(*ar, **kw)
self.show_preview(uri)
pixbuf = gtk.gdk.pixbuf_new_from_file(gst.uri_get_location(uri))
^
I figured out why that backtrace appears. You assume that if one stream has the is_image flag set then you think it's an image. You shouldn't do that, but instead check the type of the factory. If it's a PictureFileSourceFactory then it's an image, else it isn't.
Uploaded a new version with the following differences: -fix tags duplication -fix in image detection (don't use is_image but PictureFileSourceFactory) -fix assertion `width >= -1' failed -now PreviewWidget derives also from Loggable (add also debug statements) -now PreviewWidget remembers zoom level -add option to enable/disable preview -better handling of preview errors As usual git branch on: http://github.com/m3tr0/pitivi---preview-on-filechooser
Tested It all seems good to me, except the "add option to enable/disable preview". This option is useless, it doesn't make sense not to have it activated by default. Could you remove it?
If one is missing a decoder/demuxer to preview a file, what will happen ?
Created attachment 168904 [details] screenshot when previewing fails Then it will gracefully degrade, saying that it can't preview the file (same thing if you select a file that has nothing to do with previewing, such as .log file or whatever.
about the option to enable or disable preview there is no problem for me to remove it from the code, and if you want it, please consider it already done, but I'm not so sure that is useless.... I agree it should be enable by default but, if the user simply does not want a fat filechooser widget with preview capabilities for whatever reason , than it should be possible to disable it. Just my 1 cent
I agree with Jean-François Fortin Tam : if the computer is fast enough to run PiTiVi then running media preview should no be problematic. I don't see any other reason to disable it. Then having the option to enable or disable preview seems completely useless to me. In the same way image previewer in GTK File Chooser has no "disable" option for instance
There are no downsides to having the previewer on because, from what I could test, your implementation degrades gracefully in case of failure. There is a high cost in adding options that are not 100% necessary in the preferences, so I try to keep options to a bare minimum as part of the design philosophy. If you put an option there, either it is a really important choice for the user to care about, or it is something that has huge performance repercussions. If not, it is a failure to design for "what users want". Further reference/examples: http://mystilleef.blogspot.com/2005/12/just-add-option-to-turn-it-off-or-on.html http://clarkbw.net/blog/2007/04/30/dont-do-what-i-want/ http://www.joelonsoftware.com/uibook/chapters/fog0000000059.html http://www.codinghorror.com/blog/2006/09/unnecessary-dialogs-stopping-the-proceedings-with-idiocy.html http://clarkbw.net/blog/2009/05/14/negotiate-with-your-users/
Sorry for the delay... My silence was not because I didn't want to remove the preference (you have convinced me :-) )but because my notebook end it's life on monday 30 august... I hope to have a new machine in this week-end. I will post the new version asap
Uploaded a new version with the following differences: - remove option to enable/disable preview. Preview is active by default As usual git branch on: http://github.com/m3tr0/pitivi---preview-on-filechooser Sorry again for the delay....
The code is definitely mergeable quality... but we don't have time for this release. We'll be releasing again within a month, this should go in then.
As per last comment, can someone merge this before we get into the freeze again?
Created attachment 186834 [details] Image displaying over-sized file chooser window
There needs to be some limit put on the size of the meta-data, because otherwise it causes the preview dialog to grow quite large for certain files.
Also get several of these warnings: pitivi/ui/filechooserpreview.py:426: GtkWarning: Failed to set text from markup due to error parsing markup: Error on line 9: Entity did not end with a semicolon; most likely you used an ampersand character without intending to start an entity — escape ampersand as &
This has now been merged in master, with some additional changes to polish it and fix some bugs. Thanks a lot, Pier! (see also bug 586003 or bug 650807 if you want to take it a bit further)
Um, I meant bug #650806 and bug #650807.