Skip to content

Drop FFmpeg Progress Lines from Parsed Regression Errors - #1181

Merged
ptr727 merged 2 commits into
developfrom
feature/catalog-verify-errors
Nov 2, 2026
Merged

ptr727 merged 2 commits into
developfrom
feature/catalog-verify-errors

Conversation

@ptr727

@ptr727 ptr727 commented Nov 2, 2026 •

Copy link
Copy Markdown
Owner

Summary

A failing FFmpeg run at default log level ends with a progress line (frame= ... fps= ... speed=2.56x elapsed=...), and the process log carries it into that file's parsed errors. Its figures vary per run, and error_shape leaves part of a fractional speed in place (speed=N.56x), so the reduce audit counted the line as a physical error shape that no clip can match.

The change adds the progress line to BENIGN_NOISE, alongside the other host-dependent stderr that a clip is not required to reproduce.

#1174 proposed taking only VerifyMedia failures into verify_errors. That would drop the catalog buckets that come from other operations. In the 3.23.53 full run, Container-Unsupported comes from MkvMerge ReMuxToMkv failures and Encode-EmptyOutput from FFmpeg ConvertToMkv. The maintainer chose this narrower change instead. The interlace scan's own exit-69 failure, which put the idet text in the catalog, is fixed at its source in #1180.

Refs #1174

Verification

  • python3 -m unittest discover -s tests in RegressionTests: 46 tests passed, with a new test_progress_line_dropped. ruff check, ruff format --check, and mypy are clean.
  • Parsing the 3.23.53 full and reduced run logs with develop's corpus_common.py and with this branch: each log loses exactly one error line, the progress line, and gains none.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • FFmpeg progress messages containing frame counts and speed values are now excluded from extracted error reports, keeping reported errors focused on actual failures.
  • Tests
    • Added coverage to verify that progress details are filtered while genuine error messages remain visible.

A failing FFmpeg run at default log level ends with a progress line, and the
log carries it into a file's errors. Its fps, speed, and elapsed figures vary
per run, and the shape normalizer leaves part of a fractional speed in place,
so the reduce audit counted the line as a physical error shape no clip could
match. Treat it as benign noise like the other host-dependent stderr.

Refs #1174

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings November 2, 2026 22:56
@ptr727

ptr727 commented Nov 2, 2026

Copy link
Copy Markdown
Owner Author

A recorded local strict-review pass covers head ae8ed0fd4d7c9094ced1a71ef83804ba361db800, the content this pull request carries at that commit against develop, and it recorded 0 findings.

@coderabbitai

coderabbitai Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 8 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 2 included reviews currently available.

Learn how review limits work.

Review configuration:

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 45d4e60a-13d8-4731-8b1a-7f96306c473a
Commits

Reviewing files that changed from the base of the PR and between ae8ed0f and 6849690.

Files selected for processing: 2
  • RegressionTests/corpus_common.py
  • RegressionTests/tests/test_corpus_common.py

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: f6bd5e53-28e3-4bf5-bfce-0b349324c68e
Commits

Reviewing files that changed from the base of the PR and between f2d6c9a and ae8ed0f.

Files selected for processing: 2
  • RegressionTests/corpus_common.py
  • RegressionTests/tests/test_corpus_common.py

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 0 remain after this review.

Walkthrough

BENIGN_NOISE now matches FFmpeg progress text with a numeric frame= field followed by speed=. A regression test checks that extract_errors removes those fields while retaining the real error.

Changes: FFmpeg progress noise filtering

Layer / File(s) Summary
Filter progress fields and test error extraction
RegressionTests/corpus_common.py, RegressionTests/tests/test_corpus_common.py
BENIGN_NOISE matches progress text with a numeric frame= field followed by speed=. The test verifies that extract_errors discards the progress fields and retains "real".

Estimated code review effort: 2 (Simple) | ~7 minutes

Change: Bug fix

Possibly related issues

  • ptr727/PlexCleaner#1174 — The issue describes an FFmpeg output side effect in regression tooling; this change filters matching progress text.
  • ptr727/PlexCleaner#1177 — The issue requests shape normalization for FFmpeg progress lines; this change adds the pattern and a parser test.
  • ptr727/PlexCleaner#1171 — The issue includes the corpus-tooling progress-line fix; this change implements that parser update.
Merge Risk: Minimal · previous review ae8ed

The change removes matching FFmpeg progress entries while retaining adjacent real errors. No actionable merge-blocking risk was identified.

Pre-merge checks: Passed 5
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: filtering FFmpeg progress lines from parsed regression errors.
Docstring Coverage Passed No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 2 files.

Finishing Touches
Generate docstrings
  • Commit to this branch
  • Create a new PR
Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

CodeRabbit Verify

  • Run CodeRabbit Verify to test the changes in this pull request.

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Exclude FFmpeg progress lines from regression errors

🐞 Bug fix 🕐 Less than 10 minutes

Grey Divider

AI Description

• Exclude run-dependent FFmpeg progress lines from parsed errors so reduction audits compare
 reproducible failures.
• Preserve errors from all operations rather than limiting the catalog to verify failures.
• Add a regression test confirming real errors remain when a progress line is dropped.
Diagram

graph TD
  A["Process log"] --> B["Parse log"] --> C["Extract errors"] --> D{"Benign noise?"} -->|No| E["Retained errors"] --> F["Physical audit"]
  D -->|Yes| G["Discard line"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Keep only verify-operation failures
  • ➕ Would exclude progress output associated with other operations.
  • ➖ Would also lose legitimate catalog errors from remux and conversion failures.

Recommendation: Keep the targeted benign-noise filter. It removes a non-reproducible progress entry without narrowing which operations can contribute legitimate errors to the catalog.

Files changed (2) +6 / -1

Bug fix (1) +2 / -1
corpus_common.pyFilter FFmpeg progress entries from extracted errors +2/-1

Filter FFmpeg progress entries from extracted errors

• Extends the shared benign-noise pattern to recognize progress entries containing a numeric frame count and speed field. Matching entries are omitted before parsed errors are used by corpus tooling.

RegressionTests/corpus_common.py

Tests (1) +4 / -0
test_corpus_common.pyCover progress filtering without losing real errors +4/-0

Cover progress filtering without losing real errors

• Adds an extraction test with a real error alongside an FFmpeg progress entry. It asserts that only the real error remains.

RegressionTests/tests/test_corpus_common.py

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.

🟢 Approval recommended

The review evidence lists no active issues, and the change is narrowly scoped: it adds a BENIGN_NOISE pattern for FFmpeg progress segments along with a unit test for that case.

No open issues 🎉

What changed in this PR

This PR stops FFmpeg progress lines from being counted as parsed regression errors. At default log level, a failing FFmpeg run ends with a line such as frame= ... fps= ... speed=2.56x elapsed=.... Its values change every run, and the existing normalization leaves a partial speed value in place. The reduce audit then counted it as a physical error shape that no clip could match. The fix adds a progress-line pattern to BENIGN_NOISE, the list of host-dependent stderr that is ignored. It is part of the regression tooling work tracked in #1174.

Changes:

  • Adds a frame=… speed= progress-line pattern to BENIGN_NOISE in corpus_common.py.
  • Adds test_progress_line_dropped to confirm the progress segment is removed and the real error is kept.
File Description
RegressionTests/​tests/​test_corpus_common.py Adds a unit test showing a progress segment is dropped while a real error in the same string is kept.
RegressionTests/​corpus_common.py Adds the progress-line pattern to BENIGN_NOISE, which extract_errors applies to each `"

Review effort: Lite

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Real decoder errors can disappear ✓ Resolved
Description
BENIGN_NOISE uses an unanchored frame=...speed= search, so extract_errors discards any
complete error fragment containing that sequence rather than only an actual progress line. A decoder
message with those fields is retained by the verifier and logged as an error, but the catalog then
loses its signature and reduction checks can accept a clip that did not reproduce the source
failure.
Code

RegressionTests/corpus_common.py[60]

+    r"\bframe=\s*\d+\b.*\bspeed=",
Evidence
The verifier treats every non-timestamp stderr line as a decode error and preserves printable text
in the logged summary. The new search is applied to each complete extracted error fragment; if it
matches, that fragment is removed, and an empty result becomes a silent nonzero exit whose decode
signatures cannot be classified.

RegressionTests/corpus_common.py[56-60]
RegressionTests/corpus_common.py[265-274]
RegressionTests/corpus_common.py[315-316]
PlexCleaner/VerifyClassifier.cs[70-87]
PlexCleaner/MediaTool.cs[285-296]
RegressionTests/reduce_corpus.py[433-439]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new benign-noise regex matches any extracted error containing `frame=<number>` followed later by `speed=`, so a genuine decoder diagnostic with those fields can be removed from the catalog.
## Fix Focus Areas
- RegressionTests/corpus_common.py[56-60]
- RegressionTests/tests/test_corpus_common.py[46-52]
## Recommended Fix
Restrict the progress matcher to the beginning of the normalized error fragment and require the expected FFmpeg progress-line structure, such as `frame=...` followed by the progress fields and `speed=`. Add a regression test showing that a diagnostic prefix containing `frame=<number> ... speed=` is retained while a standalone progress line is dropped.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Tip of the day
💡 Did you know, you can copy the agent prompt from any finding and feed it to your IDE agent

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@ptr727

ptr727 commented Nov 2, 2026

Copy link
Copy Markdown
Owner Author

A recorded local strict-review pass covers head 684969041549180d957aaad58fcd246ac2b72c5e, the content this pull request carries at that commit against develop, and it recorded 0 findings.

@ptr727

ptr727 commented Nov 2, 2026

Copy link
Copy Markdown
Owner Author

Qodo finding "Real decoder errors can disappear": fixed in 6849690. The progress-line alternative in BENIGN_NOISE is now anchored, ^frame=\s*\d+\s.*\bspeed=, so it matches only a segment that opens as an FFmpeg stats line. extract_errors strips each segment before the search. The new test_error_naming_progress_fields_kept keeps a decoder message that names frame= and speed= mid-line. Both real 3.23.53 logs still lose only their one progress line.

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

Qodo finding "Real decoder errors can disappear": fixed in 6849690. The progress-line alternative in BENIGN_NOISE is now anchored, ^frame=\s*\d+\s.*\bspeed=, so it matches only a segment that opens as an FFmpeg stats line. extract_errors strips each segment before the search. The new test_error_naming_progress_fields_kept keeps a decoder message that names frame= and speed= mid-line. Both real 3.23.53 logs still lose only their one progress line.

Thanks for the update. Finding 1 — “Real decoder errors can disappear” is resolved in commit 68496904. The anchored matcher and regression coverage address the reported false-positive removal while preserving the intended progress-line filtering.

@ptr727
ptr727 merged commit a219600 into develop Nov 2, 2026
15 checks passed
@ptr727
ptr727 deleted the feature/catalog-verify-errors branch November 2, 2026 23:54
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