[inputstream.adaptive] Fix audio deadlock on quality STREAMCHANGE

[linzack]

Description

This PR resolves two intertwined bugs that result in severe playback stalling and complete audio pipeline failure during DASH/adaptive playback, specifically surfacing when users configure maximum video resolution limits (e.g., 1080p) on higher resolution physical displays (e.g., 4K TVs).

Each fix is in its own commit:

1. [chooser] Screen Resolution False Positives (src/common/ChooserDefault.cpp, ChooserDefault.h):
   CRepresentationChooserDefault::CheckResolution() compared the user-configured resolution limit (m_screenWidth) against the actual physical hardware resolution (m_screenCurrentWidth), resulting in perpetual false positives every 10 seconds. Dedicated tracking variables (m_screenLastWidth, m_screenLastHeight) now correctly isolate physical window resize detection from the addon's maximum configured quality limits.

2. [demux] Audio Deadlock during STREAMCHANGE (src/main.cpp):
   When a bandwidth fluctuation triggered a representation change, Kodi Core reopened all active streams sequentially. If the first stream (Video) evaluated to no quality change, the m_checkCoreReopen optimization flag short-circuited all further OpenStream callbacks. If a subsequent stream (Audio) did change quality, its Reset() and reconstruction was skipped entirely, leaving it stuck at EVENT_TYPE::REP_CHANGE and permanently blocking segment downloads. This commit hoists a !isStreamChanged conditional to guard the entire m_checkCoreReopen block, guaranteeing that a formally changed stream bypasses the fast-path optimization and executes its native Reset() cycle.

Motivation and context

When playing adaptive streams on displays with a higher physical resolution than the user's Kodi resolution cap (e.g., a 1080p cap on a 4K display), the demuxer would continuously spin false STREAMCHANGE events every 10 seconds due to a flawed m_screenWidth != m_screenCurrentWidth comparison, severely stuttering playback.

Even without these false positives, a genuine adaptive bandwidth fluctuation would trigger a legitimate DEMUX_SPECIALID_STREAMCHANGE. During the subsequent sequential OpenStream workload triggered by Kodi Core, the audio stream's Reset() operation was swallowed due to a shared m_checkCoreReopen fast-path bypass. Because the audio reader's state was abandoned mid-update (stuck at EVENT_TYPE::REP_CHANGE), it permanently blocked segment queuing.

These changes are necessary to allow inputstream.adaptive to survive legitimate adaptive bandwidth changes without permanently killing the audio output pipeline.

How has this been tested?

• Built and tested on LibreELEC on Raspberry Pi 4 with the plugin.video.youtube add-on.
• Tested on standard Kodi playback of YouTube DASH streams (webm video / mp4a audio).
• Verified via trace logs that DEMUX_SPECIALID_STREAMCHANGE gracefully reconstructs the audio FFmpeg decoders.

Log Comparisons

Bug 1: Screen Resolution False Positives (ChooserDefault.cpp)
Before Fix: Kodi routinely and spuriously detects screen resolution changes because m_screenWidth (config limit) naturally mismatches m_screenCurrentWidth (hardware rendering).

2026-02-24 02:57:35.243 T:72583   debug <inputstream.adaptive>: [Repr. chooser] Screen resolution has changed: 3840x2160
... (10 seconds later)
2026-02-24 02:57:46.157 T:72583   debug <inputstream.adaptive>: [Repr. chooser] Screen resolution has changed: 3840x2160
... (10 seconds later)
2026-02-24 02:57:57.402 T:72648   debug <inputstream.adaptive>: [Repr. chooser] Screen resolution has changed: 3840x2160

After Fix: The STREAMCHANGE storm vanishes. No Screen resolution has changed log entries are observed during prolonged, stable 4K playback sessions.

Bug 2: Audio Deadlock (main.cpp)
Before Fix: Following a STREAMCHANGE, OpenStream(1014) (Audio) hits the early-return optimization set by the preceding video stream, skipping stream construction.

2026-02-24 12:10:19.325 T:9695     info <general>: Opening stream: 1014 source: 256
2026-02-24 12:10:19.325 T:9695    debug <inputstream.adaptive>: OpenStream(1014)
2026-02-24 12:10:19.326 T:9695    debug <inputstream.adaptive>: OpenStream(1014): The stream has already been opened
2026-02-24 12:10:22.509 T:9695    debug <general>: CVideoPlayer::HandlePlaySpeed - audio stream stalled, triggering re-sync

After Fix: The hoisted !isStreamChanged conditional allows genuinely changed streams to fall through, successfully rebuilding their FFmpeg instance.

2026-02-25 11:24:56.931 T:15307   debug <inputstream.adaptive>: DEMUX_SPECIALID_STREAMCHANGE (stream quality changed)
2026-02-25 11:24:56.933 T:15307    info <general>: Opening stream: 1014 source: 256
2026-02-25 11:24:56.933 T:15307   debug <inputstream.adaptive>: OpenStream(1014)
2026-02-25 11:24:56.933 T:15307   debug <inputstream.adaptive>: UpdateSampleDescription: Codec fourcc: mp4a (1836069985)
2026-02-25 11:24:56.934 T:15307    info <general>: CDVDAudioCodecFFmpeg::Open() Successful opened audio decoder aac
2026-02-25 11:24:56.983 T:14828   debug <general>: [plugin.video.youtube] http_server:587(do_GET) ('audio', 'mp4') Range:  'bytes=8728872-8890772'

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

[kodiai]

Kodiai Review Summary

What Changed

Fixes audio deadlock during adaptive STREAMCHANGE and eliminates false positive screen resolution change detections that caused spurious quality switches.

Reviewed: core logic

Strengths

• ✅ The PR correctly identifies and fixes the root cause where unchanged streams were being short-circuited from initialization when m_checkCoreReopen was set by a different stream
• ✅ The screen resolution tracking logic properly separates physical window resizes from user-configured quality limits by introducing dedicated tracking variables

Observations

Impact

[MAJOR] src/main.cpp (230-231): Null pointer dereference when reader is null
The code checks reader && reader->EOS() for the isEos variable but then unconditionally calls reader->Reset(false) without verifying that reader is not null. When stream->GetReader() returns nullptr, this causes a crash.

    auto reader = stream->GetReader();
    if (!reader)
    {
      LOG::Log(LOGDEBUG, "OpenStream(%d): The stream has already been opened. Reader is null.", streamid);
      return true;
    }
    
    bool isEos{reader->EOS()};
    
    LOG::Log(LOGDEBUG, "OpenStream(%d): The stream has already been opened. Checking recovery. isEos=%d streamChanged=%d", 
             streamid, isEos, streamChanged);

    if (isEos)
    {
      LOG::Log(LOGINFO, "OpenStream(%d): Recovering stream from transient EOS", streamid);
      reader->Reset(false); // Clear EOS
    }
    return true; // Tell Kodi Core the stream is still valid

[MAJOR] src/main.cpp (224, 226): Redundant StreamChanged() call creates race condition window
StreamChanged() is called twice: once at line 224 to store in streamChanged, then again implicitly through the condition check at line 226. Between these calls, the adaptive stream state could change during concurrent playback, leading to inconsistent behavior where a stream that genuinely changed could incorrectly enter the optimization block.

  bool streamChanged{stream->m_adStream.StreamChanged()};
  
  if (m_checkCoreReopen && !streamChanged)

[MEDIUM] src/main.cpp (241): Behavior change from false to true may affect callers
The return value changed from false to true when m_checkCoreReopen is set. While the PR description explains this signals to Kodi Core that the stream is alive, this is a significant behavioral change. Verify that all calling code correctly handles this transition, especially for streams that are genuinely in a failure state (not just transient EOS). The comment "Tell Kodi Core the stream is still valid" suggests this is intentional, but returning true for a stream that was previously returning false could mask real initialization failures.

Preference

[MINOR] src/main.cpp (233): Debug log includes redundant streamChanged value
Optional: The log line includes streamChanged=%d, but this block only executes when !streamChanged is true (i.e., when streamChanged is always false). This parameter adds no diagnostic value. Consider removing it or logging it outside the conditional block if it's useful for understanding the control flow.

Suggestions

• Optional: Consider adding unit tests to verify the EOS recovery path doesn't interfere with genuine stream failures
• Future consideration: The m_checkCoreReopen pattern (workaround for Kodi core behavior) might benefit from more explicit state machine tracking to prevent future race conditions

Verdict

🔴 Address before merging -- 3 blocking issue(s) found

Review Details

• Files reviewed: 3
• Lines changed: +25 -4
• Profile: strict (auto, lines changed: 29)
• Author: first-time (adapted tone)
• Findings: 0 critical, 2 major, 1 medium, 1 minor (includes 4 from summary observations)
• Review completed: 2026-02-25T06:49:54.956Z
• Keyword parsing:
  • focus hints: [INPUTSTREAM.ADAPTIVE], [CHOOSER], [DEMUX]
  • breaking change in body

[github-actions]

Review

Must Fix — Unconditional return true in non-EOS path (src/main.cpp:241)

The guard block now returns true unconditionally regardless of whether EOS recovery actually occurred. The original return false was intentional: per the code comment at line 173, true signals Kodi to re-fetch and update stream properties. Returning true for every normal m_checkCoreReopen cycle (where isEos is false and nothing changed) causes Kodi to perform unnecessary stream info refresh/codec re-init on every demux parse cycle. The return true should be conditional on isEos, with the non-EOS path retaining the original return false.

Minor — Trailing whitespace on blank lines 225 and 232 in src/main.cpp

Minor — streamChanged=%d in the LOGDEBUG message at line 233 is always false inside the !streamChanged branch — redundant and potentially misleading.

[linzack]

@claude can you review again?

[kodiai]

kodiai response

Decision: APPROVE

Issues: none

[CastagnaIT]

thank you for the detailed explanations about the problems
seems to make sense i will test tonight

i ask you to split the single commit and create two commits, so one for each fix

[CastagnaIT]

LGTM