Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PVR] Fix for slow metadata updates of recordings #6085

Merged
merged 2 commits into from Jan 3, 2015

Conversation

popcornmix
Copy link
Member

See: http://forum.kodi.tv/showthread.php?tid=210774

metaron identified that entering the recordings screen wih 850 recordings on a Pi
was taking about 40 seconds. The database is opened and closed for every recording,
and this dominates the time taken. With the database open/close pulled out of the loop
it now takes 1.4 seconds.

@popcornmix
Copy link
Member Author

jenkins build this please

@FernetMenta
Copy link
Contributor

great! there is an issue with vdr 2.0.3 which triggers recordings update frequently.

@popcornmix
Copy link
Member Author

ping @opdenkamp

@opdenkamp
Copy link
Member

nice find. i think we can keep an open db instance in CPVRRecordings, so we don't need to open and close it in CPVRRecordings::GetSubDirectories?

@popcornmix
Copy link
Member Author

@opdenkamp Open database in constructor and close in destructor, and just rely on it being open within this class?

@opdenkamp
Copy link
Member

yeah afaict that should be okay too

See: http://forum.kodi.tv/showthread.php?tid=210774

metaron identified that entering the recordings screen wih 850 recordings on a Pi
was taking about 40 seconds. The database is opened and closed for every recording,
and this dominates the time taken. With the database open/close pulled out of the loop
it now takes 1.4 seconds.
@popcornmix
Copy link
Member Author

@opdenkamp updated to only open/close the database once with the CPVRRecordings class.

Note: I don't have a setup to test this (it does compile).
It will be out in a newclock4 test build tonight, but if anyone who uses PVR/recordings can test this doesn't break anything, that would be helpful.

@@ -40,6 +41,8 @@ namespace PVR
PVR_RECORDINGMAP m_recordings;
unsigned int m_iLastId;
bool m_bGroupItems;
CVideoDatabase m_db;
bool m_dbLoaded;

This comment was marked as spam.

@janbar
Copy link
Contributor

janbar commented Jan 2, 2015

@popcornmix I will test it asap tomorrow.

@xhaggi xhaggi added Type: Fix non-breaking change which fixes an issue Component: PVR labels Jan 2, 2015
@popcornmix
Copy link
Member Author

@Jalle19 updated with your suggestions

@Jalle19
Copy link
Member

Jalle19 commented Jan 3, 2015

Thanks, looks good. @xhaggi any thoughts? The change should be pretty harmless so I don't think a merge is that far away.

@opdenkamp
Copy link
Member

jenkins build this please
feel free to merge once @janbar has tested it

@janbar
Copy link
Contributor

janbar commented Jan 3, 2015

Tested (including Jalle19 fix) and approved !

@xhaggi
Copy link
Member

xhaggi commented Jan 3, 2015

Does it make sense to move UpdateMetadata() to CPVRRecordnings and pass the recording as parameter instead of the database reference?

@opdenkamp
Copy link
Member

could do, but let's do that separately and get this issue fixed first.

@xhaggi
Copy link
Member

xhaggi commented Jan 3, 2015

it will fix the issue too and we don't introduce code that we will change later. if fine for @popcornmix

@opdenkamp
Copy link
Member

i don't see the benefit tbh

Jalle19 added a commit that referenced this pull request Jan 3, 2015
[PVR] Fix for slow metadata updates of recordings
@Jalle19 Jalle19 merged commit c026452 into xbmc:master Jan 3, 2015
@Jalle19
Copy link
Member

Jalle19 commented Jan 3, 2015

Goddammit, I forgot it was unsquashed.

@popcornmix popcornmix deleted the pvr_recordings branch January 3, 2015 11:11
@MartijnKaijser MartijnKaijser added this to the Helix 15.0-alpha1 milestone Jan 3, 2015
phil65 pushed a commit to phil65/xbmc that referenced this pull request Jan 10, 2015
[PVR] Fix for slow metadata updates of recordings
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: PVR Type: Fix non-breaking change which fixes an issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants