Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FIX: Only handle 3D bitmap subs in TAB #8254

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
8 participants
@koying
Copy link
Contributor

koying commented Oct 19, 2015

3D bitmap subs cannot be detected in SBS, and there is always the option
to play with Kodi 3D disabled

/cc @FernetMenta @da-anda @popcornmix

FIX: Only handle 3D bitmap subs in TAB
3D bitmap subs cannot be detected in SBS, and there is always the option
to play with Kodi 3D disabled
@da-anda

This comment has been minimized.

Copy link
Member

da-anda commented Oct 19, 2015

I'm pretty sure there are PGS subtitles out there that also cover more than 50% of the height, like when there is a forced sub at the bottom and someone is speaking and his sub is at the top (had that on some blu-rays already). So any assumption we make will for sure have cases where they won't work (like it's the case for SBS atm).

The only reliable way would probably be to do picture analysis, like on playback start, split a random subtitle frame, remove transparent pixels around every part (to compensate for stereoscopic depth offset/shifting) and then compare the two halfs. If they match, it's a 3D sub, if not, then 2D sub. At least in theory (I'm pretty sure antialiasing of PGS subs will prevent pixel perfect matching).

But I suppose this will be to complicated to do for a fix, so we might be better off to add a setting to video OSD "stereoscopic subtitle: yes/no" that's stored along with stream details. On playback we check if this flag is enabled for the current subtitle stream and split it according to the movies 3D mode (SBS or TAB).

Any other ideas?

@FernetMenta

This comment has been minimized.

Copy link
Member

FernetMenta commented Oct 21, 2015

I would merge this until there are better/other ideas.

@Sorien

This comment has been minimized.

Copy link

Sorien commented Oct 21, 2015

@da-anda or maybe analyze it with some OCR like https://github.com/tesseract-ocr/tesseract

@da-anda

This comment has been minimized.

Copy link
Member

da-anda commented Oct 21, 2015

@Sorien good luck with analyzing Na'vi font of Avatar ;)

@FernetMenta I'd go with a setting, as with this PR everybody having SBS pixel subs (which tools like DVDFab create by default when ripping) will be bitching that subs are not working anymore in 3D mode (if I understood the code correctly)

@koying

This comment has been minimized.

Copy link
Contributor Author

koying commented Oct 21, 2015

I wonder where SBS comes from, really...
Framepacking standard is TAB, passive 3D TV's want TAB or they loose resolution twice, ...

@da-anda

This comment has been minimized.

Copy link
Member

da-anda commented Oct 21, 2015

well, most TVs are shutter these days, so for these it doesn't matter. And with SBS you can still crop black bars to save a few MB while on TAB you can't. And isn't there something that stretching a pixel in a certain direction is perceived as "better looking" than the other by the human eye/brain?

@koying

This comment has been minimized.

Copy link
Contributor Author

koying commented Oct 26, 2015

Can we agree to merge this one as a fix to the original problem?
Not perfect but at least it won't cause issue (besides the very specific TAB case highlighted by da-anda, which is low occuring)

@da-anda

This comment has been minimized.

Copy link
Member

da-anda commented Oct 26, 2015

I'm not against merging, but merging it will only shift issues. Current codebase fixes these two issue

As VLC seems to be able to handle all scenarios correctly, maybe we can have a look on how they did it?

@koying

This comment has been minimized.

Copy link
Contributor Author

koying commented Oct 26, 2015

Well, the simple alternative is to leave the logic as it is, both for SBS and TAB (I'd still do a fix, because render_stereo_mode != RENDER_STEREO_MODE_OFF was quite stupid of me).

It'll be a known issue that MVC subs (where we can safely assume they are not 3D) in SBS won't work until a larger PR implements a 3D sub setting (if there's a consensus on it), but SBS 3D sub will still work on "plain" HSBS, where we can safely assume 3D subs.

Re VLC, I'll quote you "It's only "working" in VLC, because it does no stereoscopics conversion, or does it by now?". Do they, now? ;)

@da-anda

This comment has been minimized.

Copy link
Member

da-anda commented Oct 26, 2015

well, we don't have official MVC support yet, or do we by now on a platform? On the PI it's still only in experimental builds. And if the SBS issue is only related to MVC files, I think some stream flag might be better suited than a setting. But I don't know enough of stream processing if this would be a viable solution (only remember bits from the time I messed with the 3D detection stuff on non-mkv)

RE VLC - I didn't test it myself, but users said that VLC is handling the subs "correct" in 2D (if I recall correctly). So I assumed they talked about converting a 3D movie to 2D. If it was for MVC streams, than it's ofc obvious why it's working in VLC (as they don't process it). I know, never trust a user :)

@koying

This comment has been minimized.

Copy link
Contributor Author

koying commented Oct 26, 2015

Indeed. We might delay proper handling of 2D subs in MVC to MVC merge time (post videoplayer, I assume), especially considering TAB would still work and "official" framepacking is TAB.
@popcornmix what is the stereo mode for framepacking? SPLIT or something else?

@popcornmix

This comment has been minimized.

Copy link
Member

popcornmix commented Oct 26, 2015

The hdmi mode for framepacking is 1920x2205 (with a 45 pixel gap between top and bottom eyes).
However on Pi we don't render the GUI at that resolution (the memory usage and framerate takes too much of a hit). So we use either half-SBS or half-TAB for the GUI (whichever is chosen in OSD, or through filename tags).

The video isn't rendered through GL on pi, so the gui choice doesn't matter. The firmware knows if the HDMI mode is half-SBS, half-TAB, or FP, and so composites the two eyes as appropriate.

I think at decode time you need need to flag whether you believe the sub is full, or half-SBS or half-TAB (possibly this needs some heuristics based of bitmap size or filename flags, or 3D mode of the encoded video). At render time you need to check what the GUI mode is, and scale/transform the subs to match as they may not be the same.

However, from the Milhouse builds, it seems this PR does improve things for some users and I haven't seen any regressions reported, so I don't mind this being merged with a note that further improvements are desired.

@MilhouseVH

This comment has been minimized.

Copy link
Contributor

MilhouseVH commented Dec 15, 2015

Can this be rebased on master?

Are there any plans to merge this for Jarvis?

asavah added a commit to asavah/xbmc that referenced this pull request Oct 25, 2016

@Rechi

This comment has been minimized.

Copy link
Member

Rechi commented Aug 14, 2017

@koying this needs a rebase

@koying

This comment has been minimized.

Copy link
Contributor Author

koying commented Aug 15, 2017

Abandonned

@koying koying closed this Aug 15, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.