Skip to content

fix(log): redirect mpv stderr to log file instead of --log-file#6

Merged
flamerged merged 1 commit into
mainfrom
fix/log-file-level-suppression
May 18, 2026
Merged

fix(log): redirect mpv stderr to log file instead of --log-file#6
flamerged merged 1 commit into
mainfrom
fix/log-file-level-suppression

Conversation

@flamerged
Copy link
Copy Markdown
Owner

@flamerged flamerged commented May 18, 2026

Summary

The default-mode log was still filling with per-poll IPC chatter despite my v0.5 PR setting --msg-level=…,ipc=warn,…. Two reasons:

  1. mpv's --log-file writes at internal verbose level regardless of --msg-level. Any per-module override there has zero effect on file output.
  2. mpv 0.41 has no --log-file-level (option not found). The flag I'd have used doesn't exist on this version.

The only knob that actually respects --msg-level is mpv's stderr stream. So this PR drops --log-file=$LOG and redirects mpv's stderr into the log instead:

-    --log-file="$LOG" \
     --msg-level="$msg_level" \
     "$url" \
-    >/dev/null 2>&1 &
+    2>"$LOG" >/dev/null &

Also adds ao=v to the default msg-level so the genuinely-useful audio-device-selection lines (e.g. selected audio output device: UGREEN-BT505) still surface in the default-mode log.

Validation

Test Pre-fix Post-fix
Isolated mpv idle + 10 IPC polls (5s menu refresh sim) ~600 lines 0 bytes
Live restart via plugin (mpv crashes on yt-dlp 403 — error-path noise) 327 lines / 31 KB 0 bytes
User's prior 71h playback log 614k lines / 29 MB n/a (will measure next time)

./scripts/check.sh green; coderabbit --plain clean after removing a dead local log_level declaration that an earlier draft of this fix left behind.

Bump

feat: would be feat: but this is fix: → patch bump (v0.5.0 → v0.5.1) once squash-merged.

Summary by CodeRabbit

  • Chores
    • Improved logging behavior: mpv output is now captured more reliably to the log file and log verbosity better reflects what gets written.
  • Bug Fixes
    • Reduced extraneous startup noise in logs while ensuring non-debug runs include relevant audio/output details for troubleshooting.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 18, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: c87a1462-cf99-4873-82ad-93ffc76462cf

📥 Commits

Reviewing files that changed from the base of the PR and between a91497c and c61af95.

📒 Files selected for processing (1)
  • bin/menutube.5s.sh

📝 Walkthrough

Walkthrough

action_play now adds ao=v to the non-debug --msg-level and writes mpv's stderr into "$LOG" via redirection instead of using --log-file, so --msg-level controls what appears in the saved log.

Changes

mpv logging configuration

Layer / File(s) Summary
mpv msg_level computation and stderr redirection
bin/menutube.5s.sh
msg_level now includes ao=v for audio output verbosity in non-debug mode, and mpv invocation switches from --log-file="$LOG" to stderr redirection 2>>"$LOG" while discarding stdout, so --msg-level affects the written log.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • flamerged/menutube#5: Both PRs modify bin/menutube.5s.sh's action_play mpv startup arguments to change how --msg-level controls the resulting mpv logging behavior and log handling.

Poem

🐰
I hop and tweak the launch command line,
stderr now finds its logged design.
Audio chatter slightly more loud,
Quieting IPC, pleasing the crowd.
Menutube sings — the rabbit's proud.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: replacing mpv's --log-file flag with stderr redirection to control logging output.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/log-file-level-suppression

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@bin/menutube.5s.sh`:
- Line 436: The current stderr redirection `2>"$LOG"` in the menutube startup
line truncates the log file on each launch; change it to append instead by
replacing `2>"$LOG"` with `2>>"$LOG"` (or use a shell-appropriate append form
like `&>>"$LOG"` if you want both stdout/stderr appended) so debug logs are
preserved across stop/play cycles and diagnostic context is retained.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 296bcfe5-5048-4b7a-a33e-0303eb8e363f

📥 Commits

Reviewing files that changed from the base of the PR and between 3714eae and a91497c.

📒 Files selected for processing (1)
  • bin/menutube.5s.sh

Comment thread bin/menutube.5s.sh Outdated
The previous default-mode `--msg-level=all=info,ipc=warn,ffmpeg=warn` did
not stop mpv from filling the log with per-poll IPC chatter at level v.
Two reasons:

1. mpv's `--log-file` ignores `--msg-level` entirely and always writes at
   internal verbose level — so any per-module override on msg-level has
   no effect on file output.
2. mpv 0.41 does not have `--log-file-level` either (option not found).

The only working knob is the stderr stream, which DOES respect msg-level.
Drop `--log-file=$LOG` and redirect mpv's stderr into the log instead
(`2>"$LOG"` in the launcher). Default mode log now starts at 0 bytes and
grows only with info+ entries; debug mode (msg-level=all=v) gets full
verbosity as before.

Isolated test: mpv idle + 10 IPC polls produced 0 bytes after the fix
vs ~600 lines pre-fix. Live re-test on the menutube install confirmed
0 bytes / 0 lines after restart.

Adds `ao=v` to default msg-level so the useful audio-device-selection
diagnostic line still shows up in the default log.
@flamerged flamerged force-pushed the fix/log-file-level-suppression branch from a91497c to c61af95 Compare May 18, 2026 13:56
@flamerged flamerged merged commit bda215d into main May 18, 2026
4 checks passed
@flamerged flamerged deleted the fix/log-file-level-suppression branch May 18, 2026 14:00
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.

1 participant