Created attachment 121318 [details] [review]
fast searching

Although this patch works, it unfortunately has a race condition, unless there's a way to pause anything going on inside totem during the mrl resolution.. It's non-fatal but will be more prominant on slow speed connections, it will take time to download the webpage before it can set the mrl, which may require things to be clicked twice (or more on slow connections) while it's still downloading the youtube page.

Wouldn't it be better to connect to GtkTreeSelection::changed to work out when the selected row's changed, rather than grab-broken-event?

*** Bug 557798 has been marked as a duplicate of this bug. ***

Created attachment 121359 [details] [review]
fast searching proper signal

gtk signals are syncronous, which means this should work and my hypothesis about the race condition were wrong - although with the old signal I noticed right clicking on the video immediately brings up the menu, whereas with the new one it waits for the signal to complete (and creates a noticeable lag while downloading the videos web page.) 

I switched it to treeview.get_selection().connect("changed", self.on_row_selected) and changed it to work with selections instead of the entire treeview, it does work, sorry, I tried connect("changed") before but on the treeview itself and not on get_selection() which is why it didn't work for me before. It's a trade off for fast searches and a couple extra seconds of video load time the first time the video is played.

Just tested this out, and I'm not sure about it. I don't think searches are faster enough to justify blocking the UI for a second when right-clicking, which would confuse users considerably, IMHO.

Bastien?

(In reply to comment #6)
> Just tested this out, and I'm not sure about it. I don't think searches are
> faster enough to justify blocking the UI for a second when right-clicking,
> which would confuse users considerably, IMHO.

We shouldn't be doing network IO in the UI code, to be honest. You could do the fetching of URIs in the background still, using another connection/thread, and only allow the user to activate the video when the URL has been resolved.

Search results would appear faster, but we wouldn't get the freeze when selecting the file.

 doing it in the background would be a bad idea imho. say you want to play video result 1000 after scrolling through a bunch on the list, it would take quite a long time (on the order of minutes) with a fast connection to finally actually have result 1000's page downloaded. taking into account the 999 tcp connections opened before, the http packet generated, the hd or cache seek time on the server, tcp lag, then processing the responses individually on the client, this should be expected. 

there's also the minor issue of high bandwidth waste for the small playback on click speed gain

Is it possible to cancel a video loading when the video has been clicked on?? 

If we want to avoid the lockup, my only other idea would be - when a video is started, it calls the plugin, the plugin cancels the load video inside totem itself and adds a function to be called by gtk when it's idle (perhaps in another thread), that would actually resolve the mrl, then once it has that it can tell totem to start playback. This would only solve for playback however, copy location and add to playlist would need similar reworking as well afaik.

Perhaps the method in this patch could be added as an option people could check from a configuration menu? If they don't mind the ui lockup - and perhaps the mouse pointer should have a spinner, or become the spinner, to show the user the totem is at work.

(In reply to comment #8)
>  doing it in the background would be a bad idea imho. say you want to play
> video result 1000 after scrolling through a bunch on the list, it would take
> quite a long time (on the order of minutes) with a fast connection to finally
> actually have result 1000's page downloaded. taking into account the 999 tcp
> connections opened before, the http packet generated, the hd or cache seek time
> on the server, tcp lag, then processing the responses individually on the
> client, this should be expected. 
> 
> there's also the minor issue of high bandwidth waste for the small playback on
> click speed gain

If we were to do it the way Bastien's suggested, it might be possible to only make HTTP requests for the videos currently visible, or near the visible area.

As I said in comment #6, we can't do the HTTP request when the user right-clicks, because it makes the interface too unresponsive. You might know to wait a second for the menu to appear, but I'm guessing 90% of people won't, and will click again (or worse, will think it's a bug and file bug reports).

> Is it possible to cancel a video loading when the video has been clicked on?? 

Not currently.

> If we want to avoid the lockup, my only other idea would be - when a video is
> started, it calls the plugin, the plugin cancels the load video inside totem
> itself and adds a function to be called by gtk when it's idle (perhaps in
> another thread), that would actually resolve the mrl, then once it has that it
> can tell totem to start playback. This would only solve for playback however,
> copy location and add to playlist would need similar reworking as well afaik.

A callback could be added to TotemVideoList to allow this, and I think that would be a better approach.

This has been fixed with the port of the plugin to C. It's now fully asynchronous, and URI resolution is done in a thread. The interface won't allow videos to be played until resolution is complete.