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

[DashTree] Changes to parsing values from "Role" tag #1771

Merged
merged 2 commits into from
Feb 19, 2025

Conversation

CastagnaIT
Copy link
Collaborator

Description

Changes on "Role" tag value parsing:

Disabled "subtitle", "caption" uses

"subtitle" looks like a kind of old workaround to set content type,
but "should" be used only to signal burned-in subtitles to video stream (not CC 608 etc..)

"caption" instead has been used to set impaired flag, imo its not correct thing
its not a rule that "caption" stand for impaired, as you can read also
on wikipedia this should not be a rule, but only some countries may use it as rule

IOP/ISO EIC specs say that "subtitle" and "caption" have same meaning of burned-in subtitles
so lets try to remove both and we will see if someone complains

(compared to the past the current source code have more robust code to identify the “content type” so it should be low risk of breaking some service by removing "subtitle" case value)

Deprecated custom "forced" support

i have implemented "forced" value years ago, i was not aware of "forced-subtitle" official specs value,
so lets deprecate it, for future removal

Motivation and context

fix #1768

How has this been tested?

cant be test by my part

Screenshots (if appropriate):

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:

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

"subtitle" looks like a kind of old workaround to set content type,
but "should" be used only to signal burned-in subtitles to video stream (not CC 608 etc..)

"caption" instead has been used to set impaired flag, imo its not correct thing
its not a rule that "caption" stand for impaired, as you can read also
on wikipedia this should not be a rule, but only some countries may use it as rule

IOP/ISO EIC specs say that "subtitle" and "caption" have same meaning of burned-in subtitles
so lets try to remove both and we will see if someone complains
@CastagnaIT
Copy link
Collaborator Author

@matthuisman have you some custom uses of "Role" tag values?
in case let me know if this code change may result in any headaches

@matthuisman
Copy link
Contributor

i think i can just add both forced and forced-subtitle to keep backwards compatbility?
And swap to alternate or commentary instead of caption

@matthuisman
Copy link
Contributor

matthuisman commented Jan 30, 2025

actually, i think im OK

I do have some code to work-around IA not supporting this already:
image

Thats all i do in regards to "Role"
In the above - i just set forced on the adaption set if i see forced-subtitle role

@CastagnaIT
Copy link
Collaborator Author

CastagnaIT commented Jan 31, 2025

yes you are doing right things
then you dont need change nothing
for Kodi 22 if you want you can remove the setAttribute “forced” from Role since that case will be handled

PS. anyway "forced" attrib to AdaptationSet will be kept, is not the scope of this PR

@matthuisman
Copy link
Contributor

my code is meant to be backwards compatible way back to Kodi 17 hehehe.
So ill just leave it - it wont hurt anything
itll just have 2x indicators for forced

@CastagnaIT CastagnaIT merged commit 938fc76 into xbmc:Piers Feb 19, 2025
8 of 10 checks passed
@CastagnaIT CastagnaIT deleted the iec_media_flags branch February 19, 2025 11:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature request] DASH support froced-subtitle role
2 participants