kodi-pvr / pvr.iptvsimple

IPTV Simple client for Kodi PVR
GNU General Public License v2.0
773 stars 372 forks source link

[feature request] archive support #273

Closed Sorien closed 4 years ago

Sorien commented 5 years ago

from pvr api 5.7.0 there are functions to play channel from epg IsEPGTagPlayable https://github.com/xbmc/xbmc/pull/12689

now when plugin has maintainer would be possible to implement them?

there was pr for for coreelec but never submitted upstream https://github.com/CoreELEC/CoreELEC/pull/138

also i'd suggest to add custom m3u extension

#EXT-KODI-ARCHIVE:<days> <url>

url should contain some replaceable parts like $start_time_utc

phunkyfish commented 5 years ago

Please ask @arthur-liberman, the API changes were added to allow his PR’d functionality to be possible.

arthur-liberman commented 5 years ago

IIRC, one of the problems with the new Kodi API is that it did not pass the timestamp of the selected epg item. I haven't checked whether there were any updates to it, so I'm uncertain on its status today. The CoreELEC PR was CE-only, because it introduced changes to Kodi itself in order to support the functionality I wanted to create in this feature.

phunkyfish commented 5 years ago

@ksooo was not passing the timestamp by design?

I'm re-reading the forum posts trying to figure out where this left off ;)

arthur-liberman commented 5 years ago

I'm a bit foggy on this, looking at the code now, I think the EPG tag actually does contain the start time, so I was wrong. I think what I had a problem with was to seamlessly "timeshift" to a past program from the guide. But it's working in CoreELEC, and the user experience IMHO is awesome. You pick a show, click "Play programme" and you get the LiveTV UI timeshifted to the show you selected. So you can easily click through to previous or next program.

phunkyfish commented 5 years ago

Ideally we would get this to work upstream too!

What do we need to do to progress this?

arthur-liberman commented 5 years ago

You can look at this patch to see what I had to change to get Kodi to do what I wanted. No the greatest way of doing things, but it works. https://github.com/CoreELEC/CoreELEC/blob/coreelec-9.2/packages/mediacenter/kodi/patches/kodi-ce-010-add-pvr-archive-support.patch Kodi will need a considerable refactor, I think, to allow for this functionality natively.

phunkyfish commented 5 years ago

The patch doesn’t look too big. Also, this is a patch on top of upstream kodi right?

Are you willing to try and upstream this?

arthur-liberman commented 5 years ago

It's on top of Leia, but the way it's written right now will never be accepted. Especially the bit at the end. It essentially a big hack. As I mentioned, there's probably a better way to do this, but I don't know enough about the structure in Kodi to make it work properly. Basically what we need is native way of playing an epg item as timeshifted live tv. We then probably need a custom demuxer in the addon to support the switching the stream URL on the fly. There are probably other things, but I'm drawing a blank right now. I've made this last year, and now it's just too vague :( @vpeter4 may have a better idea what is missing for this to work.

phunkyfish commented 5 years ago

I hear you, nothing is worse than something you wrote a while back. Sometimes it doesn’t look like your code ;)

Let’s see what’s missing and we can pull in who we need to to progress it. Even if we PR with a hack someone will have a suggestion on the right approach to take.

But now, early in Matrix dev is the right time to do this.

vpeter4 commented 5 years ago

If I remember correctly the demuxer part here is used only for live timeshifting. As we can read implementation like done atm will never be accepted in Kodi. Last time I looked (and also tried) how to proceed and there are some different paths. One thing is real demuxer part in pvr.iptvsimple addon. The other is improved demuxer in InputStream Adaptive addon. Or some better Kodi redo what arthur did. And probably something else. There were some discussion in a thread https://forum.kodi.tv/showthread.php?tid=337062 but after all this time I lost track. Maybe we should read the thread again and made some conclusions. The demuxer hack made in Kodi is so simple opposite to writing real demuxer.

But this demuxer part has nothing to do with playing something from EPG. This are two very different things. I looked in "Play from EPG" when I saw PR 12689 in late 2017 and using it till then. Later arthur picked my work and made what we have today.

phunkyfish commented 4 years ago

Are there changes required to both pvr.iptvsimple as well as kodi?

What I’d like to do is get a working (not necessarily correct implementation) version based on kodi 19.

Once we have a branch working we can get advice on the best way to take it forward.

Sound reasonable?

arthur-liberman commented 4 years ago

What we have in CoreELEC is a mechanism in Kodi, which only works with a specific addon name (pvr.iptvarchive). To be 100% frank, since I worked on it quite a while ago, I don't remember the exact details, but the added demuxer and inputstream in Kodi allow you to seek within a live TV stream as if it has timeshift. Once you seek to a specific timestamp, the stream is reopened, using the appropriate timestamp. You can see the changes I had to make to Kodi here and to pvr.iptvsimple here

phunkyfish commented 4 years ago

Cool, the kodi changes look reasonable to code up in v19 (even if they are not the right right way to do it). I would just need to blend them with the PVR api additions. I may need some guidance on the addon when I get to that point if you don’t mind?

arthur-liberman commented 4 years ago

Just one point to note. The way it's implemented right now, at least in my case, has some issues. When I seek the stream often, and/or the source is being a bit "slow", sometimes the timestamp goes completely out of scope. I'm not sure why it happens exactly, but I think that this implementation, although it works pretty well most of the time, can be a little finicky.

vpeter4 commented 4 years ago

For me it works perfectly and I don't see such issues. I assume it has to do something with different stream types/servers.

But current implementation in Kodi will not be accepted as told few times in the past. The "correct" approach would be to have everything handled by pvr client. Which at least in this case means lot of extra code in pvr client as opposite to what is done in Kodi (few lines against few 1000).

phunkyfish commented 4 years ago

Once there is a working version for matrix we can solve the demux problem next.

Is there a particular provider you recommend I should get an account with that works with this implementation? Based in Ireland.

vpeter4 commented 4 years ago

I wouldn't know of any free provider with epg :(

phunkyfish commented 4 years ago

Not free. Happy to pay, just knowing that it is confirmed to work.

vpeter4 commented 4 years ago

This thread should be read and maybe someone wrote provider name: https://discourse.coreelec.org/t/coreelec-with-iptv-archive-support/1950

phunkyfish commented 4 years ago

Found one that works in Ireland

phunkyfish commented 4 years ago

@arthur-liberman I've done most of a port to Matrix to see it in action and I have a question.

Both an EPG tag and a timeshift start time are stored per channel. Is preserving this state across channels important or is it just an easy place to store it as you tend to always access a channel anyway?

To say it another way, would a single struct holding both values work instead but it would just mean another thing to pass to each function call.

vpeter4 commented 4 years ago

Can you please clarify this question? It is little vague :)

phunkyfish commented 4 years ago

Sure, PvrIptvChannel has two additional members use to store state:

  EPG_TAG     epgTag;
  time_t      timeshiftStartTime;

From reviewing the code I'm not seeing where storing this state per channel makes sense. Once you switch programmes (between channels or back forward in the current channel) the previous state is overwritten anyway.

Just wondering was I missing something or is my thinking correct.

vpeter4 commented 4 years ago

I wouldn't know any of this (it is arthur's code). But will look into. Maybe this 2 members could just be global variables? I don't think that after channel switch data from previous channel matter?

phunkyfish commented 4 years ago

That’s my theory. Sure I guess I might as well try it and see does it work 😉

vpeter4 commented 4 years ago

So I moved this 2 members to global variable in client.cpp

EPG_TAG     g_epgTag;
time_t      g_timeshiftStartTime;

and replaced it's usage. On a quick test I didn't noticed any difference in functionality. Seems to be working same as before.

ksooo commented 4 years ago

global variable

Avoid such things, please. There are better alternatives in object oriented programming. I'm sure @phunkyfish knows.

vpeter4 commented 4 years ago

Ah, I just saw this addon has been rewritten for Matrix and doesn't have global variables anymore. But I'm still on Leia where it has bunch of them. That's why I just follow same rules :)

phunkyfish commented 4 years ago

Ya, it’s changed quite a bit. I think I’ll have the basic version of catchup (without timeshift) in the coming weeks.

Then the hard work starts of figuring out the right way to do the timeshift piece.

phunkyfish commented 4 years ago

But thanks for checking it, it’s means my theory is correct 😉

arthur-liberman commented 4 years ago

I'm not entirely sure why I have both timeshiftStartTime and the epgTag which already includes the start time. It's possible that I needed it in the initial iterations of the development, and never went back to change it. I just looked at the code, and it doesn't seem like timeshiftStartTime member in PVRIptvChannel is necessary.

phunkyfish commented 4 years ago

Great, thanks for confirming.

vpeter4 commented 4 years ago

I see you are doing some progress - is there anything available to test? :)

phunkyfish commented 4 years ago

Not just yet. Right now just getting regular ffmpeg to work right, then need to get it working for all platforms and finally the task of adding the archive stuff.

phunkyfish commented 4 years ago

Ok, I changed the order and went for adding the archive/catchup first.

I now have a working inputstream addon doing archive!!! But it only runs on mac currently.

What this means is that the only original kodi core change that is still needed are the ones from xbmc/pvr/guilib/PVRGUIActions.cpp.

Will clean this up in the next few days, push it to the repos and you can take a look @vpeter4 @arthur-liberman

phunkyfish commented 4 years ago

Ok, there is still one issue. When viewing as live when you hit previous or next on the player window the programme time will change but there is no equivalent call to update the catchup URL, i.e. the GetURL() call that was in DVDInputStreamFFmpegArchive.

Any ideas? There is no longer a way to call back to the addon in this case unless we introduce an update properties call to the inputstream API (which may be the only path forward).

phunkyfish commented 4 years ago

Options would be a callback to update the URL, get the programme start and end time etc. but I think would need to be done for every seek I think.

If someone could explain how seeking and updating the URL worked in the previous version maybe I could come up with a better solution.

phunkyfish commented 4 years ago

Let me try moving the URL generation logic into the inputstream addon. This should mean seeking within the current channel does not cause a stream reload. Right now on a seek it does switch it just that it’s to the same URL each time.

I have noticed that a stream load is faster using the new addon. But would need some real world user testing to confirm that.

vpeter4 commented 4 years ago

Waiting to do some tests :)

phunkyfish commented 4 years ago

The platform support will get in the way of that. As the addon builds ffmpeg I’ve figured how to build that for Mac. It’s probably best to prove the solution is viable first though before doing the platform stuff cause its very time consuming and difficult.

phunkyfish commented 4 years ago

So I've confirmed I can change the stream URL from within the inputstream addon. It may be a little clunky but it's functional so I'll copy the URL generation functions there too (they need to be in both the pvr addon and the inpurstream addon to cover cases where a user has a catchup provider but doesn't use the new inputstream addon or can't).

Once we have something functional we can figure out any performance related enhancements.

vpeter4 commented 4 years ago

Btw: here has to be LOGLEVEL_ERROR: https://github.com/phunkyfish/inputstream.ffmpegarchive/blob/Matrix/src/stream/threads/platform/pthreads/ThreadImpl.cpp#L195

phunkyfish commented 4 years ago

Thanks, spotted a few others in ifdefs ;)

phunkyfish commented 4 years ago

Ok, I think we are working well. Compared it to the original and the performance appears to be the same. Just one bug with the stream EOFing if you try to go past now.

I've updated the inputstream repo and iptvsimple PR.

Have quite a bit of cleanup work and I'm thinking of renaming the inputstream addon before moving it to xbmc repo too. Then onto tackling the platforms.

phunkyfish commented 4 years ago

Ok, the new inputstream addon is now in the kodi repo and v1.4.0 which should appear today should work for all platforms. As we also have the changes in kodi-core that we needed it should all run from the IPTV Simple PR.

So in V1:

But's it's working which is good start ;)

phunkyfish commented 4 years ago

Done ;)