joolswills / plugin.video.iplayer

This plugin is broken.
10 stars 4 forks source link

Slow refreshing of programme list - sometimes it simply won't work #126

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Select Popular programmes
2. Select any channel
3. Try start TV programme

What is the expected output? What do you see instead?

The waiting circle goes round.  If the stream fails, which is often does the 
first time around, then it has to reload the entire programme listings again. 

This only started happening after the latest update/release.  And I feel it 
something to do with the resume feature.

But those of us accessing over a slow internet speed have now found the recent 
update to affect performance extremely badly.

Couldn't there be an option to turn off the resume feature entirely.

And stop it always trying to refresh the channel listing even if playing the 
stream failed?

It seems very  unforgiving on a slower internet speed now.  The version before 
worked very well.

I would ask that certain new features could be turned off.  And I've played 
with the settings making some 0 to try  make sure they are off.

To no avail. If the stream does fail then you can be waiting up to 1 minute for 
the programme list to  refresh and then you can try  again.

It was easier when if it failed you simply tried again straight away, this time 
the stream worked.

Debug log output (http://pastebin.com/)

What version of the plugin are you using? On what OS?

XBMC, Raspberry Pi, Version: 2.4.18

Please provide any additional information below.

I'm happy to test, debug and do anything needed to help make this better for 
those without a 10mb internet connection.

I know it worked fine before but 2.4.18 improvements have made it worse.  Than 
2.4.17.

Many thanks,

Adam

Original issue reported on code.google.com by adskirem...@gmail.com on 6 Aug 2013 at 5:48

GoogleCodeExporter commented 9 years ago
I don't have the slow update problem as far as I know but my kids use this to 
watch cbeebies shows and frequently replay the same show and get frustrated 
when it skips to the end (which is possibly a bug in itself - surely it should 
remove the resume point when it gets to the end of the stream). I'd like to be 
able to disable resume in a config file as it provides no benefit to me and 
annoys my children.

Original comment by tony.mcc...@gmail.com on 20 Oct 2013 at 5:35

GoogleCodeExporter commented 9 years ago
I have the slow update problem too  - takes about 20 seconds to refresh a list 
of 5-6 programmes. Kids are getting frustrated and impatient. I have a 100mb 
internet connection (supposedly).

Original comment by malc...@congreve.com on 20 Nov 2013 at 9:25

GoogleCodeExporter commented 9 years ago
I've looked at this.  And I don't think there's anything that can be done
with slow loading on refresh.  Even on internet connection it's still
dependent on iPlayer itself.

But a xbmc left menu for refresh and an option to turn it off (so you could
manually refresh) would be better perhaps?

tbh I've not gone into the code much but maybe I should, hopefully find a
reasonable solution.

I'll post anything I do.

Original comment by adskirem...@gmail.com on 21 Nov 2013 at 3:40

GoogleCodeExporter commented 9 years ago
This is an issue with the new resume feature, nothing to do with internet 
speed. I've done a fix (amongst other things) but haven't submitted a clean 
patch as yet - looking for comments and some more free time first! See my post 
in the forums if you want to download an amended version to test.

Original comment by dougpi...@gmail.com on 20 Dec 2013 at 3:43

GoogleCodeExporter commented 9 years ago
please submit a patch here, against the svn version as there has been changes 
since the last release. thanks. Looking at your post it looks like you cover a 
few issues, so please split into multiple patches if addressing more than one 
issue and open a new ticket.

http://forum.xbmc.org/showthread.php?tid=51322&pid=1577550#pid1577550

Original comment by exob...@gmail.com on 20 Dec 2013 at 3:51

GoogleCodeExporter commented 9 years ago
Patch to fix resume performance attached.

Original comment by dougpi...@gmail.com on 21 Dec 2013 at 9:22

Attachments:

GoogleCodeExporter commented 9 years ago
thanks - please can you go over the patch a bit - in regards to the scope 
change of the variables and the functionality so I have a clear understanding 
of the changes. I shall include Slastair in this as he was the author of the 
resume functionality, and I would prefer him to sign this off and include it 
with any changes etc.

Thanks for the patch :)

Original comment by exob...@gmail.com on 21 Dec 2013 at 9:34

GoogleCodeExporter commented 9 years ago
Ok, no problem.

Previously there was a bit of a mix - the resume file contents were loaded and 
stored in instance variables when an instance of IPlayer was instantiated. In 
most cases however, the resume details were accessed by a call to static method 
load_resume_file. This method would read the file from disk, parse it and 
return the results.

To keep things consistent with a single point of access to the file, I've 
removed the class variables and everything now calls load_resume_file when it 
needs access to the resume details.

To address the performance, load_resume_file now only actually reads the file 
off disk on the first call, then stores the results in class (static) variables 
and returns that on subsequent calls. This method is called from add_programme, 
which is called once for every entry in a program list. Previously this meant 
reading the file and parsing it up to a couple of dozen times just in the 
course of displaying a program list. Coupled with the writing loop issue that 
caused huge files, this made performance completely unusable on my system 
(openelec running off a USB stick).

All the file writes update the cached copy but still write to disk as well in 
order to ensure that the file is always up to date.

Original comment by dougpi...@gmail.com on 21 Dec 2013 at 11:59

GoogleCodeExporter commented 9 years ago
The changes seem sensible. Two points:

We can't use with open(IPlayer.RESUME_FILE, 'rU') as resume_fh: ... as the xbox 
version still uses python 2.4 as far as I'm aware, and this will cause a syntax 
error.

For this part:

@@ -1221,10 +1230,13 @@
         Intended to replace xbmc.Player.play(playlist), this method begins playback and seeks to any recorded resume point.
         XBMC is muted during seeking, as there is often a pause before seeking begins.
         """
+
+        resume, dates_added = IPlayer.load_resume_file()
+
         if os.environ.get( "OS" ) != "xbox":

Can the loading of the resume file be moved inside the check that we're not on 
xbox? There should be no resume related functionality on that platform.

Original comment by alastair...@gmail.com on 29 Dec 2013 at 6:35

GoogleCodeExporter commented 9 years ago
I think we should have a global config for resume support that is configurable, 
that is defaulted to false for xbox, and other users with issues can choose 
from settings. what do you think ? btw xbmc4xbox will have python 2.7 very 
shortly, as soon as I have pushed out the new version, but as of now I try to 
keep python 2.4 compatibility if possible (in the short term only)

Original comment by exob...@gmail.com on 30 Dec 2013 at 12:14

GoogleCodeExporter commented 9 years ago
If the xbox is going to become compatible shortly, I'd probably suggest leaving 
things as they are for now, then removing the xbox specific checks at that 
point. Resume is definitely useful, and the extra options I've added in my 
other change hopefully remove the annoyance factor for those that prefer it to 
start from the beginning every time.

Original comment by dougpi...@gmail.com on 3 Jan 2014 at 2:44

GoogleCodeExporter commented 9 years ago
Alastair, fair point on the 'if' clause, updated patch attached.

Original comment by dougpi...@gmail.com on 3 Jan 2014 at 2:54

Attachments:

GoogleCodeExporter commented 9 years ago
added in r146. thanks very much for the patch. note I changed line endings of 
the project to UNIX style LF from the previous CRLF (windows), as patch was 
complaining and I prefer LF (and xbmc uses LF etc).

Original comment by exob...@gmail.com on 3 Jan 2014 at 3:54

GoogleCodeExporter commented 9 years ago
due to order of me applying patches, and the movement of the line it required 
some fixes. see r150 (resume was set below a resume.keys check so I re-ordered 
the logic a bit)

Original comment by exob...@gmail.com on 3 Jan 2014 at 4:21