[ASS] - fixed display of time overlapping ass subs #1491

Merged
merged 1 commit into from Sep 27, 2012

Conversation

Projects
None yet
4 participants
Owner

Memphiz commented Sep 27, 2012

This fixes the display of multiple ASS subtitle texts which are overlapping in time.

Example subfile here:

http://pastebin.com/qWSTbvJ8

This is how it looks now (b0rked):

https://www.youtube.com/watch?v=kUiFHHhYO6c

And this is how it looks with this fix:

https://www.youtube.com/watch?v=hXDJhNOUlOQ

Not quiet sure if this fix is correct though. @elupus one more for the queue ;)

The issue was reported here:

http://forum.xbmc.org/showthread.php?tid=141266&pid=1199684#pid1199684

Member

elupus commented Sep 27, 2012

Looks good

Memphiz added a commit that referenced this pull request Sep 27, 2012

Merge pull request #1491 from Memphiz/asssubs
[ASS] - fixed display of time overlapping ass subs

@Memphiz Memphiz merged commit e8b1903 into xbmc:master Sep 27, 2012

jpsdr commented on 0c76257 Sep 28, 2012

As we are inside the play, shouldn't code be "optimised" ?
What's the point of testing again (pOverlay) with the "if", because if the while is true, so "if" is...
Unless there is an unknow reason i didn't know, code should be :
While(pOverlay)
{
m_pOverlayContainer->Add(pOverlay);
pOverlay = m_pSubtitleFileParser->Parse(pts);
}

Could it be related to the ticket i've opened ?
http://trac.xbmc.org/ticket/13302
I'll test this WE, and update ticket if it solves it.

Owner

Memphiz replied Sep 28, 2012

There is no point in checking the pointer again (its just a leftover from wrapping that code with the loop. Thx for the headsup.

And yes this could be related to the ticket you have opened. Let me know if this fixes your ticket to.

It solves the issue, but...!!!
The insane CPU usage mentioned in the forum in the feature/10bit thread on some subtitles is back.
Here : http://forum.xbmc.org/showthread.php?tid=106051&pid=1101184#pid1101184
I kept the file for testing, and mentioned part (at around 19:50) is again unplayable on my i7@870. File is played perfectly fine with MPC-HC + Directvobsub, so, insane CPU seems to be XBMC specific related.
You can get this file ([Tsumiki]Acchi_Kocchi-02[10bit][1280x720][529E1BBB].mkv) either on newsgroup (alt.binaries.multimedia.anime.highspeed) or try, if still alive, here : http://www.nyaa.eu/?page=torrentinfo&tid=304793

I've try to take a look, but it's out of my knowledge. The only thing i think it's that maybe it mess up or make the work of ellupus on his pull request #942 useless, and this fix has to be implemented/integrated more with this PR... It's just a maybe...

I think the if statement is not needed. Only while statement is enough.

Owner

Memphiz commented Sep 29, 2012

That example is one bad ass abuse of ass subtitles really.

@elupus well i have no idea howto improve this. They just spam with subtitles in that example and around 19:53. Not really sure if the rendering is causing the cpu peak or the traversing for getting all subs which have to be displayed for one pts (traversing through that single linked list should be fast as hell imho). But on my c2d 2.4ghz i see frames dropping on that scene even if cpu load is around 80% (not maxed out!).

Owner

Memphiz commented Sep 29, 2012

and fyi - it drops frames on that scene even without my change for me.

Contributor

jpsdr commented Sep 30, 2012

For me, it was played fine(1) without your change (but i'm using a custom build with MT decoding enabled, wich may help more), but i think this case is somehow... don't know how to translate "a schoolar case". But the fact that MPC-HC+directvobsub play it perfectly fine, make me think, that, maybe, there is also a way for xbmc to play it fine also...
(1) : I've not hit "o" to see if there was drop frames, just watching is display seemed corret. Considering the static case of animation, i wouldn't have noticed a few dropped frames. But now, xbmc praticaly stop, audio stop, etc...
Nevertheless, i think this case is somehow maybe not unique, but very rare, and is absolutely not a reason to remove your fix, wich fixes a loooooot more that it may eventualy rarely broke...
But, again, MPC-HC and VLC 2.0.3 (i've just made the test), play it perfectly fine also on my other PC (i7@860) where xbmc failled totaly, means that it should probably be possible for xbmc to play it also. Finding the way may probably globaly find a way to reduce other possible cases of high cpu usage.

Contributor

jpsdr commented Oct 1, 2012

Cases may not be so rare finaly, even if they are still not frequent.
I've encounter another file (get here : http://www.nyaa.eu/?page=torrentinfo&tid=355408 or newsgroup), issue is not as bad as previous file, but result is still a little mess. It's at the end of the ending (around 23:30/23:40).
Don't worry, i don't intend to post each file i may encounter. This is just to give another sample, and just warn that maybe problem may happend more often than i thought.

Contributor

jpsdr commented Oct 13, 2012

I don't know what broke it, but actualy the issue i've describe in the ticket 13302 http://trac.xbmc.org/ticket/13302 is back.

Owner

Memphiz commented Oct 13, 2012

works here (tested on osx and atv2 - atv2 won't play this stutterfree but subs are there) - maybe some hi10 media or so)

Contributor

jpsdr commented Oct 14, 2012

works here (tested on osx and atv2 - atv2 won't play this
stutterfree but subs are there) - maybe some hi10 media or so)

Strange. With the build i've made the 6/10 the issue i've described in my ticket wasn't here, your patch fixed it, but with the build i've made ysterday, the 13/10, the issue is back. I've just at the moment downloaded XBMCSetup-20121013-e331bf6-master.exe and test it, and issue is back indeed. Have you exactly tested what i've specificaly described in my ticket post with the file (small, 15MB) i've provided ? The "fix" i've described in #942 (comment) is still working and solve the issue, but it's not of course a good final solution...

Owner

Memphiz commented Oct 14, 2012

you are only testing on windows?

Contributor

jpsdr commented Oct 14, 2012

you are only testing on windows?

Yes.

LongChair added a commit to plexinc/plex-home-theater-public that referenced this pull request Feb 5, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment