-
Notifications
You must be signed in to change notification settings - Fork 254
[PropertiesUtils] Add streams_config property #1250
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
base: Omega
Are you sure you want to change the base?
Conversation
b47bc99
to
aa6d500
Compare
I have been looking for a way for this and looks great feature. I really need this feature since I have to set every dash stream that I have to different audio track since the track is not default/original |
this is marked Draft so WIP, now i am busy finishing a more important change that has priority over this one, anyway meantime for original flag you can try use https://github.com/xbmc/inputstream.adaptive/wiki/Integration#inputstreamadaptiveoriginal_audio_language |
Thank you for the reply. Thanks again. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds a new property (streams_config) to allow flexible configuration of audio and subtitle stream selection based on language and channel preferences. Key changes include:
- Adding string utility functions (ToMap and Trim) for parsing property values.
- Updating PropertiesUtils to handle the new streams_config property and deprecate the old original_audio_language property.
- Modifying DASH parsing, AdaptiveTree stream flag adjustments, and session flag handling accordingly.
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
src/utils/StringUtils.h/.cpp | Added utility functions for parsing property strings. |
src/utils/PropertiesUtils.h/.cpp | Updated property definitions and parsing logic for streams_config. |
src/parser/DASHTree.cpp | Extended adaptation set parsing for additional roles. |
src/common/AdaptiveTree.h/.cpp | Introduced FixStreamsFlags to override stream flags as per new config. |
src/common/AdaptationSet.h/.cpp | Added helper FindAudioAdpSet to select audio streams by channel count. |
src/Session.cpp | Adjusted stream flag assignment based only on the new properties. |
inputstream.adaptive/addon.xml.in | Updated listitemprops to include the new streams_config property. |
{ | ||
auto adpSet = itAdpSet->get(); | ||
if (adpSet->GetStreamType() == StreamType::AUDIO && | ||
STRING::CompareNoCase(adpSet->GetLanguage(), langCode) && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider checking that the vector returned by GetRepresentations() is not empty before accessing its first element to avoid potential out-of-range errors.
STRING::CompareNoCase(adpSet->GetLanguage(), langCode) && | |
STRING::CompareNoCase(adpSet->GetLanguage(), langCode) && | |
!adpSet->GetRepresentations().empty() && |
Copilot uses AI. Check for mistakes.
if (isDefaultStereo) | ||
{ | ||
itAudioStream = CAdaptationSet::FindAudioAdpSet(adpSets, langCodeDef, true, true); | ||
if (itAudioStream == adpSets.cend()) // No stereo stream, find multichannels | ||
itAudioStream = CAdaptationSet::FindAudioAdpSet(adpSets, langCodeDef, false, true); | ||
} | ||
else | ||
{ | ||
itAudioStream = CAdaptationSet::FindAudioAdpSet(adpSets, langCodeDef, false, true); | ||
if (itAudioStream == adpSets.cend()) // No multichannels stream, find stereo | ||
itAudioStream = CAdaptationSet::FindAudioAdpSet(adpSets, langCodeDef, true, true); | ||
} | ||
|
||
// No stream found, try find a "impaired" stream with the "original" language code | ||
if (itAudioStream == adpSets.cend() && !langCodeOrig.empty()) | ||
{ | ||
if (isDefaultStereo) | ||
{ | ||
itAudioStream = CAdaptationSet::FindAudioAdpSet(adpSets, langCodeOrig, true, true); | ||
if (itAudioStream == adpSets.cend()) // No stereo stream, find multichannels | ||
itAudioStream = CAdaptationSet::FindAudioAdpSet(adpSets, langCodeOrig, false, true); | ||
} | ||
else | ||
{ | ||
itAudioStream = CAdaptationSet::FindAudioAdpSet(adpSets, langCodeOrig, false, true); | ||
if (itAudioStream == adpSets.cend()) // No multichannels stream, find stereo | ||
itAudioStream = CAdaptationSet::FindAudioAdpSet(adpSets, langCodeOrig, true, true); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] There is a noticeable duplication between the stereo and multichannel selection logic; refactoring this section could improve clarity and maintainability.
if (isDefaultStereo) | |
{ | |
itAudioStream = CAdaptationSet::FindAudioAdpSet(adpSets, langCodeDef, true, true); | |
if (itAudioStream == adpSets.cend()) // No stereo stream, find multichannels | |
itAudioStream = CAdaptationSet::FindAudioAdpSet(adpSets, langCodeDef, false, true); | |
} | |
else | |
{ | |
itAudioStream = CAdaptationSet::FindAudioAdpSet(adpSets, langCodeDef, false, true); | |
if (itAudioStream == adpSets.cend()) // No multichannels stream, find stereo | |
itAudioStream = CAdaptationSet::FindAudioAdpSet(adpSets, langCodeDef, true, true); | |
} | |
// No stream found, try find a "impaired" stream with the "original" language code | |
if (itAudioStream == adpSets.cend() && !langCodeOrig.empty()) | |
{ | |
if (isDefaultStereo) | |
{ | |
itAudioStream = CAdaptationSet::FindAudioAdpSet(adpSets, langCodeOrig, true, true); | |
if (itAudioStream == adpSets.cend()) // No stereo stream, find multichannels | |
itAudioStream = CAdaptationSet::FindAudioAdpSet(adpSets, langCodeOrig, false, true); | |
} | |
else | |
{ | |
itAudioStream = CAdaptationSet::FindAudioAdpSet(adpSets, langCodeOrig, false, true); | |
if (itAudioStream == adpSets.cend()) // No multichannels stream, find stereo | |
itAudioStream = CAdaptationSet::FindAudioAdpSet(adpSets, langCodeOrig, true, true); | |
} | |
itAudioStream = FindPreferredAudioStream(adpSets, langCodeDef, isDefaultStereo); | |
// No stream found, try find a "impaired" stream with the "original" language code | |
if (itAudioStream == adpSets.cend() && !langCodeOrig.empty()) | |
{ | |
itAudioStream = FindPreferredAudioStream(adpSets, langCodeOrig, isDefaultStereo); |
Copilot uses AI. Check for mistakes.
Description
This PR add a new property:
inputstream.adaptive.streams_config
the name a bit generic to allow in future possibile new extensions of these configurations relative to streams without add new properties (if you want suggest a better naming say so)
the values are handled in similar way of a pair values (or something similar of python dict) as follow:
key=value;
more values are so concatenated:
key=value;key2=value2;key3=value3;
inputstream.adaptive.streams_config
Supported configuration keys:
audio_langcode_default
: Set the "default" flag to the audio streams with specified language code (need to match the manifest)audio_langcode_original
: Set the "original" flag to the audio streams with specified language code (need to match the manifest)audio_prefer_stereo
: Can be set totrue
when you want prefeer stereo track as default instead of multichannels (it depends from audio_langcode_default and/or audio_langcode_original)audio_prefer_type
: Can be set tooriginal
to prefer set default streams with original language, orimpaired
to prefer set default streams for impaired, otherwise left empty value for default behaviour.subtitles_langcode_default
: Set the "default" flag to the subtitle streams with specified language code (need to match the manifest)Example:
audio_langcode_default=pt_br;audio_prefer_stereo=true
Motivation and context
Completed fix for #422 for DASH and in more general way,
after the recent parser rework i have improved the behaviour of
AdaptationSet
tag ISA custom attributes:default
,impaired
,forced
,original
where now can be add and used to full override default manifest stream attributes,
the reason is that Kodi stream flags have not always have the same meaning of manifests attributes
and/or there are video services that dont follow exactly the specs and lead to wrong flags set to the audio/subtitle tracks
ofc add these custom ISA attributes needs to implement a proxy on a video add-on not so easy to do for novice devs but also expensive for low end devices, therefore this PR add two new properties that allow to set (and so override) the default stream language and original stream language in easy way
HLS manifest note
This manifest type not always have appropriate language codes and also not always provide right number of audio channels in th metadata, therefore above configurations may fails, strictly depends on the service provider
WIP - i will evaluate this part in the coming days:
i will need to make some more tests
use json for property values
How has this been tested?
Tested with a sample dash
Screenshots (if appropriate):
Types of change
Checklist: