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.

Sign up for GitHub

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

Jump to bottom

[KodiProps] Fix multi wrapper separator #1691

Merged
merged 1 commit into from
Oct 13, 2024
Merged

[KodiProps] Fix multi wrapper separator #1691

merged 1 commit into from
Oct 13, 2024

Conversation

knaerzche
Copy link
Contributor

@knaerzche knaerzche commented Oct 11, 2024 •
edited
Loading

According to the wiki, the separator for multiple wrappers is ,, not +.

Description

ADP::KODI_PROPS::CCompKodiProps::ParseDrmOldProps currently builds multiple wrappers with + as seperator which makes it unparsable for HelperWv::TranslateWrapper .

Motivation and context

Fix a bug.

How has this been tested?

With a stream using json,base64 (JB)

Screenshots (if appropriate):

Types of change

  • [ x] 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:

  • [x ] I have read the Contributing document
  • [ x] My code follows the Code Guidelines of this project
  • My change requires a change to the Wiki documentation
  • I have updated the documentation accordingly

CastagnaIT
CastagnaIT requested changes Oct 12, 2024
View reviewed changes
Copy link
Collaborator

@CastagnaIT CastagnaIT left a comment

Choose a reason for hiding this comment

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

this change is wrong,
as is also explained on wiki, the code its correct
i just forgot to fix the above method comment

@knaerzche
Copy link
Contributor Author

knaerzche commented Oct 12, 2024 •
edited
Loading

It's not like I scanned the comments in this code and adapted the code to the comment: Playing a stream with the mentioned "unwrapping method" fails with: Cannot translate license wrapper, unknown type json+base64 and this change fixes it. I didn't look too deep in this code, but I'd guess it comes from here:

inputstream.adaptive/src/CompKodiProps.cpp

Line 520 in 8720557

drmCfg.license.unwrapper = "json+base64";
- it uses + as seperator, not ,

@CastagnaIT
Copy link
Collaborator

CastagnaIT commented Oct 12, 2024 •
edited
Loading

yeah good catch
please fix all them with ,, and also the above method comment

@knaerzche
[KodiProps] Fix multi wrapper separator
365ffa4 
According to the wiki, the separator for multiple wrappers is ',', not '+'.
Fix it.

Signed-off-by: Alex Bee <[email protected]>
@knaerzche
Copy link
Contributor Author

Done. (Took some time for me to find the wiki entry you were referring to)

@knaerzche knaerzche changed the title HelperWv: Fix TranslateWraper [KodiProps] Fix multi wrapper separator Oct 12, 2024
CastagnaIT
CastagnaIT approved these changes Oct 13, 2024
View reviewed changes
Copy link
Collaborator

@CastagnaIT CastagnaIT left a comment

Choose a reason for hiding this comment

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

well done
thank you for your first contrib

@CastagnaIT CastagnaIT merged commit 933a28d into xbmc:Piers Oct 13, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@CastagnaIT CastagnaIT CastagnaIT approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@knaerzche @CastagnaIT