xbmc / inputstream.adaptive

kodi inputstream addon for several manifest types
Other
453 stars 242 forks source link

[Kodi 20][Subtitles] Disn€y+ subtitles broken #836

Closed CastagnaIT closed 2 years ago

CastagnaIT commented 3 years ago

Bug report

Describe the bug

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

I am doing tests with "Ribelle - the brave" movie but i think also other movies are same

When you start (or resume) the video the subtitles not works at first start, if you try switching to another track apparently is ok, for the first time, if you switch another time subtitles are full broken

I have tried investigate but i am not able to find where the real problem come from

To note that if you force install the ISA binary of Kodi 19, to Kodi 20, the subtitles works as expected

Expected Behavior

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

Actual Behavior

Possible Fix

To Reproduce

Steps to reproduce the behavior:

  1. Open "Ribelle - the brave"
  2. (only once) Play it a first time and choose the CC subtitles in order to save it as preferred, then stop it
  3. Play it
  4. No subtitles shown

Debuglog

The debuglog can be found here: https://paste.kodi.tv/ocokocupum.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 3 years ago

I'll check today and make sure it's not being caused by something my end (addon or my proxy or the subtitle conversion stuff I do)

Oh. Just read your whole info. If switching to IA 19 fixes then wouldn't be addon

glennguy commented 3 years ago

I will look at tonight, I hope I can find the cause

glennguy commented 3 years ago

I can reproduce here, I need to find a non DRM example to make debugging bearable.

CastagnaIT commented 3 years ago

i have not found, also in Extra context menu nothing, maybe @matthuisman know if there are somewhere? i tried also to find sample MPD's with webvtt but i have not found segmented samples like on D+...

CastagnaIT commented 3 years ago

@glennguy today i am lucky

try with this: _Test_WebVTT_segmented_HLS.strm.txt found at: https://hls-js.netlify.app/demo/

seem to have same subs problem of D+

glennguy commented 3 years ago

I can't reproduce on that one :/

I've loaded into a test addon so I can have it resume, no issues so far.

CastagnaIT commented 3 years ago

to me not works good, i do this steps:

the only different thing than D+ is that in this sample the subs works good when you enable them, but will be broken after switching sub language more times

Liqianyu commented 3 years ago

I can confirm that the same problem exists for TV series. Problem Summary

  1. Subtitles are only displayed when switched for the first time. Repeated switching will prevent it from being displayed For example, start playing Eng Sub (Not Work) → Fra Sub (Work) → Chs Sub (Work) → Fra Sub (Not Work)
  2. The subtitle timeline will be advanced by 3 seconds, you can match it by manually delaying 3 seconds.
  3. Switching audio track will cause the currently displayed subtitle to stop displaying.
  4. Switching the video stream will cause both the subtitles and the screen to fail to be displayed.
  5. If buffering occurs, it seems to cause the subtitles to stop displaying.
glennguy commented 2 years ago

@CastagnaIT sorry I've been very time poor the last few weeks.

Having another look now and can confirm there's an issue.

Running out of time again but if I comment out this https://github.com/xbmc/inputstream.adaptive/blob/5134fcd8b0540f3457e55dfecceda3bcda53bd84/src/parser/HLSTree.cpp#L738 and force sub streams to refresh on stream start by modifying this https://github.com/xbmc/inputstream.adaptive/blob/5134fcd8b0540f3457e55dfecceda3bcda53bd84/src/common/AdaptiveStream.cpp#L235 to if (!(current_rep_->flags_ & AdaptiveTree::Representation::INITIALIZED) || current_rep_->containerType_ == adaptive::AdaptiveTree::CONTAINERTYPE_TEXT) with the test stream then I can switch subs as much as I like. Maybe to do with current_segment_ being at the end of the stream? (haven't looked just thinking out aloud)

So the clue is that redownloading/refreshing the representation makes it all good. If I can't find time in the next couple of days I hope the above will help someone else track it down.

CastagnaIT commented 2 years ago

thanks for the info, i don't know enough about how the code works to debuggin it so i have to rely on you or others devs

my D+ subscription is expired a few days ago then i can no longer test with D+

Liqianyu commented 2 years ago

The last two days while watching Disney Plus I noticed a strange problem. The buffering did not run out but the screen flickered (seemed to be re-matching the resolution frame rate) and the subtitles disappeared. Looking at the logs at I found that Kodi says "Opening stream: 1015 source: 256", so ISA seems to be loading another video stream?

The issue seems to be mentioned here, 'manual' stream selection setting shows the different streams in the OSD, but automatic stream switching still occurs.

@matthuisman I set the add-on PlayBack-Playback Quality from Bypass to Best which seems to workaround the problem. @CastagnaIT If you need a D+ subscription for test, I can share it with you. @glennguy ISA does not re-download the subtitles after the segmented subtitles of D+ are switched again. And I tested with Hulu and found that switching subtitles will re-download them. So I think the key is to fix the problem that switching segmented subs won't re-download them. Thanks.

CastagnaIT commented 2 years ago

@glennguy thanks 🎉 seem to works with the sample stream, but i will need to verify better in next days i should verify the behavior also with D+, maybe @Liqianyu can give some feedbacks? or as you proposed if you can sent me a temporary access to D+ would be useful (you can contact me with my e-mail from my GitHub homepage links)

however we still have a problem with DEMUX_SPECIALID_STREAMCHANGE call at every call disable the subtitles, as you pointed out in https://github.com/xbmc/xbmc/blob/7478f983b67dc1e3b09e061acf2eaa675b0f0170/xbmc/cores/VideoPlayer/VideoPlayer.cpp#L930 this has been add from PR https://github.com/xbmc/xbmc/pull/6105 seems only to fix a problem with CC subtitles "in conflict with external subtitles" it is not clear what exactly it is a super old PR, maybe reload all subtitles is needed for PVR purpose i do not know, but does not seem to be what we need with ISAdaptive where subtitle streams remain always unchanged, i don't know well how that part of kodi works i will try to examine it to see if i am able understand better the things

Liqianyu commented 2 years ago

@glennguy @CastagnaIT After testing, I can confirm that PR #841 fixes the Disney + Add-on subtitle problem. However, this branch does not merge with PR #820. does this fix have anything to do with automatic stream switching? I did not find the auto-stream switching issue that was previously present in about two hours of playtesting.

CastagnaIT commented 2 years ago

does this fix have anything to do with automatic stream switching? I did not find the auto-stream switching issue that was previously present in about two hours of playtesting

what you are talking about precisely?

Liqianyu commented 2 years ago

@CastagnaIT I mean, during two hours of test playback. There was no Kodi flicker (ISA automatically switched streams). So there is no automatic stream switching fault in that branch? I was previously using the PR #820 branch.

glennguy commented 2 years ago

@Liqianyu Current behaviour is that ISA will remember the download speed from last playback, and use that information when selecting which video stream to play when you start a new playback. If you've got a fast connection, there will be no reason to switch streams. The test stream mentioned in the PR is pretty slow to download, try playing that for a couple of minutes then go back to D+. Alternatively delete the bandwidth.bin file in the addon_data/inputstream.adaptive folder to reset the recorded download speed to 4Mbps, and make sure to play a stream with bandwidth higher than that.

glennguy commented 2 years ago

@CastagnaIT Seems like #6105 might have been to do with issues caused from having external subs (like from a local file) as well as ones contained within the file itself. This is pretty old and pre-VideoPlayer, it's worth trying to see if it causes any issues to remove it?

Liqianyu commented 2 years ago

@glennguy It could be that my internet speed is currently sufficient so that I can't trigger the automatic stream switch. I can't say for sure, a lot of the current ISA failures seem to be related to insufficient buffering (not enough internet speed). E.g. video stops playing (buffering at 0%) and the audio and subtitles continue. As well as known problems with rewind jams. Also turning on Kodi Player Debug Info causes frame skipping (not sure if this is ISA related), these issues seem to be non-existent or minimal on Kodi 19. Is it possible to merge PR #820 and #841 and compile them to facilitate my testing?

CastagnaIT commented 2 years ago

@Liqianyu for testing the changes you have to cherry-pick the commits from respective PR's branches then build it manually

The PR's could be joined because the problem now seem related to Kodi core only, but since we are not in a hurry i think it is better to wait a bit to better understand the problem caused by DEMUX_SPECIALID_STREAMCHANGE, trying to avoid making regressions in current functionality

i have tried comment the CloseStream(m_CurrentSubtitle, false); line and at least with local files i have not see any difference/problems by mixing external subs and video embedded CC subs, maybe is needed to try test with PVR? i have never used it, and i do not know how works, i will have to try follow some guides (if no specific hardware is required)

but also it seems that this change not fix our problem, i see if between today and tomorrow i can do some more tests

CastagnaIT commented 2 years ago

i have found this behaviour, i am testing always with trailers (to trigger more easily DEMUX_SPECIALID_STREAMCHANGE i am playing at same time two youtube videos, but ISA in future should have a setting to run this kind of demo-test), then example:

i have not understand in full how work all things, but this block of code https://github.com/xbmc/xbmc/blob/7478f983b67dc1e3b09e061acf2eaa675b0f0170/xbmc/cores/VideoPlayer/VideoPlayer.cpp#L925-L944 is the cause of our problems, recap: This block of code cause the redownloadings of sub stream, reinitialization of the kodi subtitle parser and the reparsing of subtitles currently shown on screen, plus, disabling the subtitles when it not match the kodi player settings

If we exclude this block of code when DEMUX_SPECIALID_STREAMCHANGE happen the subtitles works as expected so fix all ours problems, but to to this seems to suggest that we need to create a new type of DEMUX_SPECIALID_STREAMCHANGE in kodi I don't know if this is the right way but I can give it a try