fetzerch / xbmc-pvr-addons

XBMC PVR add-ons
GNU General Public License v3.0
19 stars 17 forks source link

Implementing the MythTV 0.27 features of the recordedseek table #135

Closed nerddtvg closed 11 years ago

nerddtvg commented 11 years ago

Looking up millisecond offsets to closest frame marks during commercial and cutlist skipping.

Reference: http://forum.xbmc.org/showthread.php?tid=131016&pid=1513174#pid1513174

janbar commented 11 years ago

Hi nerddtvg,

Make a particular attention of this table, recordedseek. It is the bigger table of myth database and count millions rows for lot of users. We must access it using index path to keep low cost for queries and try to order data using index instead scanning. The commit 97f2ea56e2b2bb4dee50d36f9a0d34165dbce5bb from repo https://github.com/janbar/xbmc-pvr-addons.git, branch internal_edl, shows you how to query the table to retrieve offset near a mark for given type. The function cmyth_mysql_get_recording_seek_offset() implements what you need:

int8_t cmyth_mysql_get_recording_seek_offset(cmyth_database_t db, cmyth_proginfo_t prog, int64_t mark, int64_t psoffset, int64_t nsoffset)

It returns mask of found values: previous offset (poffset) and next offset (noffset) around given mark for type 9.

You can use it adding a new parameter "type" to specify any other type than 9. So the new spec could be:

int8_t cmyth_mysql_get_recording_seek_offset(cmyth_database_t db, cmyth_proginfo_t prog, int16_t type, int64_t mark, int64_t psoffset, int64_t nsoffset)

Then i think we must check db version (>=1309) to avoid unnecessary db call: see MythDatabase::GetSchemaVersion(). And sure for >=1309 , fallback using framerate if no mark 33 yet exist (then returned mask is 0).

Let me know

janbar commented 11 years ago

@nerddtvg The commit 24229dcd4feb6fd893cd17230408bd0625117671 add wrapper for cmyth_mysql_get_recording_seek_offset() in the addon. Just you need to add new parameter type to call for type 33.

nerddtvg commented 11 years ago

Thank you for that information. As billed it is just a hack. I will review the code with what you said later this evening and make adjustments. I realize it gets big which is why using the chanid and start time with a restrictive WHERE clause would hopefully keep the indices in use and make for fast searching. On Sep 24, 2013 4:29 PM, "Jean-Luc Barrière" notifications@github.com wrote:

@nerddtvg https://github.com/nerddtvg The commit 24229dchttps://github.com/fetzerch/xbmc-pvr-addons/commit/24229dcd4feb6fd893cd17230408bd0625117671add wrapper for cmyth_mysql_get_recording_seek_offset() in the addon. Just you need to add new parameter type to call for type 33.

— Reply to this email directly or view it on GitHubhttps://github.com/fetzerch/xbmc-pvr-addons/pull/135#issuecomment-25044173 .

nerddtvg commented 11 years ago

Jean-Luc,

I reworked my code to be based upon the commits you referenced me. I did find a problem in the cmyth_mysql_get_recording_seek_offset function at the bottom where it determines whether to use next/prev_offset. The values for mask == 1 and mask == 2 are swapped.

I did include the type in searching, but also provided an overloaded MythDatabase::GetRecordingSeekOffset which defaults to type = 9 if one is not provided.

Four important verification steps in the code:

  1. Checks DB schema to version 1309 or higher, per your suggestion. My current version is 1317 with Myth 0.27 so that may need to go up if 1317 is the level we should be at. (This is implemented in the commercial skip code, not the cmyth code as cmyth has a variable type.)
  2. If start mark is zero, it saves that DB time by assuming zero for the offset
  3. If the retval for either start or end offset searching is an error, it ignores that entire break and falls back to static frame rates. This could possibly just fall back for that one value.
  4. In a couple of my test programs I had some invalid offsets which had the end frames with a smaller offset than the start frames. This can not happen and will fall back to static frame rates again.

I do have a lot of logging built into this because it was rough trying to determine at which step it failed. That should be stripped out if going forward with the code.

https://github.com/nerddtvg/xbmc-pvr-addons/commit/9f2d5def1b79cdbc18cb10dc1e0105696161ce4b

I hope this one is up to snuff. If so I will drop this pull request and reapply with the new commit.

Drew

fetzerch commented 11 years ago

@nerddtvg First of all, thanks for your contribution

There's no need to drop the pull request. Just add commits (or change the current one with git commit --amend) and force push to the remote branch. That way this PR just gets updated and we keep all the history.

nerddtvg commented 11 years ago

@fetzerch Thank you for that information. I don't know if I did it now or it happened when I pushed again last night, but there are two commits showing up in this PR. I'm sorry for my ignorance on this, I've only ever used git for patch tracking, not submissions.

I'm hoping that if this works out I'll attempt to contribute more patches in the future. I really appreciate this project and would love to see it get more fleshed out, if it is in my ability to do so.

janbar commented 11 years ago

Thanks nerddtvg,

Can i push a PR to your repo ? I will explain you how to deliver commits to the projects. Currently we have 2 sources: cmyth and mythtv-cmyth. We need separated commits for each of them. You will see my PR and you could apply it in your local branch and see my comments. Then if you agree with it you could push again to your public branch. There this PR will be updated with new commits. JLB

nerddtvg commented 11 years ago

Go right ahead. On Sep 26, 2013 4:38 AM, "Jean-Luc Barrière" notifications@github.com wrote:

Thanks nerddtvg,

Can i push a PR to your repo ? I will explain you how to deliver commits to the projects. Currently we have 2 sources: cmyth and mythtv-cmyth. We need separated commits for each of them. You will see my PR and you could apply it in your local branch and see my comments. Then if you agree with it you could push again to your public branch. There this PR will be updated with new commits. JLB

— Reply to this email directly or view it on GitHubhttps://github.com/fetzerch/xbmc-pvr-addons/pull/135#issuecomment-25154839 .

fetzerch commented 11 years ago

replaced by #138