Skip to content

fix: converge IETF language repair and detect Direct Play failures (#747, #746)#748

Merged
ptr727 merged 8 commits into
developfrom
fix/747-746-ietf-loop-directplay
Jun 21, 2026
Merged

fix: converge IETF language repair and detect Direct Play failures (#747, #746)#748
ptr727 merged 8 commits into
developfrom
fix/747-746-ietf-loop-directplay

Conversation

@ptr727

@ptr727 ptr727 commented Jun 21, 2026

Copy link
Copy Markdown
Owner

Summary

Fixes two production issues found in monitor mode.

#747 — infinite remux loop on invalid IETF language tags

A track with an unmappable IETF/BCP-47 tag (e.g. language='und' language_ietf='enc') was flagged for remux, but mkvmerge preserves the tag, so every monitor cycle re-detected the same error and rewrote the file forever (reporter saw 5,927 rewrites of one 1.5 GB file).

  • Fix it: RepairLanguageTags repairs invalid ISO 639 / IETF tags in place via mkvpropedit — recovering the tag from the valid counterpart, else setting both to und (kept consistent so the next pass doesn't see a mismatch). Detection lives in the repair step (HasInvalidLanguageTag), inspection (TrackProps.SetLanguage) is unchanged.
  • Guarantee convergence: metadata repair is skipped once a file is VerifyFailed, and after a repair the targeted error states are re-checked — if they persist the file is marked VerifyFailed and is no longer re-processed, breaking the loop for any unfixable condition.

#746 — Verify measured tool-parseability, not player Direct-Play-ability

A file can be clean to ffprobe/ffmpeg/mkvmerge/mkvinfo yet fail Direct Play in Jellyfin/Emby/Shield because the player's EBML keyframe parse throws.

  • New MatroskaKeyframe runs the same structural EBML parse the player uses (NEbml, mirroring Jellyfin's MatroskaKeyframeExtractor) as a deterministic pass/fail oracle.
  • RepairMatroskaStructure remuxes only files that provably fail (video files only), leaving valid files untouched. Same non-convergence guard applies.
  • Validated against the real production file: it throws the exact InvalidOperationException: Operation is not valid due to the current state of the object Jellyfin reports, while good files and the monitor mode: infinite remux loop on files with unmappable IETF language tags (non-converging re-processing) #747 file pass.

Notes

  • Adds the NEbml dependency (1.1.0.5, same version Jellyfin uses).
  • Version stays in the current 3.18 cycle (NBGV auto-bumps the patch); HISTORY.md and README updated.
  • Media-behaviour testing is via the regression harness (both troublesome files added to the Troublesome set); comprehensive synthetic unit tests are a separate planned exercise.

Closes #747
Closes #746

🤖 Generated with Claude Code

ptr727 and others added 2 commits June 21, 2026 14:05
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
, #746)

#747: In monitor mode an invalid/unmappable IETF language tag caused an
infinite remux loop because remuxing preserves the tag. Repair invalid
ISO 639 / IETF tags in place via MkvPropEdit (recover from the valid
counterpart, else undefined), and add a non-convergence guard that marks
a file VerifyFailed and stops re-processing when a repair does not resolve
the targeted errors.

#746: Some Matroska files pass all tool checks yet fail Direct Play because
the player's EBML keyframe parse throws. Add a deterministic structural
parse mirroring Jellyfin's MatroskaKeyframeExtractor (NEbml) and remux only
files that provably fail, leaving valid files untouched.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings June 21, 2026 22:12

Copilot AI left a comment

Copy link
Copy Markdown

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 addresses two monitor-mode production problems by (1) making invalid/unmappable language-tag repairs converge, and (2) adding a deterministic Matroska structural parse check (player-parity) to detect Direct Play failures and remux only proven-bad files.

Changes:

  • Add in-place repair of invalid ISO/IETF language tags via mkvpropedit, plus a non-convergence guard that marks files VerifyFailed to prevent repeated rewrite loops.
  • Introduce a NEbml-based Matroska EBML/Cues structural parse (“player-like” check) and a targeted remux repair path for failures.
  • Update docs/changelog and add the NEbml dependency.

Reviewed changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
README.md Updates release notes + feature bullets to document convergence fix and new Direct Play structural check; adds NEbml to the tools list.
HISTORY.md Adds 3.18 release history entries describing both fixes and the non-convergence guard.
PlexCleaner/ProcessFile.cs Implements language-tag repair + convergence guard; adds Matroska structural verification/remux logic.
PlexCleaner/Process.cs Wires the new Matroska structure repair step into the processing pipeline.
PlexCleaner/MkvPropEditTool.cs Extends SetTrackLanguage to optionally include undefined tags so invalid tags can be overwritten deterministically.
PlexCleaner/MatroskaKeyframe.cs New NEbml-based Matroska structural parser used as a Direct Play pass/fail oracle.
PlexCleaner/PlexCleaner.csproj Adds NEbml package reference.
Directory.Packages.props Pins NEbml version centrally.
PlexCleaner.code-workspace Updates recommended VS Code TODO extension.
.gitignore Ignores .artifacts output directory.

Comment thread PlexCleaner/MatroskaKeyframe.cs Outdated
Comment thread PlexCleaner/MatroskaKeyframe.cs Outdated
Comment thread PlexCleaner/MatroskaKeyframe.cs Outdated
…it stays in sync

Address review feedback: document that the structural parse intentionally
mirrors Jellyfin's extractor (ignored mandatory-element returns and the
open Segment scope are deliberate for player parity), pin the upstream
revision it was ported from, and note the sync strategy.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated 2 comments.

Comment thread HISTORY.md Outdated
Comment thread PlexCleaner/ProcessFile.cs
…log message

Address review feedback: the HISTORY entry overstated the guard scope, it
covers language-tag repair, metadata remux, and the Matroska structure
check, not set-flags/clear-tags. Rename the helper to SetVerifyFailed and
use a neutral message since it also covers the repair-disabled case.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated 2 comments.

Comment thread PlexCleaner/MatroskaKeyframe.cs Outdated
Comment thread PlexCleaner/ProcessFile.cs
…in port) and close SetLanguage convergence gap

Licensing: the previous parser was ported from Jellyfin (GPL-2.0) into this
MIT project, which is incompatible. Reimplement independently from the
Matroska element specification using the MIT NEbml reader. Instead of
mirroring the player's parse and relying on it to crash, validate the seek
index logically: the SeekHead must reference Info, Tracks and Cues, and the
Cues must be positioned after the Tracks/Info (a forward-only reader, like
the player, cannot reach Cues placed earlier). Verified against the real
regression file: the broken file is detected, good files pass.

Convergence: the post-remux guard now also re-checks SetLanguage errors when
SetIetfLanguageTags is enabled, mirroring the remux trigger condition, so a
remux that fails to resolve them cannot loop.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated 3 comments.

Comment thread PlexCleaner/ProcessFile.cs
Comment thread PlexCleaner/ProcessFile.cs
Comment thread PlexCleaner/MatroskaStructure.cs
Address review feedback: ClearMetadataErrors now also resets per-track
HasErrors (which AnyErrors checks) so cleared files are truly clear, and the
seek-index check stops after the first cue point instead of walking the
whole index.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated 1 comment.

Comment thread PlexCleaner/ProcessFile.cs Outdated
Address review feedback: the comment now states that mkvpropedit sets the
legacy language property and the --normalize-language-ietf global option
derives the consistent IETF tag, rather than implying it sets both directly.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated 1 comment.

Comment thread PlexCleaner/MatroskaStructure.cs
Clarify that ReadAt resolves against the entered Segment container, which
matches the Segment-relative SeekHead positions, so no manual base offset
is needed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated no new comments.

@ptr727 ptr727 merged commit 1fdf938 into develop Jun 21, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants