[doktourtv]

Using windows 10 64 with Kodi 18 alpha nightly (clean install) - MediaPortal server 1.17.0.0

MediaPortal PVR addon is not skipping commercials. Thinking it was a problem with Kodi, I opened a bug tracker:

https://trac.kodi.tv/ticket/17584

It was determined that the problem was not with the Kodi code or the built-in player, but with the MediaPortal PVR addon. This was proven in the following manner: If the file is run from the 'files' menu in Kodi, the commericals are skipped properly, however if the same file is run from the MediaPortal PVR addon, the edl file was ignored and the commericals were not skipped.

Here is the debug log (log is from previous nightly however the issue still exists in the Nov 30th nightly):

https://pastebin.com/Ex9ceM9s

In Kodi 17.x the MediaPortal PVR addon skipped the commericals witout issue.

Hoping to make the developers of the MediaPortal PVR addon aware of the issue.

If there is already a fix for this, please advise.

Thank you

[margro]

The MediaPortal PVR addon has never supported commercial skipping. The reason why it worked is that the recordings were learned to Kodi by passing the file path to the recording directly. As a result, Kodi plays the recording like in the video section (as normal file).
This functionality has been removed from the PVR API. Now recordings are played through the PVR API as stream and there is no known way for me to tell Kodi which EDL to use.
The v18 PVR API is much worse than the v17 one. I'm not able to fix this. I'm sorry about this.

[doktourtv]

The MediaPortal PVR addon has never supported commercial skipping. The reason why it worked is that the recordings were learned to Kodi by passing the file path to the recording directly. As a result, Kodi plays the recording like in the video section (as normal file).
This functionality has been removed from the PVR API. Now recordings are played through the PVR API as stream and there is no known way for me to tell Kodi which EDL to use.
The v18 PVR API is much worse than the v17 one. I'm not able to fix this. I'm sorry about this.

Regrettable, but understood.  Thank you for the reply and for your work on the addon.

[doktourtv]

The MediaPortal PVR addon has never supported commercial skipping. The reason why it worked is that the recordings were learned to Kodi by passing the file path to the recording directly. As a result, Kodi plays the recording like in the video section (as normal file).
This functionality has been removed from the PVR API. Now recordings are played through the PVR API as stream and there is no known way for me to tell Kodi which EDL to use.
The v18 PVR API is much worse than the v17 one. I'm not able to fix this. I'm sorry about this.

 I had a thought. Is there some way to add "play using" to the pvr playback context menu? Then one could configure a player in playerfactory.xml and use it instead.

[ksooo]

This functionality has been removed from the PVR API. Now recordings are played through the PVR API as stream and there is no known way for me to tell Kodi which EDL to use.

That's not correct.

If you implement GetRecordingStreamProperties you can pass the recording url to Kodi and let Kodi do the playback like in v17.

Why can't you implement GetRecordingEdl? This has nothing to do with any stream vs. URL API changes, imo.

EDIT: besides that, it could be that edl support in v18 is currently broken, this is unrelated to any add-on. Please keep in mind, that v18 is still in pre alpha state.

[doktourtv]

This functionality has been removed from the PVR API. Now recordings are played through the PVR API as stream and there is no known way for me to tell Kodi which EDL to use.

That's not correct.

If you implement GetRecordingStreamProperties you can pass the recording url to Kodi and let Kodi do the playback like in v17.

Why can't you implement GetRecordingEdl? This has nothing to do with any stream vs. URL API changes, imo.

EDIT: besides that, it could be that edl support in v18 is currently broken, this is unrelated to any add-on. Please keep in mind, that v18 is still in pre alpha state. 

I believe edl support in 18 was fixed according to this bug report: https://trac.kodi.tv/ticket/17584 

The fact that commercials are skipped if the video is played from "files" supports the fact that edl support IS working in 18.

[margro]

That's not correct.

If you implement GetRecordingStreamProperties you can pass the recording url to Kodi and let Kodi do the playback like in v17.

Why can't you implement GetRecordingEdl? This has nothing to do with any stream vs. URL API changes, imo.

Of course I already tried using GetRecordingStreamProperties() to implement it like in v17, but this function works different than the old v17 functionality. Originally, the (now removed) strStreamURL argument got the filename (e.g. "recording.ts") and the ReadEditDecisionLists() function found the EDL. If I pass the same file name via GetRecordingStreamProperties(), the ReadEditDecisionLists() function receives a "pvr://recording/something.pvr" argument and it does not check the new stream URL for EDL info. So, right now the old v17 way is not possible anymore.

As said earlier, I never implemented EDL support in the addon since I'm not using it myself. I can implement GetRecordingEdl() but that means that I have to read the files, and transform them to a times list in the PVR addon.
This is a bit weird since Kodi already knows how to read these files. I would prefer to learn the EDL file to Kodi instead. Maybe we could add a property to the GetRecordingStreamProperties() list for this purpose...

[ksooo]

That's not correct.

If you implement GetRecordingStreamProperties you can pass the recording url to Kodi and let Kodi do the playback like in v17.

Why can't you implement GetRecordingEdl? This has nothing to do with any stream vs. URL API changes, imo.

...
If I pass the same file name via GetRecordingStreamProperties(), the ReadEditDecisionLists() function receives a "pvr://recording/something.pvr" argument and it does not check the new stream URL for EDL info. So, right now the old v17 way is not possible anymore.
...

Maybe this is just a (small) bug in the Kodi core which should be fixed. I will take a look.

[ksooo]

ReadEditDecisionLists

@margro does this patch fix your problem?

diff --git a/xbmc/cores/VideoPlayer/VideoPlayer.cpp b/xbmc/cores/VideoPlayer/VideoPlayer.cpp
index 607579bc3c..d1ba6a4126 100644
--- a/xbmc/cores/VideoPlayer/VideoPlayer.cpp
+++ b/xbmc/cores/VideoPlayer/VideoPlayer.cpp
@@ -3676,7 +3676,7 @@ bool CVideoPlayer::OpenVideoStream(CDVDStreamInfo& hint, bool reset)
     if (hint.fpsrate > 0 && hint.fpsscale > 0)
     {
       float fFramesPerSecond = (float)m_CurrentVideo.hint.fpsrate / (float)m_CurrentVideo.hint.fpsscale;
-      m_Edl.ReadEditDecisionLists(m_item.GetPath(), fFramesPerSecond, m_CurrentVideo.hint.height);
+      m_Edl.ReadEditDecisionLists(m_item.GetDynPath(), fFramesPerSecond, m_CurrentVideo.hint.height);
     }
 
     if (s.stereo_mode == "mono")

[margro]

@ksooo Yes, that will work since GetDynPath() returns the original file name when I'm sending this as stream URL using GetRecordingStreamProperties(). You should of course only try the GetDynPath() for PVR recordings when it is not empty.

[doktourtv]

@ksooo Yes, that will work since GetDynPath() returns the original file name when I'm sending this as stream URL using GetRecordingStreamProperties(). You should of course only try the GetDynPath() for PVR recordings when it is not empty.

 I will be happy to test the 64bit  on windows once the patch is applied. Let me know.

[ksooo]

@ksooo Yes, that will work since GetDynPath() returns the original file name when I'm sending this as stream URL using GetRecordingStreamProperties(). You should of course only try the GetDynPath() for PVR recordings when it is not empty.

Cool. I will PR the fix.

BTW: GetDynPath implementation is smart enough to return the "static" path (always present) if no "dynamic" path is given. So, no extra checks needed here.

[margro]

 I will be happy to test the 64bit  on windows once the patch is applied. Let me know.

 The Kodi patch is already merged (thanks @ksooo) , but I still need to update the MediaPortal PVR addon to restore the similar behavior as before in v17. Will post in this thread when I've updated the addon as well.

[ksooo]

@ksooo Yes, that will work since GetDynPath() returns the original file name when I'm sending this as stream URL using GetRecordingStreamProperties(). You should of course only try the GetDynPath() for PVR recordings when it is not empty.

Cool. I will PR the fix.

BTW: GetDynPath implementation is smart enough to return the "static" path (always present) if no "dynamic" path is given. So, no extra checks needed here. 

The fix will be available with todays Kodi 18 Leia nightly builds.

[doktourtv]

Many thanks to ksooo and margro for your hard work and for not abandoning this!!