Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mpv_identify script broken #15782

Closed
6 tasks done
tobiasjakobi opened this issue Jan 31, 2025 · 1 comment · Fixed by #15784
Closed
6 tasks done

mpv_identify script broken #15782

tobiasjakobi opened this issue Jan 31, 2025 · 1 comment · Fixed by #15784
Labels

Comments

@tobiasjakobi
Copy link
Contributor

mpv Information

mpv 0.39.0 Copyright © 2000-2024 mpv/MPlayer/mplayer2 projects
libplacebo version: v7.349.0
FFmpeg version: 6.1.2
FFmpeg library versions:
   libavcodec      60.31.102
   libavdevice     60.3.100
   libavfilter     9.12.100
   libavformat     60.16.100
   libavutil       58.29.100
   libswresample   4.12.100
   libswscale      7.5.100

Other Information

- Linux version: Gentoo x86-64
- Kernel Version: 6.12.11
- GPU Model: AMD Renoir
- Mesa/GPU Driver Version: 24.3.3
- Window Manager and Version: Sway 1.9
- Source of mpv: Distro
- Latest known working version: Unkown
- Issue started after the following happened: Unkown

Reproduction Steps

This isn't really a problem with mpv per se, but with the mpv_identify.sh helper script from TOOLS.

Currently the following happens:

$ . ~/local/bin/mpv_identify.sh id_ ~/test1.wav
bash: id_video_params/aspect=1.000000: No such file or directory

My guess is that due to how variable expansion works in Bash, putting video-params/aspect in local allprops broke stuff. The issue is most like the forward slash and how Bash interprets it. As far as I understand the script, the entries from allprops are used to construct environment variable names.

Since I mainly use the identify script on audio files, I have just removed the "offending" line. Which lets things work again.

Not sure how to properly fix this. Most likely the entries from allprops need to be sanitized in some way, before the envvar names can be constructed.

With best wishes,
Tobias

Expected Behavior

Calling mpv_identify.sh should not trigger a Bash error.

Actual Behavior

Calling mpv_identify.sh triggers a Bash error.

Log File

Nothing here.

Sample Files

Nothing here

I carefully read all instruction and confirm that I did the following:

  • I tested with the latest mpv version to validate that the issue is not already fixed.
  • I provided all required information including system and mpv version.
  • I produced the log file with the exact same set of files, parameters, and conditions used in "Reproduction Steps", with the addition of --log-file=output.txt.
  • I produced the log file while the behaviors described in "Actual Behavior" were actively observed.
  • I attached the full, untruncated log file.
  • I attached the backtrace in the case of a crash.
@Traneptora
Copy link
Member

Try applying this patch and see if this fixes the issue:

diff --git a/TOOLS/mpv_identify.sh b/TOOLS/mpv_identify.sh
index fa0e134937..c1bbe1cd5d 100755
--- a/TOOLS/mpv_identify.sh
+++ b/TOOLS/mpv_identify.sh
@@ -94,7 +94,7 @@ EOF
     local key
     for key in $allprops; do
         propstr="${propstr}X-MIDENTIFY: $key \${=$key}$LF"
-        key="$(printf '%s\n' "$key" | tr - _)"
+        key="$(printf '%s\n' "$key" | tr /- _)"
         unset "$nextprefix$key"
     done
 
@@ -112,7 +112,7 @@ EOF
                     fileindex="$((fileindex+1))"
                     nextprefix="${nextprefix}${fileindex}_"
                     for key in $allprops; do
-                        key="$(printf '%s\n' "$key" | tr - _)"
+                        key="$(printf '%s\n' "$key" | tr /- _)"
                         unset "$nextprefix$key"
                     done
                 else
@@ -126,7 +126,7 @@ EOF
                 local key="${line#X-MIDENTIFY: }"
                 local value="${key#* }"
                 key="${key%% *}"
-                key="$(printf '%s\n' "$key" | tr - _)"
+                key="$(printf '%s\n' "$key" | tr /- _)"
                 if [ -n "$nextprefix" ]; then
                     if [ -z "$prefix" ]; then
                         echo >&2 "Got X-MIDENTIFY: without X-MIDENTIFY-START:"

Traneptora added a commit to Traneptora/mpv that referenced this issue Feb 1, 2025
Property names contain '-' characters, which are translated into
underscores because POSIX shell variables cannot contain hyphens.

We should do the same thing for forward slash characters (i.e. '/')
because these also cannot be present in the names of shell variables.

Fixes: mpv-player#15782
Traneptora added a commit to Traneptora/mpv that referenced this issue Feb 4, 2025
Property names contain '-' characters, which are translated into
underscores because POSIX shell variables cannot contain hyphens.

We should do the same thing for forward slash characters (i.e. '/')
because these also cannot be present in the names of shell variables.

Fixes: mpv-player#15782
kasper93 pushed a commit that referenced this issue Feb 5, 2025
Property names contain '-' characters, which are translated into
underscores because POSIX shell variables cannot contain hyphens.

We should do the same thing for forward slash characters (i.e. '/')
because these also cannot be present in the names of shell variables.

Fixes: #15782
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants