[opdenkamp]

I've started cleaning up the PVR API. This will be a breaking change, so if you're developing PVR add-ons, make sure you implement the necessary changes as soon as the new API is implemented. I'll do the changes in the add-ons that are currently in the tree myself.

This is a list of changes to come:

• all data structures will be changed: members names will be changed to match their equals on XBMC's side and currently unused members will be removed.
  
• currently unused or unneeded methods will be removed.
  
• some method names and types will be changed to match their names on XBMC's side (e.g. bouquet -> channelgroup)
  
• timezone conversions will be removed: all times will have to be provided as UTC.
  
• the EPG and PVR APIs will be split up. I'll split up the implementations later.
  

Some methods that are removed now will probably be reimplemented later, but not before PVR has been merged with mainline. We need a stable and clean API now, to prepare for the binary add-on framework and the merge with mainline.

I'll do the changes in a separate branch first, which will probably be on github later this week end.

[opdenkamp]

I'll split up the EPG interface later, when doing the changes for binary add-ons (when they're here, said a lot of times now ;-))

this is the result now: https://github.com/opdenkamp/xbmc/compar...ce-cleanup

still got to fix the times, as they are now all utc. they still have to be converted to xbmc's local time.

add-on authors are welcome to comment :-)

[opdenkamp]

not getting any comments here, on github or on irc, so I'm pushing this to the main pvr tree.

the changes have been tested with tvheadend and I've done my best to do the changes in the other add-ons too, but I'm sure pretty sure I've missed a thing or two somewhere.

if you're an add-on author and find out that your add-on is broken after this merge, check these things:

• are all methods defined in xbmc/addons/include/xbmc_pvr_dll.h implemented?
  
• your add-on has to send all times as UTC and will receive all times as UTC. I haven't removed localtime conversions in other add-ons than tvheadend so you might have to remove some code.
  
• method parameters have changed. I've changed most things in other add-on than tvheadend, but you have to check the usage of the unique channel id param. some add-ons were using a "client index" before, but that's no longer part of the interface as it is unneeded.
  

[margro]

I've seen your changes and (without testing) I do not expect much problems to fix my MediaPortal addon. Probably only the removal of the UTC to local time correction.

As you are currently changing the API, I would like to propose a minor change (actually more a bugfix) to the PVR_TIMER struct for the addition of new timers.
Currently, the startTime and endTime fields are corrected for the "start margin" and "end margin" settings. The addon and the backend do not "know" this correction. As a result, MediaPortal uses the EPG title from the previous program to store the recording.

The only way to solve this is to be able to read the global XBMC PVR settings from within the addon.
Another option is to add a "marginStart" and "marginStop" field to the PVR_TIMER struct (and not to correct the real start and end times).

See my Dharma-pvr modification for the idea: margro/Dharma git rev 521037e0336759914f02c543be29272f64a52ce8

[opdenkamp]

another minor change to the channel group structs:
ids are removed and names should be set instead (or we would get trouble with multiple backends)

Code:

typedef struct PVR_CHANNEL_GROUP
  {
    const char * strGroupName;         /*!< @brief (required) name of this channel group */
    bool         bIsRadio;             /*!< @brief (required) true if this is a radio channel group, false otherwise. */
  } ATTRIBUTE_PACKED PVR_CHANNEL_GROUP;

  typedef struct PVR_CHANNEL_GROUP_MEMBER
  {
    const char * strGroupName;         /*!< @brief (required) name of the channel group to add the channel to */
    unsigned int iChannelUniqueId;     /*!< @brief (required) unique id of the member */
    unsigned int iChannelNumber;       /*!< @brief (optional) channel number within the group */
  } ATTRIBUTE_PACKED PVR_CHANNEL_GROUP_MEMBER;

@margro:
I'll add the margins to the struct.

[CrashX]

I've seen your changes and (without testing) I do not expect much problems to fix my MediaPortal addon. Probably only the removal of the UTC to local time correction.

As you are currently changing the API, I would like to propose a minor change (actually more a bugfix) to the PVR_TIMER struct for the addition of new timers.
Currently, the startTime and endTime fields are corrected for the "start margin" and "end margin" settings. The addon and the backend do not "know" this correction. As a result, MediaPortal uses the EPG title from the previous program to store the recording.

The only way to solve this is to be able to read the global XBMC PVR settings from within the addon.
Another option is to add a "marginStart" and "marginStop" field to the PVR_TIMER struct (and not to correct the real start and end times).

See my Dharma-pvr modification for the idea: margro/Dharma git rev 521037e0336759914f02c543be29272f64a52ce8

How does this work if you have only one tv tunner and two shows are recording back to back ?

[quintesse]

How does this work if you have only one tv tunner and two shows are recording back to back ?

I'd say that there is no problem when they're on the same channel, but when they're on different channels (which would most likely be the case) one will have to "win", you either run the risk of losing the end of one recording or the beginning of the other.

Because of you decide to not take into account the margins for back-to-back recordings you run risk of losing BOTH the end AND the beginning of the two recordings!

On the other hand if your channel providers have good EPG data and almost never have under/overrun of their programs and you have the margins "just to be sure" it might actually be better to just change the moment the next program starts.

It not simple to come up with something that always works

[opdenkamp]

I'd say that there is no problem when they're on the same channel, but when they're on different channels (which would most likely be the case) one will have to "win", you either run the risk of losing the end of one recording or the beginning of the other.

you can give a priority to recordings, so when you only have 1 tuner on your backend, one of them will "win". if both have the same priority, the second recording will only start after the first recording finished (+margin). at least that's how tvheadend handles situations like that. can be different for other backends though.

Because of you decide to not take into account the margins for back-to-back recordings you run risk of losing BOTH the end AND the beginning of the two recordings!

margins should be handled by the backend itself if it's supported. if not, the add-on should substract and add the margins to the begin and end time.

the only thing that has been changed is that the margins aren't added and substracted by xbmc anymore, but in the add-on or backend.

[margro]

margins should be handled by the backend itself if it's supported. if not, the add-on should substract and add the margins to the begin and end time.

the only thing that has been changed is that the margins aren't added and substracted by xbmc anymore, but in the add-on or backend.

Exactly. The MediaPortal TVServer has already its own start and stop margins. As different backends can have different capabilities, its up to the PVR addon to use or ignore the XBMC margins. The problem upto now is that the PVR addon cannot ignore the XBMC margin settings, because XBMC already incorporates them in the start and end times of the timer.

For MediaPortal, I can provide a custom margin for every timer, but it is up to the MediaPortal TVServer to handle that in a good way (e.g. with priorities).
My current implementation (in my Dharma builds) is that when XBMC has margins of 0 (zero), use the Mediaportal TVServer default margins, otherwise use the supplied margins from XBMC.

[opdenkamp]

implemented it in the dev branch too yesterday: https://github.com/opdenkamp/xbmc/commit...26427a26b6

[Red_F]

Ah nice to see that the API changes are finalising. I'll need to retrieve some of the ideas I jotted down while doing the ForTheRecord development. Are you still open to suggestions?

[opdenkamp]

I'm open to suggestions, but the api won't be changed (much) anymore before binary add-ons are implemented. spiff is looking into that at the moment.

[opdenkamp]

I added one more method on the interface, part of the new channel group transfer part:

Code:

/*!
   * @brief Ask the PVRManager to refresh it's channel groups list.
   * @param addonData A pointer to the add-on.
   */
  static void PVRTriggerChannelGroupsUpdate(void *addonData);

[CrashX]

@dushmaniac

Are you getting enough information from PVR addon to be able to lookup via scrapers so that you can get full metadata for it for XBMC library ?

For example, mythtv pvr addon provides program id as EP007322830110 ..

[opdenkamp]

that wouldn't fit in any structure on the current interface. what does it represent? it doesn't match anything in the current htsp interface from tvheadend.

could you define "enough information" for me please.