Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Fix Blu-Ray external subtitle load from movie folder BDMV/index.srt #4291

Closed
wants to merge 1 commit into from

7 participants

@amityony

Currently the Blu-Ray folder playback does not support external subtitle automatic load.
XBMC Eden did support the load of the BDMV/index.srt but it got broken due to the bluray:// url change.

This fix was tested on few BDMV folders and it now load the index.srt correctly.

@elupus
Collaborator

You should really check that it's the index file that has been started. We should not do this when starting menus, or when starting a specific title.

@amityony

1) When using menus, you still want to load the external subtitle file - index.srt.
2) The playback in the new XBMC versions is of MPLS playlist file and not index.bdmv. On any MPLS blu-ray playlist you want to load the index.srt file as in previous versions and in other streamers.
3) When starting a specific m2ts file, the prefix is not "bluray://" so it should be OK.

@elupus
Collaborator

1) Eh why? you will display strange subtitles in strange places.
2) Eh why? In that case you want to load the subtitle for the specific playlist file, not for the complete bluray. There can be multiple titles on blurays, and luckily they are easy to separate by playlist file.

I'm fine with index.* is used in the case when we start the "main" title file. But we don't do that, we should look for subtitles for the title we are playing.

@amityony

First you need to understand that in current xbmc release there is completely no support for external subtitle for BDMV folder. It is broken, since the name start with "bluray://" and does not enable folder scan.

I also agree with your comment.

We can use a different naming and place the SRT file not in index.srt, but with the same name of the MPLS playlist. This will solve the two issues, but it will be a different behavior from all the stremaers our there. It will also require the user to know what playlist is going to be played and name the SRT correctly with that number - not easy.

I can modify the patch to first check if there is an SRT with the MPLS playlist name, and only then check for index.srt. I am not sure that someone will really use that.

What do you think ?

@amityony

Another thing to mention, for Blu-Ray ISO the behavior is the same as the patch that I have added for BDMV. One SRT file, for any playlist you play.

@elupus
Collaborator

I've somewhat changed my mind. Since it's a very very special case situation. I'm okey with it as this pull is now. We can fix it better at some other stage.

@t-nelson

We can fix it better at some other stage.

Can, but rarely do. Historically we just let this type of stuff rot in some half-assed form. I'd really like to NOT allow stuff in until someone who is familiar with the area is truly willing to own it.

@amityony

This patch just fix external SRT for BDMV folder and restore the behavior as in XBMC 11. It got broken in version 12, since the movie name contains now "bluray://" prefix and no external SRT is loaded at all.
It does not add new functionality or break anything.

It is the same behavior (BDMV/index.srt) as in popular streams like: popcorn hour, dune, xtremer and others. It is also very similar to the ISO Blu-ray playback behavior in XBMC.

People other then me are complaining about this issue since version 12. You can see some complains here including demo video and screen capture:
http://forum.xbmc.org/showthread.php?tid=150699

Improvements are always welcome, but we don't yet know how to improve the behavior for external SRT. It seem that the current solution is good enough, since 99% of the movies that require external SRT have only one playable playlist and the user can always manually turn subtitle off if needed.

This problem in xbmc force many users to use commercial streams for Blu-ray playback with subtitles.
I would really like to see this problem solved as soon as possible.

@ace20022
Collaborator

This problem in xbmc force many users to use commercial streams for Blu-ray playback with subtitles.
I would really like to see this problem solved as soon as possible.

Loading ext subs via audiosettings should (always) work.

@amityony

I know, but doing it for every movie manually is not a solution. Specifically not for simple users that just expect the subtitle to show as for any other movie in any other device.

This bug need to be fixed.

@ace20022
Collaborator

This bug need to be fixed.

Of course, but it's not that highly dramatic as you stated.

@MartijnKaijser MartijnKaijser added this to the Pending for inclusion milestone
@Wanilton

@amityony, Hi, this work fine for load srt for BDMV folder, but don´t load external srt file when is ISO BD, put index.srt namemovie.srt or number mpls.srt togheter with ISO don´t go, try created manually folder BDMV, and put 3 srt files there, XBMC don´t load too. Now if i playing and get online subtitle, it´s work, for resume too, but if restart for begin, only get internal subtitle. If you have one idea how I solve this, I appreciate your job, thanks for try fix this situation.

@MartijnKaijser

@ace20022
thoughts on this for Helix or what needs to be changed?

@ace20022
Collaborator

I have very little time at the moment. Please re-ping me if I will not have picked it up till the end of July.

@ace20022
Collaborator

@amityony When I play a bluray folder strmovie is not prefixed with bluray:// . Could you please detail how you got the prefix? However playing isos the filename of the iso can be determined with

  CURL test;
  test.Parse(strMovie);
  CURL test2;
  test2.Parse(test.GetHostName());

@jmarshallnz is there an easier way?

@jmarshallnz
Owner

You can simplify that a bit, as in:

// iso files are bluray://<url_encoded_iso_path>/?someotherstuff?
std::string iso_path = CURL(strMovie).GetHostName();

Not sure what (if anything) the someotherstuff is :)

@ace20022
Collaborator

it's, e.g.,
strMovie = "bluray://udf%3a%2f%2fF%253a%255cVideo%255cblu%255cHDMV-2d%255cHDMV-2d.iso%2f/BDMV/MovieObject.bdmv"

So udf is also involved...

@MartijnKaijser

@amityony can you update this PR with the comments made?

@ace20022
Collaborator

@MartijnKaijser This is a replacement #5204

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 13 additions and 1 deletion.
  1. +13 −1 xbmc/Util.cpp
View
14 xbmc/Util.cpp
@@ -1906,7 +1906,19 @@ void CUtil::ScanForExternalSubtitles(const CStdString& strMovie, std::vector<CSt
URIUtils::Split(strArchive, strPath, strMovieFileName);
strLookInPaths.push_back(strPath);
}
-
+
+ if (StringUtils::StartsWith(strMovie, "bluray://"))
+ {
+ CURL url(strMovie);
+ CStdString strArchive = url.GetHostName();
+ URIUtils::Split(strArchive, strPath, strMovieFileName);
+ CStdString strPath2 = URIUtils::AddFileToFolder(strPath, "BDMV/");
+ strLookInPaths.push_back(strPath2);
+
+ // set the subtitle lookup name to be able to locate "BDMV/index.srt" in the blu-ray movie folder
+ strMovieFileNameNoExt = "index";
+ }
+
int iSize = strLookInPaths.size();
for (int i=0; i<iSize; ++i)
{
Something went wrong with that request. Please try again.