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-x86_64-v3-20231106-git-84de84b.7z build doesn't work #12818

Closed
Obegg opened this issue Nov 6, 2023 · 16 comments · Fixed by #12821
Closed

mpv-x86_64-v3-20231106-git-84de84b.7z build doesn't work #12818

Obegg opened this issue Nov 6, 2023 · 16 comments · Fixed by #12821
Labels

Comments

@Obegg
Copy link

Obegg commented Nov 6, 2023

(Created the same issue on mpv-winbuild-cmake in case this is not a mpv issue)

mpv-x86_64-v3-20231105-git-7480efa.7z = No issues.
mpv-x86_64-v3-20231106-git-84de84b.7z = mpv.exe have issues.
I tried to debug the issue by using the log-file command but mpv doesn't seem to create the file, so I can't debug it.
Weirdly enough when I open Task Manager I do see mpv.exe.

It's probably not an issue with mpv-packaging because nothing was changed today.
So my guess it's either mpv daily changes or it's mpv-winbuild-cmake daily changes.

(Yes, I know I ignored-issue-template but I can't provide logs because of this issue)

@Obegg Obegg added the os:win label Nov 6, 2023
@Dudemanguy
Copy link
Member

Do any of today's build artifacts work for you?

@Hrxn
Copy link
Contributor

Hrxn commented Nov 6, 2023

Can confirm for

So 84de84b

Same on my machine..
Running mpv simply does not terminate. Seems like stuck in an infinite loop..

NB; For those not really aware how this works on Windows:
mpv actually means mpv.com which then runs mpv.exe, basically..
But I haven't seen any changes related to mpv.com in a long time, so I guess it's the other artifact that's bad.

Edit:

@Obegg
Not sure, but I don't actually see any leftover mpv.exe running in my task manager...
What happens on your side if you first run mpv and then do Ctrl+C?

@Dudemanguy
Copy link
Member

Are you positive it's actually 84de84b? I would have assumed the windows threading stuff (so 076be24 as I linked above).

@zhongfly
Copy link

zhongfly commented Nov 6, 2023

@Hrxn It is recommended to use the link file provided by Dudemanguy for testing, there was also an update to mpv-winbuild-cmake 13 hours ago and we need to rule it out.

@Hrxn
Copy link
Contributor

Hrxn commented Nov 6, 2023

I am positive..

But by looking at https://github.com/mpv-player/mpv/commits/master/
Seems here
84de84b is the newest commit
076be24 is seven commits down from the latest

So the automatic builds from @zhongfly and @shinchiro should include this obviously.

I'm afraid, but it might acutally be the windows threading stuff 😄

@Hrxn
Copy link
Contributor

Hrxn commented Nov 6, 2023

FFS, I cannot download the automatic build artifacts linked by @Dudemanguy above with curl or pwsh.

Only in the browser, apparently, but then they all have the same "wrong" filename, which is annoying.

@Hrxn
Copy link
Contributor

Hrxn commented Nov 6, 2023

Ah, well, nevermind.

Dudemanguy's links above all get me zipfiles, each with a contained zipfile, and these have the (shortened) SHA commit ID, so it's okay.

@Hrxn
Copy link
Contributor

Hrxn commented Nov 6, 2023

Okay, just tested
84de84b

And it does NOT show the behaviour mentioned above.
So the windows threading stuff may actually be fine, and the culprit is the cmake stuff that @zhongfly mentioned

@sevz17
Copy link

sevz17 commented Nov 6, 2023

Hello, this sounds pretty similar to my problem.
I'm using Gentoo, the problem does not happen when I build mpv directly from git, it only happens when I install it from portage

The last working commit is 3a8b107

Here is the output of strace using 174df99: http://0x0.st/HtQg.strace

@Dudemanguy
Copy link
Member

Can you narrow down whatever compiler flag you're using that makes the difference?

@sevz17
Copy link

sevz17 commented Nov 6, 2023

I used the same meson arguments and {C,LD}FLAGS

EDIT: I just removed the pkg-config paths and the --native-file args that portage adds to the meson invocation

@Hrxn
Copy link
Contributor

Hrxn commented Nov 6, 2023

To be honest @sevz17, I don't think this is the same issue.

Playback is working for me here (Windows 10 22H2 19045.3636)

PS C:\> mpv -V
mpv 84de84b Copyright © 2000-2023 mpv/MPlayer/mplayer2 projects
 built on Nov  5 2023 18:05:11
libplacebo version: v7.341.0
FFmpeg version: git-2023-11-05-44a0148
FFmpeg library versions:
   libavutil       58.31.100
   libavcodec      60.32.102
   libavformat     60.17.100
   libswscale      7.6.100
   libavfilter     9.13.100
   libswresample   4.13.100

PS C:\>

This is 84de84b which should be the latest commit.

Only one issue here:
Terminal gets spammed with this

[vo/gpu-next/libplacebo] libplacebo compiled without LittleCMS 2 support!
[vo/gpu-next/libplacebo] libplacebo compiled without LittleCMS 2 support!
AO: [wasapi] 48000Hz stereo 2ch float
VO: [gpu-next] 1920x1080 yuv420p
[vo/gpu-next/libplacebo] libplacebo compiled without LittleCMS 2 support!
[vo/gpu-next/libplacebo] libplacebo compiled without LittleCMS 2 support!
AV: 00:00:00 / 00:24:26 (0%) A-V:  0.000
[vo/gpu-next/libplacebo] libplacebo compiled without LittleCMS 2 support!
AV: 00:00:00 / 00:24:26 (0%) A-V:  0.006
[vo/gpu-next/libplacebo] libplacebo compiled without LittleCMS 2 support!
AV: 00:00:00 / 00:24:26 (0%) A-V:  0.005
[vo/gpu-next/libplacebo] libplacebo compiled without LittleCMS 2 support!
AV: 00:00:00 / 00:24:26 (0%) A-V:  0.005
[vo/gpu-next/libplacebo] libplacebo compiled without LittleCMS 2 support!

@Dudemanguy
Copy link
Member

EDIT: I just removed the pkg-config paths and the --native-file args that portage add to the meson invocation

That's could be a big difference..., linking to different libraries, etc. who knows what's going on here. I'm not sure whatever you're experiencing is actually the same thing.

@sevz17
Copy link

sevz17 commented Nov 6, 2023

Probably, do you want me to open another issue?

@Dudemanguy
Copy link
Member

Sure feel free to.

@kasper93
Copy link
Contributor

kasper93 commented Nov 6, 2023

Just so we don't wonder around too far.

The root cause of the issue is that we deadlock during init and AcquireSRWLockExclusive() due to recursive lock. This mutex has MP_MUTEX_RECURSIVE flag set, so it should never call SRW lock. And with my build and as we can see build from mpv's CI it does work correctly and calls EnterCriticalSection(). I've looked quickly at the file and I don't see anything obviously wrong, any stupid UB that could explain that miscompilation. I'm almost sleeping rn, so maybe I missed something...

ntdll.dll!NtWaitForAlertByThreadId()
ntdll.dll!RtlAcquireSRWLockExclusive()
[Inline Frame] mpv.exe!mp_mutex_lock(mp_mutex * mutex) Line 69
mpv.exe!mp_input_disable_section(input_ctx * ictx, char * name) Line 988
mpv.exe!mp_input_enable_section(input_ctx * ictx, char * name, int flags) Line 1008
mpv.exe!mp_input_init(mpv_global * global, void(*)(void *) wakeup_cb, void * wakeup_ctx) Line 1330
mpv.exe!mp_create() Line 295

EDIT: Removed dummy diff. Issue will be fixed by #12821

kasper93 added a commit to kasper93/mpv that referenced this issue Nov 6, 2023
Dudemanguy pushed a commit that referenced this issue Nov 6, 2023
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.

6 participants