DVDDemuxPVRClient vs timeshift

(2012-11-04 22:17)FernetMenta Wrote:  I am not happy with all those quick hacks. This makes the API and design decay before it has even started. Most of the new addons do still struggle with the basics, so why implementing a new unstable feature?

I'm not sure which addons you're referring to, but since the NextPVR plugin is working well and other addons have started implementing timeshift, my users are pushing hard to have this feature included. At this stage XBMC is very much a moving target though, and not clear which direction I should take my addon to ensure I can give my users both fast channel changes and timeshift.

Quote:If you are that in a hurry, why does nobody lay out the requirements and use cases properly. There are lots of problems you run into and have not even thought about. e.g. what do you expect the player to do when you navigate back after a stream change. You might lose audio or even crash the system if video stream has changed.

I know it was only an example, but for that specific scenario, I'd personally only like the user to be navigate back as far as the last channel change - since this is the way NextPVR, and I'd say many of the other PVRs operate. I've yet to come across a PVR-capable set top box that could navigate back past a channel change. If you want to have this, make the behaviour optional for the addon, since it'd be a pain in the arse to implement.

Also, when the developers are considering the requirements, I'd like it to accommodate PVRs that will only have certain range of the timeshift buffer that is navigable. NextPVR works like many set top boxes and has a timeshift buffer up to a certain number of minutes, like last 30 minutes. I wouldn't want an infinite timeshift buffer, where user leaves XBMC watching a channel and server eventually runs out of disk space.

Quote:I'd personally only like the user to be navigate back as far as the last channel change

A stream change is not necessarily a channel change. Stream changes can happen while watching a channel. Does your addon observe this?

Do you mean a PMT change, like local news/advert insertion etc? NextPVR is able to handle theses, but I have no idea whether XBMC does or not. The transport stream that the NextPVR addon is feeding XBMC will include any changes to that channel's transport stream (whether different PIDs altogether, or changes to characteristics to the of streams on the existing PIDs). Is there something I need to be actively doing in my addon when this type of change happens?

I don't disagree with you on that front generally. However why should the other backend's benefit from even worse hacks (which is what I consider them) which have already been included. At least my changes follow the existing demuxer API as best as possible for supporting addons. They don't require DVDPlayer core changes like the previous mods.

The problem we have is a chicken and egg one, timeshift in TVH had not been implemented (despite constant requests) for 2 reasons 1) no one had time to do it (I'm only now finding time and I admit I'm not necessarily the best candidate, but I'm all we have) and 2) our main client (XBMC) did not have any support so why bother implementing something that couldn't be used by most anyway.

The last point is still true, adding this feature to XBMC now, while somewhat experimental has limited impact on existing code, far less than the other hack in my opinion which muddies the water in the DVDPlayer core by adding PVR specific hacks, and at least means that people are likely to be able to use existing XBMC builds to get at least some (even if not all) features once they become available in the backend.

Ultimately I had hoped to do these changes earlier but simply didn't have the time and had accepted Lar's input that this would now need to be left until after Frodo. However he reviewed the changes and asked me to submit them on the basis that they were well contained (to PVR only code) and therefore should have limited impact.

That being said if they don't make it into Frodo I'm not going to lose any sleep, I will still be able to add the patches to my own builds, it will just limit the ability for a wider audience to begin testing them.

Adam

what hacks to DVDplayer are you guys referring to? Those? https://github.com/margro/xbmc/commit/9d...6f128e271c

@adamsutton: the dvdplayer changes are only used to prevent pausing and seeking when the PVR addon reports that it is not supported.
The situation before was that all commands to seek/pause were blocked at the application level and in the skin (which was also seen as a hack).
Your change is indeed a missing piece for the current timeshift support.

The change in the DVDPlayer is needed to support also the timeshift case were XBMC is just playing a stream like RTSP. In that case, XBMC does all the playback stuff.

In all cases FernetMenta is right about the problem around stream changes. I'm preventing rewind/fast forward across channel switches in the MediaPortal and ForTheRecord addons,
What is not covered is the possible changes in for example audio streams (2.0 to 5.1) while playing a channel (but that was also not covered before for PVR addons that don't do the demuxing).

I've seen the commit from Elupus that continuously scans for stream changes, but this only takes into account the case in which the PVR addon delivers an already demuxed stream to XBMC.

The challenge for a proper timeshift capable PVR API is to handle at least all four current Live TV playback cases (wit examples of the pvr addons):
1) Live TV playback via a fixed stream url (url is known before tuning the channel) (pvr demo addon)
2) Live TV playback via a stream url that is returned by the addon after tuning the channel (MediaPortal in ffmpeg RTSP mode)
3) Live TV playback of a raw stream delivered by the addon (MediaPortal in TSReader mode / ForTheRecord)
4) Live TV playback demuxed streams delivered by the addon (TVHeadend, VDR)

The actual way in which the backend supports timeshifting is also dependent on the backend. Some have always a timeshfit buffer while others need to start a recording and might need to switch to a different stream url/demuxer to play the delayed Live TV stream.
In all cases we would need to handle changes in streams and stream order.

I am not referring to a particular "hack". What I am saying is that before an API change was done, one should lay out all use cases. Not doing so I consider as a hack.
I agree to some changes to dvdplayer as margro laid out though I don't like the grayed out buttons. I don't like e.g. the change to DVDInputStreamPVRManager. This input stream was designed and optimized for live streams which are by nature not navigable. From an OO point of view this change was not very good. Why not having implemented an additional class for input streams which support timeshift. On rewind the players are likely to stall. In this case PVR input streams would engage caching for live streams.

What we have just done to pvr demuxer is an approach of centralizing required functions. When navigating a "timeshift" stream you need to follow certain constraints. e.g. you can't seek to the very end of the stream. Those constraints are the same for every addon and I think this should done by some centralized functions, not in every single addon.

EDIT: timeshift is not only pausing a live stream. What about starting to watch a recording which is still recording. I consider this as an important use case. I program a timer and start watching 30 min later, just to have enough buffer to skip commercials.

Quote:I've seen the commit from Elupus that continuously scans for stream changes, but this only takes into account the case in which the PVR addon delivers an already demuxed stream to XBMC.

This does not really scan for PMT changes. It just detects changes of stream parameters. The notification has to come from the source which is able to detect PMT changes (tvheadend, vdr).

This might be a challenge when using ffmpeg demuxer. I am not sure if ffmpeg does expose this.

I was referring to some of the PVR specific mods in DVDPlayer core that da-anda mentioned. I do understand why they were done, I just think that from a purely aesthetic and idealised software design point of view they needn't have been done that way. But then I'm new to XBMC internals, so I'll try and keep quiet (Not to mention I'm not all that good with media playback anyway, I'm trying to learn as I go).

@FernetMenta indeed timeshift principles can be applied to in-progress recordings the same as live tv, ultimately this will be how TVH works (when we get around to it), i.e. from a clients POV there will be no difference between an in-progress recording and a live stream the same API will exist. (Whether this will extend to complete recordings for which a complete file exists is a matter for internal debate and probably not relevant to XBMC).

With regard to the contraints about handling the end of the buffers etc... there are several ways in which this can be handled. XBMC can (and should) be provided with the relevant information (about the size/duration and cur position, etc...) but equally it might be possible for the addons to simply signal when such conditions are met. This may be harder for file based systems, as opposed to those with their own container/demuxer etc.. For example if you hit the beginning of the timeshift buffer with my (experimental) TVH code it will simply output a message indicating the stream has been paused, if you hit the other end (live point) it will switch back to live playback and indicate this in the same manner.

So I agree that better understanding of all the requirements is indeed something that needs to be done, but at the same time having at least "some" elements of the required APIs within the XBMC core will help reduce the barrier to entry for many users that want to test these features (whatever the current state is) even if they aren't able to get the full and proper experience without further XBMC improvements.

@margro with regard to the stream changes I also agree with FernetMenta that this must be taken into account. While my current timeshift code does not actually output the relevant change info, the provision for it doing so has been included, I've simply ignored it for now for reasons of simplicity (since my own streams rarely change their makeup). But ultimately it will be no different to the way the TVH addon currently works on a stream change, I will simply index the change points in the buffer and output them as and when they are crossed etc...

Adam

(2012-11-06 13:24)adamsutton Wrote:  With regard to the contraints about handling the end of the buffers etc... there are several ways in which this can be handled. XBMC can (and should) be provided with the relevant information (about the size/duration and cur position, etc...) but equally it might be possible for the addons to simply signal when such conditions are met.

I support this, and further believe it should extend into the core player. I can control pretty much everything about playback handling from a python program except growing files on a file system or web-based without a Content-Length. There is dramatic difference with these streams in how various versions of XBMC navigate within the stream and react to the initial duration point and it would be nice to have a standard approach.

Martin