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

[New feature] External subtitles over UPnP #6184

Merged
merged 3 commits into from
Jan 31, 2015

Conversation

7pepo7
Copy link

@7pepo7 7pepo7 commented Jan 11, 2015

Like in title, it allows to play video with external subtitle over UPnP.

Supported subtitle file extensions: txt, srt, ssa, ass, sub, smi. (most
popular ones)

Subtitle files are searched like in internal player (in video file
folder and in folders specified in options). If more than one subtitle
file are found (multiple language versions), Kodi takes the one with
language choosed in options (prefered language for subtitles), otherwise
first found.

Subtitles are added as 2 resources, 2 sec resources and 3 attributes added to
video file resource, for greater compatibility with more UPnP devices.
Device take the last one it could handle, and skip ones it can't
"understand":

<res protocolInfo="http-get::video/mp4:" xmlns:pv="http://www.pv.com/pvns/" pv:subtitleFileUri="http://192.168.1.13:1722/video.srt" pv:subtitleFileType="SRT">http://192.168.1.13:1722/video.mp4 </res>
<res protocolInfo="http-get::text/srt:"> http://192.168.1.13:1722/video.srt </res>
<res protocolInfo="http-get::smi/caption:"> http://192.168.1.13:1722/video.srt </res>
<sec:CaptionInfoEx sec:type="srt">http://192.168.1.13:1722/video.srt&lt;/sec:CaptionInfoEx>
<sec:CaptionInfo sec:type="srt">http://192.168.1.13:1722/video.srt&lt;/sec:CaptionInfo>

Samsung devices get subtitles uri from "CaptionInfo.sec" field in http response header for movie file request (it contains "getCaptionInfo.sec" field).

This all should be (and probably it would be) changed, when Profile Manager (for DLNA devices) will be available in Kodi.

while (entry)
{
didl += " ";
PLT_Didl::AppendXmlEscape(didl, (NPT_String&)(*entry)->GetKey());

This comment was marked as spam.

@Montellese
Copy link
Member

I don't use subtitles but this looks like it will make a lot of people happy.

Please split this into three commits:

  1. Only the changes to lib/libUPnP/Platinum/
  2. Add a patch file of the previous commit to lib/libUPnP/patches
  3. All the other changes to our UPnP integration

@7pepo7
Copy link
Author

7pepo7 commented Jan 12, 2015

Thanks for review. I'll try to do changes as quickly as possible. But I have few questions. I'm new in github. I work in Mercurial with SourceTree and mostly for sync my code between work and home, not for team work, so I didn't make pull requests and patches. Could you explain me how to split this to three commits: make new 3 commits and new pull request or something else? What with this pull request? And how to make a patch and add it to remote folder. Sorry for neewbe questions, but I prefer to ask than to do something for which you want to kill me :D Please for understanding :D

@Montellese
Copy link
Member

No need for hurry. I can only provide you the git bash commands as I'm not using any GUI for git. You need to be on the branch for this PR i.e. master-subtitles.

  1. git reset --soft HEAD^ will remove the commit but keep the changes staged
  2. git reset HEAD will unstage the changed files but still keep the changes
  3. Make the necessary changes to the source code and make sure it still compiles and works
  4. Use git add for every changed file in lib/libUPnP/Platinum/
  5. Use git commit -m "<message>" to commit those changes
  6. git format-patch -1 will create a .patch file in your working directory of the previous commit.
  7. Move that file to lib/libUPnP/patches and change the leading number accordingly to be the last in the list of the patches
  8. Use git add on the patch file
  9. Use git commit -m "<message>" to commit the patch file
  10. Use git add to add the remaining files/changes
  11. Use git commit -m "<message>" to commit those changes
  12. git push -f origin master-subtitles will force push your changes to your remote branch and this PR will automatically be updated.

@7pepo7
Copy link
Author

7pepo7 commented Jan 12, 2015

Wow, thank you for so detailed explanation. I'll make changes as soon as I find some time. Thanks again.

Added custom data to resources field - needed to add some attributes to res.
Added new struct PLK_SecResource - needed by Somasung devices. It's not specialized struct (just general one), because I can't find any "sec" specification.
@7pepo7 7pepo7 force-pushed the master-subtitles branch 3 times, most recently from d639b95 to ad6aa8e Compare January 21, 2015 18:59
@7pepo7
Copy link
Author

7pepo7 commented Jan 22, 2015

OK. If you could review it, Montellese.

I didn't change logic for searching best subtitles, cause in DVDPlayer it's based on streams, and here it's worked on files (on paths). I did'nt want to make changes in DVDPlayer class.

It's all should be changed when ProfileManager will be available for Kodi. Now subtitles are added in any available way, to support most of devices. But it should add subtitles only in one way (specific for target device), descibed in Profile for target device. I don't have too much free time, but I'll try to implement ProfileManager feature (but it could take some time). For now I think, that this solution will make some people happy (mostly no English-speaking).

@Montellese
Copy link
Member

Thanks for the adjustments. There are only a few minors left in the last commit.

@Montellese Montellese added Type: Feature non-breaking change which adds functionality v15 Isengard labels Jan 22, 2015
@7pepo7
Copy link
Author

7pepo7 commented Jan 22, 2015

Ok, I made adjustments. Thanks for guide me in git.

@7pepo7
Copy link
Author

7pepo7 commented Jan 22, 2015

I think that now it's ok. Sorry for that. It's Visual Studio and its automatic tabulation.

@Montellese Montellese added this to the I******* 15.0-alpha2 milestone Jan 22, 2015
@Montellese Montellese self-assigned this Jan 22, 2015
@Montellese
Copy link
Member

Looks good for the next merge window. Thanks for taking the time to clean this up.

@ace20022
Copy link
Member

Just a general note, preferred is misspelt.

Supported subtitle file extensions: txt, srt, ssa, ass, sub, smi. (most popular ones)

Subtitle files are searched like in internal player (in video file folder and in folders specified in options). If more than one subtitle file are found (multiple language versions), Kodi takes the one with language choosed in options (prefered language for subtitles), otherwise first found.

Subtitles are added as 2 resources, 2 sec resources and 3 attributes added to video file resource, for greater compatibility with more UPnP devices. Device take the last one it could handle, and skip ones it can't "understand":

<res protocolInfo="http-get::video/mp4:" xmlns:pv="http://www.pv.com/pvns/" pv:subtitleFileUri="http://192.168.1.13:1722/video.srt" pv:subtitleFileType="SRT">http://192.168.1.13:1722/video.mp4 </res>
<res protocolInfo="http-get::text/srt:"> http://192.168.1.13:1722/video.srt </res>
<res protocolInfo="http-get::smi/caption:"> http://192.168.1.13:1722/video.srt </res>
<sec:CaptionInfoEx sec:type="srt">http://192.168.1.13:1722/video.srt</sec:CaptionInfoEx>
<sec:CaptionInfo sec:type="srt">http://192.168.1.13:1722/video.srt</sec:CaptionInfo>

Samsung devices get subtitles uri from "CaptionInfo.sec" field in http response header for movie file request (it contains "getCaptionInfo.sec" field).

This all should be (and probably it would be) changed, when Profile Manager (for DLNA devices) will be available in Kodi.
@7pepo7
Copy link
Author

7pepo7 commented Jan 22, 2015

Thanks for notice, ace20022 (english it's not my primary language). Corrected.

@Montellese
Copy link
Member

jenkins build this please

@ace20022
Copy link
Member

Thanks for notice, ace20022

Thanks for this great contribution! As Montellese already said: "it'll make a lot of people happy" 👍

@MartijnKaijser
Copy link
Member

@Montellese can we get your alpha2 milestone list merged as well?

@Montellese
Copy link
Member

@MartijnKaijser: I don't follow. All of the PRs I've assigned to the next merge window should be ready to be merged (and have been built by jenkins).

@MartijnKaijser
Copy link
Member

it's not worth releasing a first alpha1 while we still have frequent crashing on Android-x86 and Windows. So might as well already get the rest in and close merge window nearing ending of February. That way you can also go through your PRs more rapid.

Montellese added a commit that referenced this pull request Jan 31, 2015
upnp: support for external subtitles
@Montellese Montellese merged commit 30350ff into xbmc:master Jan 31, 2015
@MartijnKaijser MartijnKaijser modified the milestones: I******* 15.0-alpha2, I******* 15.0-alpha1 Feb 1, 2015
@Forage
Copy link

Forage commented Feb 3, 2015

Not sure to what extend the bug tracker is being monitored for this feature, but this PR mean that at least http://trac.kodi.tv/ticket/15142 can be closed/updated.

@NedScott
Copy link
Contributor

Just a heads up, it seems this is causing quite a heavy load for some people's networks: http://forum.kodi.tv/showthread.php?tid=220840

@Montellese
Copy link
Member

@NedScott: Thanks for the hint. The logic should probably be adjusted to only look for external subtitles and provide them in the DIDL response if an item is starting to play. In all other cases the information about additional subtitles is not really of any use.

@7pepo7: Do you have time to look into this?

@7pepo7
Copy link
Author

7pepo7 commented Mar 11, 2015

Ok. I'll take a look into this at saturday (sorry, no time at the moment).

@Montellese
Copy link
Member

I've taken a stab at this in https://github.com/Montellese/xbmc/commits/upnp_fix_ext_subtitles but I won't be able to test it myself but a forum user has volunteered. Will report back once I get some test results.

xaiki pushed a commit to butterproject/butter-desktop that referenced this pull request Oct 23, 2015
Implemented streaming of subtitles over DLNA/UPnP

I noticed Kodi (XBMC) has recently added the feature of transmitting external subtitles over UPnP: xbmc/xbmc#6184

I got inspired and decided to try implementing it for Popcorn Time. I did a little study regarding the UPnP protocol, and found out that it is possible to pass a xml-encoded metadata to the UPnP renderer device. You have to fill the `CurrentURIMetaData` parameter of the `SetAVTransportURI` function of the `AVTransport`. The nodejs module `upnp-mediarenderer-client` handles these messages, and it is already used by Popcorn Time to transmit video through UPnP. Fortunately, its latest version (0.0.3) added support for passing the metadata information to the client object.

The XML metadata contains the URIs of the video and of the subtitle, disposed in several different ways. This is necessary to maximize the compatibility with different UPnP/DLNA devices, since each vendo implements the retrieval of subtitles over UPnP in different ways. For instance, Samsung devices inspect the CaptionInfo information. I also noticed that some devices (XBMC, for example) require a title for the video. This is an example of the XML tree I'm using so far, and seems to be working well:

```XML
<DIDL-Lite xmlns="urn:schemas-upnp-org:metadata-1-0/DIDL-Lite/" xmlns:dc="http://purl.org/dc/elements/1.1/" xmlns:upnp="urn:schemas-upnp-org:metadata-1-0/upnp/" xmlns:dlna="urn:schemas-dlna-org:metadata-1-0/" xmlns:sec="http://www.sec.co.kr/" xmlns:xbmc="urn:schemas-xbmc-org:metadata-1-0/">
    <item id="0" parentID="-1" restricted="1">
        <dc:title>Popcorn Time Video</dc:title>
        <res protocolInfo="http-get:*:video/mp4:" xmlns:pv="http://www.pv.com/pvns/" pv:subtitleFileUri="http://192.168.1.3:9999/video.srt" pv:subtitleFileType="srt">http://192.168.1.3:60082/video.mp4</res>
        <res protocolInfo="http-get:*:text/srt:*">http://192.168.1.3:9999/video.srt</res>
        <res protocolInfo="http-get:*:smi/caption:*">http://192.168.1.3:9999/video.srt</res>
        <sec:CaptionInfoEx sec:type="srt">http://192.168.1.3:9999/video.srt</sec:CaptionInfoEx>
        <sec:CaptionInfo sec:type="srt">http://192.168.1.3:9999/video.srt</sec:CaptionInfo>
        <upnp:class>object.item.videoItem.movie</upnp:class>
    </item>
</DIDL-Lite>
```

So far, the metadata is sent only if there's a subtitle selected/available. Notice that it can be used to send many other information, like description, date and even the cover/poster art images. For future versions, it would be interesting seeing, for example, the posters of the movies in external DLNA players.

I was able to test and confirm it is working in these devices:
- Kodi 14.1
- Raspbmc (XBMC for Raspberry Pi)
- LG TV 2012
- LG Home Theater 2014 (partially)

The LG HT was crashing at the beginning. I noticed the HT was indeed interpreting the metadata correctly, and was even requesting the "video.srt" file. Later, I realized that the issue was that, due to a bug in Popcorn Time's subtitle server (which does not handle partial range-based http requests of the file), the HT was indefinitely requesting the subtitle file from Popcorn Time, and this was causing both PT and the HT to crash. I figured out a way to fix this, but I plan to create a separate issue and merge request for this, since this touches more files and (hopefully) affects only some devices. This crash is apparently rare, and does not happen when not using subtitles.

**Steps to test the feature:**
1. Turn on and setup a DLNA/UPnP renderer device in the same network of the Popcorn Time.
2. Open Popcorn Time in your computer, and select a movie/tv in the main screen;
3. Select a subtitle language from the list;
4. Select your DLNA/UPnP device from the list of external players;
5. Click on play, and wait for the video to start streaming in the device;
6. Test either subtitles are shown or not.

Please report the vendor/type of your device or software used as DLNA/UPnP player.

See merge request !17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Feature non-breaking change which adds functionality v15 Isengard
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants