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

Support UTF-8 Encoded M3U Playlists #21992

Merged
merged 1 commit into from Dec 18, 2022
Merged

Support UTF-8 Encoded M3U Playlists #21992

merged 1 commit into from Dec 18, 2022

Conversation

complexlogic
Copy link
Contributor

@complexlogic complexlogic commented Oct 6, 2022

Description

This PR adds support for UTF-8 encoded M3U playlists with file extension .m3u8.

Current Behavior

Kodi can read UTF-8 encoded data in an M3U playlist created by another program, but only for the .m3u file extension, not .m3u8. The bigger issue is with the playlist editing feature. When saving a playlist, Kodi strips out any characters that are not in the locale's default non-Unicode encoding. This means that any files with paths that contain Unicode characters can't be played back, even though Kodi never raised any errors when the playlist was saved.

Proposed Implementation

Write M3U8 format by default for all new playlists. When editing existing playlists, preserve its file extension.

Motivation and context

This has been a longstanding issue and feature request. See the following:

How has this been tested?

  • Verified new playlists are created as .m3u8
  • Verified saving an existing .m3u playlist does not convert it to .m3u8
  • Verified HLS streaming still works (it also uses .m3u8 file extension)

What is the effect on users?

Feature request is implemented. See Motivation and context section

Screenshots (if appropriate):

N/A

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

@lrusak lrusak added this to the Nexus 20.0 Beta 1 milestone Oct 7, 2022
@lrusak lrusak added Type: Improvement non-breaking change which improves existing functionality v20 Nexus labels Oct 7, 2022
@fuzzard
Copy link
Contributor

fuzzard commented Oct 8, 2022

Out of curiosity. Is there really a need for the advanced setting? Could we just not go to the m3u8 and call it a day?
We would still be able to open an m3u if i understand correctly, its just the writing would become an m3u8.

You mention sharing with "legacy" apps, if users wanted/needed that, could they not just curate their m3u lists in some other tool, and then that m3u list can be imported into kodi, but that same list can be used for their "legacy" app.

My query really stems from the "another advanced setting" standpoint.

@complexlogic
Copy link
Contributor Author

@fuzzard Yes, I am fine with removing the advanced setting if you think it is not warranted. I would assume most people that create playlists within Kodi don't intend to share them with other players, anyway.

@fuzzard
Copy link
Contributor

fuzzard commented Oct 8, 2022

Cool, hold off on the work though, i was just throwing out a thought really.

Maybe @DaveTBlake or @the-black-eagle can chime in from a Music perspective inside kodi.

@scott967
Copy link
Contributor

scott967 commented Oct 8, 2022

I can just say from my perspective, m3u/m3u8 files relate to filenames, so if a device/software can't handle utf-8 in m3u it probably can't handle the filenames either. But as it exists m3u files created in Kodi are worthless for me in Kodi or any other device since all unicode outside of latin1 or cp437 are replaced with spaces.

@fuzzard
Copy link
Contributor

fuzzard commented Nov 5, 2022

@complexlogic if you have time, lets go ahead with dropping the advanced setting. We'll just go with saving m3u8 by default regardless. I know other devs have been trying to bring more natural unicode support into other areas, so this is an easy win with minimal downsides.

@complexlogic
Copy link
Contributor Author

@fuzzard I just made the following changes:

  • Rebase against master
  • Remove the advanced setting. Use .m3u8 format for all new playlists, preserve format when editing an existing playlist
  • Update the PR description

Please let me know if anything further is required.

@jenkins4kodi
Copy link
Contributor

I've made some formatting changes to meet the current code style. The diffs are available in the following links:

For more information please see our current code style guidelines.

Comment on lines +72 to +74
bool utf8 = false;
if (URIUtils::GetExtension(strFileName) == ".m3u8")
utf8 = true;
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is appropriate to be inline

Suggested change
bool utf8 = false;
if (URIUtils::GetExtension(strFileName) == ".m3u8")
utf8 = true;
bool isUtf8 = URIUtils::GetExtension(strFileName) == ".m3u8";

Comment on lines +220 to +222
bool utf8 = false;
if (URIUtils::GetExtension(strFileName) == ".m3u8")
utf8 = true;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
bool utf8 = false;
if (URIUtils::GetExtension(strFileName) == ".m3u8")
utf8 = true;
bool isUtf8 = URIUtils::GetExtension(strFileName) == ".m3u8";

@fuzzard fuzzard merged commit 99b9f11 into xbmc:master Dec 18, 2022
@fuzzard
Copy link
Contributor

fuzzard commented Dec 18, 2022

Thanks for the PR @complexlogic . we'll merge early for v21, and see if any issues come up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Improvement non-breaking change which improves existing functionality v21 Omega Wiki: Done
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants