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] missing cdda & dvdnav features #23211

Open
llm96 opened this issue Jan 27, 2025 · 8 comments · Fixed by #23212
Open

[mpv] missing cdda & dvdnav features #23211

llm96 opened this issue Jan 27, 2025 · 8 comments · Fixed by #23212
Labels
enhancement Package requires more dependencies, build options, different packaging style etc.

Comments

@llm96
Copy link

llm96 commented Jan 27, 2025

Package name

mpv

New version number

N/A

Other information that may be useful

The mpv package currently depends on libcdio, libcdio-paranoia, libdvdnav & libdvdread but doesn't enable the related features.

Seems like it was enabled before the switch to Meson:
https://github.com/lazka/MINGW-packages/blob/e0d5ae00d1b21721da4371249145735b3c113cc1/mingw-w64-mpv/PKGBUILD#L73-L91

Would be cool to have them enabled again. Arch does it but none of the Windows builds from mpv.io do.

[cplayer] Configuration: -Dlibmpv=true -Djavascript=enabled -Dprefix=/ucrt64 -Dbuildtype=plain -Dwrap_mode=nodownload
[cplayer] List of enabled features: build-date caca cplugins d3d-hwaccel d3d11 d3d9-hwaccel direct3d dos-paths dxgi-de
bug dxgi-debug-d3d11 ffmpeg gl gl-dxinterop gl-dxinterop-d3d9 gl-win32 glob glob-win32 gpl iconv javascript jpeg lcms2
 libarchive libass libavdevice libbluray libplacebo lua5.1 pathcch rubberband rubberband-3 shaderc spirv-cross uchardet
 vaapi vaapi-win32 vapoursynth vector vk-khr-display vulkan wasapi win32 win32-desktop win32-executable win32-threads
 zimg zimg-st428 zlib
# mpv -h=cdda
Usage:   mpv [options] [url|path/]filename

Basic options:
 --start=<time>    seek to given (percent, seconds, or hh:mm:ss) position
 --no-audio        do not play sound
 --no-video        do not play video
 --fs              fullscreen playback
 --sub-file=<file> specify subtitle file to use
 --playlist=<file> specify playlist file

 --list-options    list all mpv options
 --h=<string>      print options which contain the given string in their name
Options:


Total: 0 options

Are you willing to submit a PR?

Yes

@Biswa96 Biswa96 added enhancement Package requires more dependencies, build options, different packaging style etc. and removed update-request labels Jan 28, 2025
@Biswa96
Copy link
Member

Biswa96 commented Jan 28, 2025

Thank you for reporting the issue. I have added a pull request to fix this issue. Would you like to test the packages from GitHub Actions artifacts of that pull request? https://github.com/msys2/MINGW-packages/actions/runs/13005037936

@llm96
Copy link
Author

llm96 commented Jan 28, 2025

Thanks! CD playback now works as expected with all of the built packages.

https://github.com/user-attachments/assets/7d291552-63f5-4d92-8e3c-0707d622abda

DVD playback works in MINGW64 & UCRT64.

https://github.com/user-attachments/assets/70fcc275-5014-4a51-928c-d910224fff5d

Hitting an assertion in libdvdnav with CLANG64 though:
https://code.videolan.org/videolan/libdvdnav/-/blob/6.1.1/src/vm/getset.c#L52

Image

@llm96
Copy link
Author

llm96 commented Jan 28, 2025

Narrowed it down to libdvdread. Building against this commit fixes the crash for me:
https://code.videolan.org/videolan/libdvdread/-/commit/ea158e812abe9990c971b095a13d35c9bfae4cc7

@Biswa96
Copy link
Member

Biswa96 commented Jan 28, 2025

Narrowed it down to libdvdread.

Awesome 👍 I have emailed the author of that commit to confirm it.

@robUx4
Copy link
Contributor

robUx4 commented Jan 29, 2025

We also have this patch in VLC. It is necessary for LLVM builds.

It has been merged upstream but there's no release with it yet.

@Biswa96
Copy link
Member

Biswa96 commented Jan 29, 2025

Is that the only patch required to fix the llvm related issue?

@robUx4
Copy link
Contributor

robUx4 commented Jan 29, 2025

Is that the only patch required to fix the llvm related issue?

That's the minimum one. There's a set of patches that avoids reading packed structures straight to memory but it hasn't been approved yet (but tests show it's working fine)

Then there's dvdnav that needs patching. It can also do with these patches to avoid more asserts/exit on bogus DVDs.

@Biswa96
Copy link
Member

Biswa96 commented Jan 29, 2025

Thanks for the info. I don't have the hardware to test those patches. I shall keep this issue open for those who can fix it with proper testing.

@Biswa96 Biswa96 reopened this Jan 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Package requires more dependencies, build options, different packaging style etc.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants