Skip to content

Conversation

CastagnaIT
Copy link
Collaborator

@CastagnaIT CastagnaIT commented May 4, 2023

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 to true 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 to original to prefer set default streams with original language, or impaired 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

  • 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

@CastagnaIT CastagnaIT added Type: Feature non-breaking change which adds functionality v21 Omega labels May 4, 2023
@CastagnaIT CastagnaIT force-pushed the stream_lang_flags branch from b47bc99 to aa6d500 Compare June 7, 2023 09:07
@CastagnaIT CastagnaIT changed the title [PropertiesUtils] Add stream_audio_cfg, stream_subtitles_cfg properties [PropertiesUtils] Add streams_config property Jun 7, 2023
@kino0924
Copy link

kino0924 commented Feb 9, 2025

@CastagnaIT

I have been looking for a way for this and looks great feature.
However, its not merged for long time.
Is there reason that its not merged or not closed?

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

@CastagnaIT
Copy link
Collaborator Author

this is marked Draft so WIP,
before this PR there was a need to make other code changes
this code also needs to be revisited because there were points that were not really clear

now i am busy finishing a more important change that has priority over this one,
but i have little free time in these months

anyway meantime for original flag you can try use https://github.com/xbmc/inputstream.adaptive/wiki/Integration#inputstreamadaptiveoriginal_audio_language

@kino0924
Copy link

kino0924 commented Feb 9, 2025

this is marked Draft so WIP, before this PR there was a need to make other code changes this code also needs to be revisited because there were points that were not really clear

now i am busy finishing a more important change that has priority over this one, but i have little free time in these months

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.
And yes, I already tried original_audio_language and sadly it didnt work.
I really hope this feature gets implemented someday!

Thanks again.

@garbear garbear requested a review from Copilot May 25, 2025 06:51
Copy link

@Copilot Copilot AI left a 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) &&
Copy link
Preview

Copilot AI May 25, 2025

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.

Suggested change
STRING::CompareNoCase(adpSet->GetLanguage(), langCode) &&
STRING::CompareNoCase(adpSet->GetLanguage(), langCode) &&
!adpSet->GetRepresentations().empty() &&

Copilot uses AI. Check for mistakes.

Comment on lines +445 to +472
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);
}
Copy link
Preview

Copilot AI May 25, 2025

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.

Suggested change
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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Feature non-breaking change which adds functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants