Skip to content

Fix regressions and improvements to stream selection#1824

Merged
CastagnaIT merged 5 commits intoxbmc:Piersfrom
CastagnaIT:stream_selection_fixes
Apr 13, 2025
Merged

Fix regressions and improvements to stream selection#1824
CastagnaIT merged 5 commits intoxbmc:Piersfrom
CastagnaIT:stream_selection_fixes

Conversation

@CastagnaIT
Copy link
Copy Markdown
Collaborator

Description

  • Fix regression double video quality change with «Ask quality», an attempt to fix has been done on PR [Chooser][ask quality] Fix bad preselection and fixed quality #1810 but was not working good, that in some cases cause crashes
  • Fix regression on Adaptive quality that dont change while in playback, due to wrong condition on Higher method
  • Fix regression on DASH AdaptationSet «default» override parameter for video stream, that was not taken in account
  • Add new setting «Limit resolution» to Stream selection «Ask Quality». I thought it might be useful for everyone, when you play multi-codec video a very long list of resolutions can be listed, which you struggle to scroll through, this option limits the list to only the set resolution, at least it tries to limit them based also on the capabilities (streams provided) of the manifest

Motivation and context

new drm rework has changed some code introducing a codec selection with priority order that was incomplete
and other things was need to be fixed

fix #1821

How has this been tested?

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: Fix non-breaking change which fixes an issue Type: Feature non-breaking change which adds functionality v22 Piers labels Apr 12, 2025
@Ronny-nerd
Copy link
Copy Markdown

Ronny-nerd commented Apr 12, 2025

Thanks CastagnaIT...it works fine...I noticed on Windows 10 Pro ( 64 BIT ) Kodi 22 Piers.

Android 11...Nvidia Shield TV Pro 2019 = OK

@CastagnaIT
Copy link
Copy Markdown
Collaborator Author

@Ronny-nerd have you tested also the use case of youtube issue #1821?
because im not able to test youtube addon, every time ask me to login (using Kodi Cast extension on browser) but signin menu on addon dont works

@CastagnaIT CastagnaIT force-pushed the stream_selection_fixes branch from be12e16 to 3ed74bd Compare April 12, 2025 14:29
Comment thread src/Session.cpp Outdated
CODEC::FOURCC_HEVC, CODEC::FOURCC_AV01, CODEC::NAME_AV1, CODEC::FOURCC_VP09,
CODEC::NAME_VP9, CODEC::FOURCC_AVC_, CODEC::FOURCC_H264};

CAdaptationSet* overrideAdp{nullptr}; // Override done by manifest custom parameter
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This can be simplified to return currAdp immediately if it is default, without needing to continue the loops:

  CAdaptationSet* defaultAdp{nullptr}; // Default determined by codec order

  for (auto& codecCC : videoCodecOrder)
  {
    CAdaptationSet* currAdp{nullptr};
    uint32_t adpIndex{0};
    while ((currAdp = m_adaptiveTree->GetAdaptationSet(adpIndex++)))
    {
      if (currAdp->GetRepresentations().empty() || currAdp->GetStreamType() != StreamType::VIDEO)
        continue;

      if (currAdp->IsDefault())
      {
        return currAdp;
      }
      else if (CODEC::Contains(currAdp->GetCodecs(), codecCC) && !defaultAdp)
      {
        defaultAdp = currAdp;
      }
    }
  }

  return defaultAdp;

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

yeah also one less variable, change done
good to go?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sorry, while the other changes seem to make sense don't have a machine/connectivity to properly runtime test. Only really checked the adaptation set default selection on initial playback start.

There have been some reports of both DRM and non-DRM streams failing to play with v22.2.0 but no logs so I don't know what may be causing it.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

you don't need to test each change, just just to know for your youtube case was ok

@CastagnaIT CastagnaIT force-pushed the stream_selection_fixes branch from 3ed74bd to 704bd42 Compare April 12, 2025 15:26
@mirh mirh mentioned this pull request Apr 12, 2025
11 tasks
@CastagnaIT CastagnaIT merged commit 7f3abfd into xbmc:Piers Apr 13, 2025
10 checks passed
@CastagnaIT CastagnaIT deleted the stream_selection_fixes branch April 13, 2025 07:49
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 Type: Fix non-breaking change which fixes an issue v22 Piers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] PR#1724 introduces regression with default AdaptationSet selection

3 participants