search

xbmc / inputstream.adaptive

kodi inputstream addon for several manifest types
Other
452 stars 241 forks source link

[Disney+] Subtitles appear 3 seconds early #993

Closed MauriceW67 closed 2 years ago

MauriceW67 commented 2 years ago

Bug report

Describe the bug

Here is a clear and concise description of what the problem is:

NOTE 1: I am not sure where the cause of this issue lies, but @glennguy has asked me to create an issue in the IS:A GitHub (see his reply in this Kodi forum topic: https://forum.kodi.tv/showthread.php?tid=353316&pid=3098252#pid3098252).

NOTE 2: The issue already started with an earlier IS:A version (20.1.2), but was not reported before.

NOTE 3: The issue does not happen with the same version of the Disney+ addon with Kodi 19.4 Matrix with the Matrix version of IS:A installed.

When playing some titles with @matthuisman 's Disney+ addon, the subtitles appear exactly 3 seconds early.

Shows that have the issue: Loki, Marvel Studios LEGENDS, The Book of Boba Fett (*) Shows/movies that don't have the issue: The Mandalorian, Avengers: Infinity War, Buffy the Vampire Slayer

(*) Seems like only newer titles have the issue?

Expected Behavior

Here is a clear and concise description of what was expected to happen:

The subtitles should appear at the correct time and not 3 seconds early.

Actual Behavior

Subtitles appear 3 seconds early on some shows.

Possible Fix

To Reproduce

Steps to reproduce the behavior:

  1. Start Kodi v20
  2. Open the Disney+ addon
  3. Browse a show that has the issue (I used The Book of Boba Fett, Chapter 7)
  4. Play the episode

Debuglog

The debuglog can be found here: Part 1: https://paste.kodi.tv/ewoqoyajaw.kodi Part 2: https://paste.kodi.tv/veyequhiho.kodi

MPD/M3U8s/ISM

An example or copy of a manifest (or manifests for HLS - master and variants) can be found here:

Screenshots

Here are some links or screenshots to help explain the problem:

Additional context or screenshots (if appropriate)

Here is some additional context or explanation that might help:

Your Environment

Used Operating system:

note: Once the issue is made we require you to update it with new information should that be required. Team Kodi will consider your problem report however, we will not make any promises the problem will be solved.

matthuisman commented 2 years ago

This issue could also maybe be in Kodi itself? Is there any earlier version of Isa on the same Kodi 20 version that works? Eg 20.1.1?

Ps: great bug report. Thank you

MauriceW67 commented 2 years ago

@matthuisman I was looking through the different branches and for Nexus it started with a 20.0.0 version:

https://jenkins.kodi.tv/blue/organizations/jenkins/xbmc%2Finputstream.adaptive/branches

To be perfectly honest I don't remember in which version I noticed the problem first. As far as I recall the issue started as soon as I started testing v20 nightlies while it doesn't occur in Kodi 19 Matrix.

I'm not sure it's possible to install older IS:A versions on new Kodi v20 nightlies, but perhaps @glennguy can answer that?

matthuisman commented 2 years ago

my gut feeling is the issue is in kodi core. @CastagnaIT what do I need to get you (files etc) to help you solve this?

matthuisman commented 2 years ago

hopefully this helps / is a start manifest.m3u8.txt video.m3u8.txt audio.m3u8.txt subs.m3u8.txt bumper_seg_00000.vtt.txt seg_00000.vtt.txt

I suspect the issue may be bumper subs The Disney+ bumper is 3 seconds long

**#EXT-X-PROGRAM-DATE-TIME:2019-01-01T00:00:00.000Z

EXTINF:3,

8e5fdcd5-e949-4ee6-b58b-fb51437688cb/3349-BUMPER/06/subtitles_1/seg_00000.vtt

EXT-X-DISCONTINUITY

EXTINF:302.208,

bc83f25e-4169-4f5f-b3cf-9ef56a07e572/1ac3-MAIN/06/subtitles_1/seg_00000.vtt**

If Kodi / IA is skipping that initial bumper vtt and going straight to the main, that would make the subs 3 seconds ahead of the video / audio

@MauriceW67 Content without the Disney+ logo I suspect is ok? UPDATE: I tested and confirmed all the content with issues has the Disney+ 3s bumper PS: i hade no idea Disney+ had buffy - cool!

CastagnaIT commented 2 years ago

webvtt files are good then yes is more likely the mentioned #EXTINF:3 the problem

but i see that the "bumper" there is also on video m3u8 so there is to find the reason why on video side this seem not considered

matthuisman commented 2 years ago

are we ignoring EXT-X-DISCONTINUITY / periods for "text" / "subs" maybe? Or is IA / Kodi ignoring that as its just an empty file?

looking at the log, we go and download the first 8x vtt segments. Is this part of the new buffer segments ahead? The vtt segments are 302 seconds long each - so seems we would just need 1x to get our Xmin buffer? Maybe @glennguy knows about more about this

MauriceW67 commented 2 years ago

@matthuisman I'm at work so can't check for bumpers now, but it seems like you already did the research, so that's great :)

Good to see there is some kind of direction of things to look at.

PS: Yeah, Buffy is one of my guilty pleasures too LOL.

glennguy commented 2 years ago

#EXTINF:3 is just saying that the segment is 3 seconds long. We're not ignoring it, just that there's no info in the vtt to help align the time.

From another multiperiod vtt file:

WEBVTT
X-TIMESTAMP-MAP=LOCAL:00:19:04.000,MPEGTS:102972690

00:19:30.920 --> 00:19:33.200 align:center
Right, I'm going to have a shower.

etc...

We have a tag to say "This file starts at this time" Kodi 19 looks at the MPEGTS part but ignores the LOCAL part. I did some work on this a while back but never finished: https://github.com/glennguy/inputstream.adaptive/commit/661ded45d84bb39dfffc7dc726ca4d7621cbd337

However the D+ ones just start back at 0 again. Seems to me like the time elapsed for previous chapters is not being taken into account. Not sure however though if the correct solution would involve sending this extra time as the sub time, or Kodi ignoring previous chapter time elapsed.

CastagnaIT commented 2 years ago

we dont have never edit webvtt metadata, the problem lies elsewhere from X-TIMESTAMP-MAP specs:

   If a WebVTT segment does not have the X-TIMESTAMP-MAP, the client
   MUST assume that the WebVTT cue time of 0 maps to an MPEG-2 timestamp
   of 0.
CastagnaIT commented 2 years ago

i dont have D+ account that i cannot test what i wondering is for example, in second vtt file seg_00001.vtt.txt the first text is this:

00:05:02.208 --> 00:05:04.666 line:85.19%,end Heeft Fett nog andere hulpbronnen?

someone can check if by using website this text sub appears exactly at 5:02?

glennguy commented 2 years ago

Ok all good, I can see through debugging that the subtitle sample packet is being sent with chapter start time added. All looks pretty fine on IA side of things. In my test case that does have the timestamp map tag, this effectively sends the packet pts as double what the start pts of the segment is, so presumable that gets cancelled out on Kodi side once the tag is read. Maybe the problem is where Kodi is calculating the chapter start pts?

MauriceW67 commented 2 years ago

@CastagnaIT That subtitle does not appear at 5:02 on the website, but at 5:05. Which is exactly the 3 second difference we're talking about I believe :)

glennguy commented 2 years ago

If a WebVTT segment does not have the X-TIMESTAMP-MAP, the client MUST assume that the WebVTT cue time of 0 maps to an MPEG-2 timestamp of 0.

So looking at the 5:02 vs 5:05 presentation time of the cue: Kodi is given the PTS of 5:05 (including chapter starting time: https://github.com/xbmc/inputstream.adaptive/blob/f51c6482b0ca2ee95f5eac318ae82e035619c2e9/src/main.cpp#L2156 but based on seeing the cue at 5:02, Kodi must not be including chapter start time on subtitles, at least for inputstreams. we are not mapping the mpeg timestamp of 0 (start of chapter) to webvtt cue time of 0. Instead it seems that webvtt cue time of zero is being mapped to start of entire stream.

CastagnaIT commented 2 years ago

so you mean that we should obtain in some way the charapter start time in the Kodi parser and then shift the webvtt start/stop times based on that offset

glennguy commented 2 years ago

Almost, maybe more correct to get the pts from the start of the chapter and that becomes an offset. The timestamp map is there (or not there) to build the relationship between pts and the final offset we use.

For example the file I sent you yesterday has the timestamp map. I believe this is because the reason for the discontinuities is to insert ads (not inserted btw in this case) therefore the actual program content has pts values that are continuous from one period to the next i.e period 2 ends on 19:03.999 and period 3 starts at 19:04.000. In the D+ example posted here, first period of 3 seconds is unrelated to the main program content. Main program starts at 0pts and uninterrupted therefore doesn't not need explicit timestamp map.

Can we have the start pts of what's being sent to the codec handler in Kodi?

CastagnaIT commented 2 years ago

on the kodi parser we can have the pts tied to the subtitle demux packet sent from ISA (so the pts of a webvtt segment)

matthuisman commented 2 years ago

Please keep in mind that it works on Kodi 19 :)

glennguy commented 2 years ago

CastagnaIT did a massive overhaul of the whole subtitles system in Kodi 20, so it's hard to compare.

CastagnaIT commented 2 years ago

the differences from v19/v20 is that in the v19 the fake parser have the LOCAL value unhandled in X-TIMESTAMP-MAP and the FragmentedSampleReader set the SetPTSOffset to the webvtt handler that could influence start/stop webvtt timestamps: https://github.com/xbmc/inputstream.adaptive/blob/Matrix/src/main.cpp#L1316-L1322 https://github.com/xbmc/inputstream.adaptive/blob/Matrix/src/parser/WebVTT.cpp#L133-L137

this call seems to come from: https://github.com/xbmc/inputstream.adaptive/blob/Matrix/src/main.cpp#L3140

matthuisman commented 2 years ago

@glennguy did you see my comment above? Pasting again below:

looking at the log, we go and download the first 8x vtt segments. Is this part of the new buffer segments ahead? The vtt segments are 302 seconds long each - so seems we would just need 1x to get our min buffer? Maybe @glennguy knows about more about this

glennguy commented 2 years ago

Yeah that doesn't look right. Maybe the offset is causing this? Like it's trying to find -3 seconds and never can...

MauriceW67 commented 2 years ago

@CastagnaIT @glennguy Do you think a possible fix could be developed? Or do you need more time to investigate?

CastagnaIT commented 2 years ago

@MauriceW67 i am not sure how investigate since i dont have account to D+ is possible we should find a way to detect this offset in some way from kodi core to change all webvtt timestamps

CastagnaIT commented 2 years ago

@glennguy are you able to test this to see what happens? (needed both diff xbmc/isa) https://github.com/CastagnaIT/inputstream.adaptive/commit/de7367b9ba6ed894fe1289be508f61d07a86215b https://github.com/CastagnaIT/xbmc/commit/baee005e3bce98e0b48b9814490c69cd1ad243ba

EDIT: links updated

glennguy commented 2 years ago

Tested and D+ would crash on seek or stop.

Adding pts = 0; here https://github.com/xbmc/inputstream.adaptive/blob/f51c6482b0ca2ee95f5eac318ae82e035619c2e9/src/parser/HLSTree.cpp#L637-L639 seems to have fixed it.

Never caused issues before but I found myself the other day when trying to fix this issue that it needs to be in there, otherwise we end up with pts effectively double what it should be at the start of chapters 2+.

glennguy commented 2 years ago

Subs look to be timed well btw.

I also tested with the stream I sent you. Works very well.

CastagnaIT commented 2 years ago

(if you miss i have updated links above) i just tested your akamai sample and works but if i seek to a middle of next chapter the subs arent shown works only if you do not seek

glennguy commented 2 years ago

Was working for me but when seeking chapter subtitles revert to disabled - have to press T.

CastagnaIT commented 2 years ago

if subtitles become disabled when you seek chapter is because you have kodi subtitle language setting set different from "english" and then sub stream track language is different from the kodi sub language setting, then when DEMUX_SPECIALID_STREAMCHANGE is sent run again PredicateSubtitlePriority https://github.com/xbmc/xbmc/blob/master/xbmc/cores/VideoPlayer/VideoPlayer.cpp#L931-L933 and invalidate current user setting, which is what i intended to avoid with my rejected changes

however to me still not works after seek i think we have different sources

MauriceW67 commented 2 years ago

@CastagnaIT @glennguy Thanks for continuing to look into this.

If you need any help testing stuff or generating logs or anything, please let me know.

glennguy commented 2 years ago

if subtitles become disabled when you seek chapter is because you have kodi subtitle language setting set different from "english" and then sub stream track language is different from the kodi sub language setting, Thanks so much for pointing out - I changed to chinese a while ago when testing the Elephants Dream video back when we were fixing other wvtt issues.

https://github.com/CastagnaIT/inputstream.adaptive/commit/7b42715f71b6564be80842cdee9ab4d07716af5d works, https://github.com/CastagnaIT/inputstream.adaptive/commit/de7367b9ba6ed894fe1289be508f61d07a86215b does not.

I'm not sure how we do this without it looking like a hack though. So in the example stream I gave, on 3rd chapter PTS sends 1144s, DTS sends 0 which works well with the offset made inside Kodi, but it seems wrong to do it this way. I'm not super familiar with Kodi side of things, do you have any alternate ideas?

CastagnaIT commented 2 years ago

~i am not able to replicate your situation, tried more times, the https://github.com/CastagnaIT/inputstream.adaptive/commit/7b42715f71b6564be80842cdee9ab4d07716af5d to me not works i tried also add/remove the hls pts = 0; after seek still nothing works, i do not understand where i am wrong~ solved

yes my example was only a test, IMO i think the better way is add some kind of a new "offset" variable to the DEMUX_PACKET struct then we can access to this var directly from OverlayCodecWebVTT.cpp packet because DemuxPacket is derived from InputStream DEMUX_PACKET

glennguy commented 2 years ago

@CastagnaIT Ok after lots and lots of time I can play and seek through the test stream I sent you, and subs stay alive and synced. Needs your test changes in Kodi and more changes on top of your test commit in IA: https://github.com/glennguy/inputstream.adaptive/commit/6e1820d44c5b965fd038d081505655528c9209e7

From a quick test these changes break for mp4 subs, and the copy/paste in AdaptiveStream.cpp is ugly. Ran out of time tonight to go further but I hope this helps right now to do proper testing for the fix Kodi side until I get some more time.

CastagnaIT commented 2 years ago

great! I didn't think there were so many problems please remove my old test commit e9b722a396e66eeda3d879c32e68213c58d4063a from your test branch and build kodi by using my new PR https://github.com/xbmc/xbmc/pull/21523

i have tested your changes this my feedback: 1) not found regressions with webvtt ISO-MP4, tried seeks, change language and adaptive stream test mode 2) player freeze can happens when start playback from a chapter (this before seem was not happening) to replicate: -open your akamai test stream -seek in a casual point of second chapter about middle is ok -on the player bar click exaclty on the white chapter dot, to seek at start of second chapter: immagine -now kodi player will be freezed 3) sometime after a seek, the subtitle text is displayed doubled notice that this was happening also before these new changes, is needed to check why ISA after a seek sometime send the same vtt subtitle segment packet twice to kodi demuxer, from the log: https://paste.kodi.tv/gacugihopo.kodi you can see at line 1489 the flush raised by seek event in the kodi overlay subtitle codec, at this point kodi has cleaned from memory the stored subtitle texts after that 1508 you can see the first packet sent by ISA to kodi, and after this packet, at line 1798 there is another packet received from ISA the same with same data

glennguy commented 2 years ago

not found regressions with webvtt ISO-MP4

my bad, I had one of the changes commented out when testing

player freeze can happens when start playback from a chapter

This happens when seeking to the very last segment of a chapter. I was able to replicate clicking slightly to the left. The stream goes back to 0:00 and playback resumes once it hits 12:22 again. During this time the log is filled with ActiveAE - large audio sync error: 742023.341064 which gradually reduces down to 0. This is exactly the behaviour that happens as described in https://github.com/xbmc/inputstream.adaptive/pull/477#issuecomment-707964149 and possibly other places in the repo, can't remember. I've probably mentioned it to you or Matt Huisman a bunch of times too. My strong suspicion on this is being related to https://github.com/xbmc/inputstream.adaptive/issues/976. There must be some variable in VP that is not yet initialized when moving chapters or not yet being reset from previous chapter. I know already the answer if the issue were to be raised over there though: "You are doing the incorrect thing in your add-on". So, let's try and get to the bottom of that one first.

sometime after a seek, the subtitle text is displayed doubled

I noticed this too before I had these current changes, but haven't seen since.

matthuisman commented 2 years ago

@glennguy I wonder if your hls changes may help with x-discontinuity. Still having users report issues with Channel 10 (now using ssai) and Pluto with IA. Been meaning to open an issue about it.

glennguy commented 2 years ago

@CastagnaIT I've tried https://github.com/xbmc/xbmc/pull/21523 with most of the changes in https://github.com/glennguy/inputstream.adaptive/commit/6e1820d44c5b965fd038d081505655528c9209e7 with some differences:

image

Seems to be very robust with the test stream I've given you (btw, I found out that the first .ts segment in chapter 3 is malformed for the 720p stream which ends up with a bad sub offset somehow) but the sintel test stream with the fmp4 subs does not like when https://github.com/xbmc/inputstream.adaptive/blob/3e55aa75e2a785e8a3c872004d568b5659f916d4/src/common/AdaptiveStream.cpp#L1006-L1008 is removed (video is affected for some reason - stillframe) so more work is needed to make all cases happy

CastagnaIT commented 2 years ago

now the changes will be availables on next kodi nightlies i reopened just to keep track of the last changes that will needed

MauriceW67 commented 2 years ago

now the changes will be availables on next kodi nightlies i reopened just to keep track of the last changes that will needed

Thanks for all the effort you put into this.

I assume that before we can test this, the other changes need to be implemented first? Are they changes to InputStream?

CastagnaIT commented 2 years ago

in theory for your use case should work anyway the new kodi nightly should be available from tomorrow instead the updated ISA you can download from here: https://jenkins.kodi.tv/blue/organizations/jenkins/xbmc%2Finputstream.adaptive/detail/Nexus/142/artifacts

MauriceW67 commented 2 years ago

I tested with today's nightly and indeed the shows with subtitles in the Disney+ addon that had issues before now show them at the correct time.

Thanks again @CastagnaIT and @glennguy !

@matthuisman FYI.

matthuisman commented 2 years ago

Thanks for letting me know. Great work IA team :)

Feel free to close your ticket

GG-76 commented 2 years ago

@CastagnaIT Hello, i tried it on the latest version of inputstream adaptive 20.2.3 but i have Kodi from Kodinerds version net.kodinerds.maven.kodi20.armeabi-v7a-202207090150-662a292c-Nexus.apk. And I still get the subtitle bug. Could it be a Kodi version issue? Thank you.

matthuisman commented 2 years ago

@Kravatak76 20.2.3 from the repo does not include the fix. No new version of IA has been released since this fix was merged. Maybe @glennguy would like to do a 20.2.4 release?

In the meantime, you can download a IA with the fix from here: https://jenkins.kodi.tv/job/xbmc/job/inputstream.adaptive/job/Nexus/142/artifact/cmake/addons/build/zips/inputstream.adaptive+android-armv7/inputstream.adaptive-20.2.3.zip

Also, your kodi build requires commit: 79719691a8208b061fe78309e88046a2be5167f6 (https://github.com/xbmc/xbmc/pull/21523/commits/79719691a8208b061fe78309e88046a2be5167f6) This was merged 8 days ago and looks like your build was generated 3 days ago - so should be there. You'd need to check with kodinerds whether they had merged in the above upstream commit yet in their builds

GG-76 commented 2 years ago

@matthuisman sorry my mistake I checked the link here and saw version 20.2.3 there so I thought it was the one released in the repo :) So thanks, i try it :)

GG-76 commented 2 years ago

@matthuisman great, its working now :)

MauriceW67 commented 2 years ago

@CastagnaIT @glennguy I wanted to download the IA with the fix for 32-bit (only downloaded the 64-bit one before), but the link is no longer working. Is it already incorporated in a release?

matthuisman commented 2 years ago

@MauriceW67 https://jenkins.kodi.tv/blue/organizations/jenkins/xbmc%2Finputstream.adaptive/detail/Nexus/149/artifacts

CastagnaIT commented 2 years ago

yes has not been released yet i think @glennguy want wait to try add remaining subtitle fixes

MauriceW67 commented 1 year ago

@CastagnaIT @glennguy I would like to re-open this issue if possible.

I've been testing the latest Kodi Omega (build from April 7) together with IS:A version 21.1.0 for Omega and the subtitles on the newer Disney+ shows now require a delay of 6 seconds in order to be in sync (used to be 3 seconds before).

Not sure if all the fixes in Kodi and IS:A are in place yet, but it would be nice to stay updated on this please?