-
Notifications
You must be signed in to change notification settings - Fork 93
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
Implement predictive tuning using multiple tuners #23
Conversation
Many thanks. Will take a closer look asap (next days probably). Would you mind rebasing to latest pvr.tvh master? |
Please remove the German strings.po from the commit. Translation to any other language than en_gb will be done later using Transifex. |
Are you familiar with the htsp spec? https://tvheadend.org/projects/tvheadend/wiki/Htsp Might be helpful when thinking about subscription priorities... |
What do you think about the possibility to define "some" favourite channels in the addon settings and to try to prefetch as many as possible/configured. Just thinking out loud... |
I think this should be something for the tvheadend side, not for the addon, but that's my opinion.. |
Thanks for the pointer with the subscription weight. I'll have a look into that, actually shouldn't be too hard to implement. Having some favourite channels would be really nice, but I think that is something for a next iteration.
I actually thought about that, but I don't agree. Having the data streamed to Kodi makes it available immediately on channel switch. This avoids buffering in DVDPlayer, and allows immediate playback. When using fast tuners, the time for buffering is non-negligible compared to the time TVHeadend needs to tune on the channel and deliver that data. |
What would be even better, is if this feature had some other way to be re-triggered. In a predictive manner. For example: when I pick up my air mouse remote, or mobile phone, well that aught to triggers it's accelerometer. And if Kodi is in the pvr menus at the time. And Kodi is being informed about that event. We can be reasonably confident that the user may be intending to change the pvr state somehow. So that also would be the ideal time to start buffering alternative channels... What you may need for that is perhaps specific kind of API interaction between the main Kodi and it's pvr addons. e.g. a some generic kodi-wide notification that can be subscribed to by our pvr.hts addon. Of course. That ^ above example may not be possible today. However a generic 'recent activity' notification would not be limited purely just to accellerometer remote input. It could also detect regular keypresses. Or potentially come from other kinds of future support too. e.g. Microsoft Kinect etc. Or even broader than that. For current users, just regular remotes (no special fancy input support). Then the trigger can be just when any remote key has been pressed while within the pvr addon's menus. Including back, menu, select, 'info, epg buttons etc. |
It would be better to have predictive muxes instead of channels. And with with timeshift? if all streams will use timeshifting, your system gets stressed a lot with no reason. |
For switching to happen quickly it has to be streamed to Kodi continuosly. Many devices are limited to 100 Mbit/s so streaming more than a few channels at a time becomes unpractical. I like this idea and I think we should try to start out simple, i.e. a setting where you can choose to "buffer" previous/next/both previous and next channels. Quick sequential zapping is the main use case and that's what we should focus on initially. |
I fully agree to what @Jalle19 said. It's a cool feature with lots of potential for future enhancements, imo. Let's start simple, guys, and let's see where the journey will lead us to. ;-) The only drawback I see is risk to take your network and tvh itself to the limits, but this is just user fault (wrong configuration) - it' a power user feature, right? And it can be fully failed, that's important. And default should be disabled (which it is with the current implementation). I also agree that this is not a back-end topic. Implementing it at Kodi-side is the right approach, I think. Btw, I'm on the loser side here as I only have two tuners... |
a/failed/disabled |
For me, the one thing that is missing for now is to do all predictive subscriptions with very low priority to make sure no tuners are blocked for other clients that want to do a real subscription and there must be some resources available for tbheadendv itself to do background mux and epg scanning and such... |
Typing correct sentences on the phone is pita, isn't it? |
I've updated the patch to implement subscription weight management. For the subscription currently watched it now explicitly uses a weight of 150, which in the latest TVHeadend is the implicit default. For predicted channels it uses a weight of 50, and for channels zapping away from a weight of 40. Timeshifting has not been a problem in my tests, even with four and more active subscriptions. I actually think it is a great feature, as you can switch between channels, and do time-shifting on channels that you haven't watched (say you missed switching back on time to your movie after the commercials). Also, newer TVHeadend versions support timeshifting to memory buffers, which avoids heavy load on your TVHeadend server disk. Further, I agree that a first version of this functionality should be simple, more or less the feature-set we have. While there are many ways we could improve prediction, the current patch covers the main use cases and is rather simple. |
@@ -191,6 +208,15 @@ void CHTSPDemuxer::Speed ( int speed ) | |||
SendSpeed(); | |||
} | |||
|
|||
void CHTSPDemuxer::Weight ( uint32_t weight ) | |||
{ | |||
CLockObject lock(m_conn.Mutex()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd move this lock to CHTSPDemuxer::SendWeight
to keep the lock scope as small as possible (that's how we do it in most other places).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, done.
Thanks for reviewing the patch. I've pushed a new version, comments above. |
Thanks, I'll do some runtime testing later. |
I'm testing it and seems to work well! |
#define DEFAULT_HTSP_PORT 9982 | ||
#define DEFAULT_CONNECT_TIMEOUT 10 | ||
#define DEFAULT_RESPONSE_TIMEOUT 5 | ||
#define DEFAULT_PRETUNERS 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps we should change this setting to TOTAL_TUNERS instead since that's what it actually represents, right? I have a feeling someone is going to be confused and set this value to zero because they only have one tuner.
@martinwilli if you could take care of my comments and rebase that would be great! |
Thanks for the review. I've rebased the original patch, and addressed the mentioned issues. I've pushed individual commits for review, but of course can squash these into the original commit for a merge. |
@martinwilli Can you make *.deb packages with this patch? |
@martinwilli could you please rebase against latest master? |
@martinwilli and please squash down to a single commit. |
If multiple tuners are available in TVHeadend, and the addon is configured to use them, this patch tries to anticipate the users zapping behavior, and have the channel tuned and buffered before the channel switch happens. This allows almost instant channel switches if the prediction was correct. When using the default of a single tuner, the tuning behavior does not change. If two tuners are configured, sequential up/down-zapping pre-tunes the next channel in order; for direct channel selection the previously tuned channel is kept to allow fast channel swapping. If more tuners are configured, previously used channels are kept open to allow up/down tuning in any order. The plugin uses a configurable timeout to close a predicted or previously used channel if it does not get used, defaulting to 10s. It can be disabled by configuring 0. For subscribing to the currently watched channels, an explicit weight of 150 is used, which is the implicit default previously used. For predicted channels a weight of 50 is used, while the channels zapping away from use a weight of 40.
Done. Looking forward to finally get this merged. |
Implement predictive tuning using multiple tuners
@ksoo I think this has broken the cat kodi.log
12:27:22 T:140614899762944 NOTICE: PVRManager - starting up
12:27:22 T:140614242907904 NOTICE: Thread PVRManager start, auto delete: false
12:27:22 T:140614259693312 NOTICE: Thread PVRClient start, auto delete: false
12:27:22 T:140614251300608 NOTICE: Thread BackgroundLoader start, auto delete: false
12:27:22 T:140614242907904 ERROR: SQL: Abort due to constraint violation
Query: INSERT INTO channels (iUniqueId, bIsRadio, bIsHidden, bIsUserSetIcon, bIsUserSetName, bIsLocked, sIconPath, sChannelName, bIsVirtual, bEPGEnabled, sEPGScraper, iLastWatched, iClientId, idEpg) VALUES (301732504, 1, 0, 0, 0, 0, 'http://id:sn00zey0ul00se@192.168.2.3:9981/1_0_C_2922_90B_3B_11A0000_0_0_0.png', 'Freesat Info', 0, 1, 'client', 0, 15641, -1)
12:27:22 T:140614242907904 ERROR: CommitInsertQueries - failed to execute queries
12:27:22 T:140614242907904 ERROR: SQL: Abort due to constraint violation
Query: INSERT INTO channels (iUniqueId, bIsRadio, bIsHidden, bIsUserSetIcon, bIsUserSetName, bIsLocked, sIconPath, sChannelName, bIsVirtual, bEPGEnabled, sEPGScraper, iLastWatched, iClientId, idEpg) VALUES (301732504, 1, 0, 0, 0, 0, 'http://id:sn00zey0ul00se@192.168.2.3:9981/1_0_C_2922_90B_3B_11A0000_0_0_0.png', 'Freesat Info', 0, 1, 'client', 0, 15641, -1)
12:27:22 T:140614242907904 ERROR: CommitInsertQueries - failed to execute queries
12:27:22 T:140614242907904 ERROR: SQL: Abort due to constraint violation
Query: INSERT INTO channels (iUniqueId, bIsRadio, bIsHidden, bIsUserSetIcon, bIsUserSetName, bIsLocked, sIconPath, sChannelName, bIsVirtual, bEPGEnabled, sEPGScraper, iLastWatched, iClientId, idEpg) VALUES (301732504, 1, 0, 0, 0, 0, 'http://id:sn00zey0ul00se@192.168.2.3:9981/1_0_C_2922_90B_3B_11A0000_0_0_0.png', 'Freesat Info', 0, 1, 'client', 0, 15641, -1)
12:27:22 T:140614242907904 ERROR: CommitInsertQueries - failed to execute queries
12:27:22 T:140614242907904 ERROR: CPVRChannelGroups - Load - failed to load channel groups
12:27:22 T:140614242907904 ERROR: PVRManager - Process - failed to load PVR data, retrying
12:27:22 T:140614603630336 ERROR: AddOnLog: Tvheadend HTSP Client: pvr.hts - failed to read packet (Bad file descriptor)
id@emachines-e520:~/.kodi/temp$ |
@dreamcat4 I believe we are in alpha phase, so you have to expect difficulties when dealing with bleading edge. However, the log fragment you posted contains no hint that the problem is in any way related to this feature. And, no the feature is not turned on by default. => https://github.com/kodi-pvr/pvr.hts/blob/master/pvr.hts/resources/settings.xml#L18 (total_tuners =1) |
I've seen that error before, it's not related to this PR but it's interesting nontheleess. I'm not sure if Kodi screws up (likely) or if tvheadend can somehow use the same ID for multiple channels. |
[EDIT] Select: LiveTV --> General --> Clear Data And then it will reload / re-import all of the PVR channels list and EPG data from scratch. Problem solved. |
If multiple tuners are available in TVHeadend, and the addon is configured
to use them, this patch tries to anticipate the users zapping behavior, and
have the channel tuned and buffered before the channel switch happens. This
allows almost instant channel switches if the prediction was correct.
When using the default of a single tuner, the tuning behavior does not change.
If two tuners are configured, sequential up/down-zapping pre-tunes the next
channel in order; for direct channel selection the previously tuned channel
is kept to allow fast channel swapping. If more tuners are configured,
previously used channels are kept open to allow up/down tuning in any order.
The plugin uses a configurable timeout to close a predicted or previously used
channel if it does not get used, defaulting to 10s. It can be disabled by
configuring 0.
I have plenty of free tuners in my TVHeadend server, so I'd like to make better use of them. Im no C++ expert, nor have I any experience with the Kodi codebase, so let me know if you think something can be done simpler or cleaner. Also let me know if you want me to split up the patch to smaller changes.
The tuning prediction is currently rather simple and certainly can be improved, but it works actually very fine here for a first version.
One of the tricky points is to Trim() the input buffer correctly on anticipated/unused channels, so we don't waste memory and have just enough data so DVDPlayer doesn't need buffering. I currently use a fixed count of packets; maybe we could improve this by buffering more or less data depending on the bitrate of the channel.
We also have some room for improvements regarding subscription priority; not sure how difficult it is, but we could use a lower priority for predicted/unused streams to not interrupt other frontends using TVHeadend.