Skip to content

Conversation

@pat357
Copy link
Contributor

@pat357 pat357 commented May 4, 2018

When compiling MPV 64bit, you'll notice a warning that "if ARCH_X86_64" evaluates to zero (wundef).
If you look at the code from \build\nv-codec-headers-git\include\ffnvcodec\dynlink_loader.h you will see that this prevents using the 64 bit version from "nvEncodeAPI64.dll" and instead the 32 bit version"nvEncodeAPI.dll"will be used to compile a 64 bit program!
Obviously this can never work.
Adding the cflag -DARCH_X86_64=1 solves this problem.

#if defined(_WIN32) || defined(__CYGWIN__)
# define CUDA_LIBNAME "nvcuda.dll"
# define NVCUVID_LIBNAME "nvcuvid.dll"
# if ARCH_X86_64
#  define NVENC_LIBNAME "nvEncodeAPI64.dll"
# else
#  define NVENC_LIBNAME "nvEncodeAPI.dll"
# endif
#else
# define CUDA_LIBNAME "libcuda.so.1"
# define NVCUVID_LIBNAME "libnvcuvid.so.1"
# define NVENC_LIBNAME "libnvidia-encode.so.1"
#endif

pat357 added 2 commits May 4, 2018 20:56
….dll"



When compiling MPV 64bit, you'll notice a warning that "if ARCH_X86_64" evaluates to zero (wundef).
If you look at the code from \build\nv-codec-headers-git\include\ffnvcodec\dynlink_loader.h you will see that this prevents using the 64 bit version from "nvEncodeAPI64.dll" and instead the 32 bit version"nvEncodeAPI.dll"will be used to compile a 64 bit program!
Obviously this can never work.
Adding the cflag -DARCH_X86_64=1 solves this problem.

#if defined(_WIN32) || defined(__CYGWIN__)
# define CUDA_LIBNAME "nvcuda.dll"
# define NVCUVID_LIBNAME "nvcuvid.dll"
# if ARCH_X86_64
#  define NVENC_LIBNAME "nvEncodeAPI64.dll"
# else
#  define NVENC_LIBNAME "nvEncodeAPI.dll"
# endif
#else
# define CUDA_LIBNAME "libcuda.so.1"
# define NVCUVID_LIBNAME "libnvcuvid.so.1"
# define NVENC_LIBNAME "libnvidia-encode.so.1"
#endif
@wiiaboo
Copy link
Member

wiiaboo commented May 4, 2018

This should be fixed upstream. Send mpv this issue.

Also, mpv doesn't use nvenc directly, so I don't think they'll care about it either.

@wiiaboo wiiaboo closed this May 4, 2018
@wiiaboo wiiaboo added the wontfix label May 4, 2018
@pat357
Copy link
Contributor Author

pat357 commented May 5, 2018

We might as well remove the ffnvcodec-headers from the suite as we both know by now it can never work.
I'm sure you are also against misleading the other users like I'm ; we should warn them to disable this option (because by default it's enabled).

@wiiaboo
Copy link
Member

wiiaboo commented May 5, 2018

What do you mean? It works fine for me. ffmpeg uses both nvdec and nvenc and mpv uses just nvdec.

@pat357
Copy link
Contributor Author

pat357 commented May 5, 2018

The related file "nvEncodeAPI64.dll" is only used for encoding ; FFmpeg figures out on his own when we're compiling 64bit and adds this c-flag automatically. If you look at the ffmpeg code, it has a lot of such "if ARCH_X86_64" statements.

MPV could use the nvEncodeAPI for encoding if it has the right architecture flag to see if the 32bit nvEncodeAPI or the 64 bit nvEncodeAPI64 should be called.

@pat357
Copy link
Contributor Author

pat357 commented May 5, 2018

BTW, I would understand your wontfix if there were major disadvantages to my patch, but AFAIK there are zero/zip/ null/nada disadvantages.
If you see any potential problems with it, please share them with me.

@wiiaboo
Copy link
Member

wiiaboo commented May 5, 2018

Why the hell would mpv use nvenc? It uses libavcodec/libavformat for every part relative to encoding.
Either you're not getting the point, or I'm not explaining it good enough for you.

mpv isn't mencoder. You've compiled FFmpeg with nvenc support, then mpv uses that, it doesn't recompile every fucking shit again like mencoder,

Your patch is pointless since mpv doesn't use nvenc directly, so that define being undefined amounts to fucking nothing being wrong.

@wiiaboo
Copy link
Member

wiiaboo commented May 6, 2018

@pat357
Copy link
Contributor Author

pat357 commented May 6, 2018

Fixed what ? You just said that there was nothing wrong ...?

@wiiaboo
Copy link
Member

wiiaboo commented May 7, 2018

He fixed your non-issue. And hope you enjoyed your last comment.

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 this pull request may close these issues.

2 participants