[VP] Fix short stream changes

[peak3d]

Description

Thx to @FernetMenta who provide a proper fix for the sync issues if we have short chapters devided by DMX_SPECIALID_STREAMCHANGE.

Motivation and Context

If multi chapter streams start with a short into (in my example 3 sec), DMX_SPECIALID_STREAMCHANGE arrives from demuxer before the first packet was decoded and therefore player started. This leads to AV sync issues and depending on the device to long chapter crossings.

How Has This Been Tested?

Play Mandalorian S1E1 with Disney addon on NVIDIA Shield TV
Play VTM Go "Loïc: Zot van Koken"

Types of change

• Bug fix (non-breaking change which fixes an issue)
• Clean up (non-breaking change which removes non-working, unmaintained functionality)
• Improvement (non-breaking change which improves existing functionality)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that will cause existing functionality to change)
• Cosmetic change (non-breaking change that doesn't touch code)
• None of the above (please explain below)

[michaelarnauts]

This is also a fix for xbmc/inputstream.adaptive#332 and add-ons/plugin.video.vtm.go#139

The previous attempt to fix this was #16658, but that PR was reverted.

[mediaminister]

I tested this a bit and I can confirm this fixes AV sync issues with one small period followed by a longer period. But, it seems this doesn't fix the issues when there are multiple small periods followed by a longer period.

This also introduces a new problem with seeking. I got eof reading from demuxer when seeking with the up arrow key (seeking to next period?) with VTM GO add-on.
Here is a full debug log:
seekdebug.log

[peak3d]

Thx, will look later at it

[peak3d]

Forced pushed a change @mediaminister, thx for testing!

[mediaminister]

Thanks, I can confirm seeking with the up arrow key works perfectly now.

But when the stream starts with two small periods, there is still an AV sync issue when the third (long) period starts. Here is a full debug log:
twosmallchapters.log

[peak3d]

Yes, I know, will try to fix this, too

[peak3d]

@mediaminister I added a condition -> pls. check again

[mediaminister]

I tested but this doesn't solve it.
Log: notyetsolved.log

[peak3d]

I'll add some more logging, I'm in opinion that your update was not successful.
I started multiple of the series you watch, and they are all fine now

[mediaminister]

I started multiple of the series you watch, and they are all fine now.

Did you try with "Loïc: Zot van Koken", because there is an extra sponsor advertisement before the program starts.
For instance:

xbmc.executebuiltin('PlayMedia(plugin://plugin.video.vtm.go/play/catalog/episodes/cd5d1db0-519f-40de-b07e-bceb716e4848)')

in autoexec.py

For me it isn't solved on Linux x64. Here is a new log with the extra logging:
notyetsolved_withlog.log

[Rechi]

Please follow the current code guidelines.

[Rechi]

Conventional operators have to be surrounded by one space on each side.

https://github.com/xbmc/xbmc/blob/master/docs/CODE_GUIDELINES.md#34-whitespace

[peak3d]

@mediaminister next try, thx for testing

[mediaminister]

Thanks a lot, it plays perfectly now, all av sync issues seems to be fixed!

But, the eof reading from demuxer bug when seeking with the up arrow key is back.

I tested with

xbmc.executebuiltin('PlayMedia(plugin://plugin.video.vtm.go/play/catalog/episodes/dbbf9404-323a-42b9-ad8c-65d651e6f303)')

in autoexec.py

Debug log:
seekbugagain.log

[peak3d]

@mediaminister the stream should now continue after seek, but - assuming that you start seeking while in a short chapter - continues at the beginning of the chapter seeked. This is something I'll fix in a second step in inputstream.adaptive.

[mediaminister]

Yes, the stream continues after seeking now, but the AV sync bug when a third (long) period starts is reintroduced:
avsyncissue.log

[peak3d]

@mediaminister latest commit should be working now.

[mediaminister]

Thanks a lot @peak3d, I can confirm all AV sync issues are gone and seeking works:
everythingok.log

[michaelarnauts]

OMG. It finally happened! @peak3d and @mediaminister you guys rule!

[phunkyfish]

I don't think this is a hack. It kind of does what @FernetMenta suggested in the post below. Postpone any further reads (or stream changes) until the current one is complete. In this case it mean starting the players. Nice work @peak3d

#16658 (comment)

[peak3d]

@fritsch this PR is now for several weeks in the milhouse builds (rbpi / x86 linux) and I have not found a single negative report in his forum thread. How should we proceed? I use this PR for long time, too, without any issues.

If you know how to reproduce refering to the comment of @FernetMenta, can you pls. reproduce or guide me / us to get it reproduced?

[phunkyfish]

I reverted this change in my local build as it does indeed break with PVR. I couldn't reproduce reliably (it was on the family box in the living room) but I did have several instances where I had to power down the device. Since the revert zero power cycles.

[FernetMenta]

@FernetMenta
How much do we need pay you for you to tell us how to do this correctly?
My wife says I give a good back massage...?

This PR fixes quite a few issues so be nice to get a correct fix merged in (and backported) :)

The problem here is that folks are egoistic and only interested in their own use cases, not showing respect for other scenarios which are not their own priorities (pvr in this case). Even users with no coding knowledge at all chime in and try to push their use cases regardless if it breaks existing stuff.
Instead of discussing the design and trying to find a solution that is suitable for all, people defend this change and push others to prove that it is wrong.

Nevertheless

tell us how to do this correctly?

It's that simple. Ask for help and you'll get it. Try this change (I did some time ago) first:
https://pastebin.com/KpH7GcRk
It fixes dynamic program change for pvr and might also fix this issue here. Even if it did not fix the issue here, it should be applied before further investigation.

[peak3d]

For me @FernetMenta fix works (Disney+), curious if it also works with vtm go
Edit: vtm go seems to work, too

[mediaminister]

I can confirm this works for VTM GO: shortshapter.log

Can this be backported to the Leia branch?

[FernetMenta]

If you don't apply the full patch I provided, dynamic program changes on live TV won't get fixed.

[peak3d]

The first hunk is not in current master, it was rejected. But let me search again.
Edit: @FernetMenta the first hunk is not in any way in current kodi master, can you guide me to the place to change pls.?

[FernetMenta]

@peak3d sorry, you are right. this was some debug helper code that never made it to master.

[peak3d]

Jenkins build this please

[peak3d]

IOS build issue unrelated