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

[upnp]: add all KODI supported real subtitle formats when act as a DLNA render #22038

Merged
merged 1 commit into from Oct 18, 2022

Conversation

ihipop
Copy link
Contributor

@ihipop ihipop commented Oct 16, 2022

upnp: add all KODI supported real subtitle formats when act as a DLNA render
and share the white list with the KODI DLNA server

Description

Motivation and context

Reading :

How has this been tested?

  • as a DLNA render:

local build pass and tested with emby dlna server,all subttitles functional

  • as a UPNP/DLNA server:

local build pass and tested with Bubble upnp app

  • ENV
Distributor ID:	ManjaroLinux
Description:	Manjaro Linux
Release:	22.0.0
Codename:	Sikaris

What is the effect on users?

user will have a full functional subtitle selection when play video by supported DLNA control point

Screenshots (if appropriate):

2022-10-12_14-45

Types of change

  • Bug fix (non-breaking change which fixes an issue)
  • Clean up (non-breaking change which removes non-working, unmaintained functionality)
  • Improvement (non-breaking change which improves existing functionality)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that will cause existing functionality to change)
  • Cosmetic change (non-breaking change that doesn't touch code)
  • None of the above (please explain below)

Checklist:

  • My code follows the Code Guidelines of this project
  • My change requires a change to the documentation, either Doxygen or wiki
  • I have updated the documentation accordingly
  • I have read the Contributing document
  • I have added tests to cover my change
  • All new and existing tests passed

@ihipop ihipop force-pushed the fix-upnp-dlna-render-subtitle-whitelist branch 4 times, most recently from 3305fc0 to 5c7f320 Compare October 16, 2022 16:32
@ihipop ihipop force-pushed the fix-upnp-dlna-render-subtitle-whitelist branch from 5c7f320 to 8b9fd24 Compare October 17, 2022 00:41
@enen92 enen92 added Type: Fix non-breaking change which fixes an issue Type: Improvement non-breaking change which improves existing functionality v20 Nexus Component: UPnP labels Oct 17, 2022
@enen92 enen92 added this to the Nexus 20.0 Beta 1 milestone Oct 17, 2022
Copy link
Member

@enen92 enen92 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for the contribution and for fixing this. Left a few comments where I think this can be improved

xbmc/network/upnp/UPnPInternal.cpp Outdated Show resolved Hide resolved
xbmc/network/upnp/UPnPInternal.cpp Outdated Show resolved Hide resolved
xbmc/network/upnp/UPnPInternal.cpp Outdated Show resolved Hide resolved
xbmc/network/upnp/UPnPInternal.cpp Outdated Show resolved Hide resolved
xbmc/network/upnp/UPnPInternal.cpp Outdated Show resolved Hide resolved
xbmc/network/upnp/UPnPInternal.cpp Outdated Show resolved Hide resolved
xbmc/network/upnp/UPnPInternal.cpp Outdated Show resolved Hide resolved
xbmc/network/upnp/UPnPInternal.cpp Outdated Show resolved Hide resolved
@ihipop ihipop force-pushed the fix-upnp-dlna-render-subtitle-whitelist branch 4 times, most recently from 53e0d68 to f4f7301 Compare October 18, 2022 06:44
@ihipop ihipop requested a review from enen92 October 18, 2022 06:56
Copy link
Member

@enen92 enen92 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Please squash the commits

@enen92 enen92 closed this Oct 18, 2022
@enen92 enen92 reopened this Oct 18, 2022
…and share the white list with the kodi DLNA server
@ihipop ihipop force-pushed the fix-upnp-dlna-render-subtitle-whitelist branch from f4f7301 to 5a7c335 Compare October 18, 2022 08:52
@ihipop ihipop requested a review from enen92 October 18, 2022 08:53
Copy link
Member

@enen92 enen92 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks will be merged once jenkins gets green

@enen92 enen92 added the hacktoberfest-accepted Hacktoberfest tagging label Oct 18, 2022
@ihipop
Copy link
Contributor Author

ihipop commented Oct 18, 2022

Thanks will be merged once jenkins gets green

did jenkins is building against wrong commit?

image

@enen92 enen92 closed this Oct 18, 2022
@enen92 enen92 reopened this Oct 18, 2022
@enen92
Copy link
Member

enen92 commented Oct 18, 2022

Thanks will be merged once jenkins gets green

did jenkins is building against wrong commit?

image

It's building a merge commit of yours on top of the master head commit:
dd37b9b

image

@enen92 enen92 merged commit a72285c into xbmc:master Oct 18, 2022
@ihipop ihipop deleted the fix-upnp-dlna-render-subtitle-whitelist branch October 19, 2022 08:52
ihipop added a commit to ihipop/xbmc that referenced this pull request Oct 19, 2022
ihipop added a commit to ihipop/LibreELEC.tv that referenced this pull request Oct 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Subtitles Component: UPnP hacktoberfest-accepted Hacktoberfest tagging Type: Fix non-breaking change which fixes an issue Type: Improvement non-breaking change which improves existing functionality v20 Nexus
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants