intel / libva

Libva is an implementation for VA-API (Video Acceleration API)
http://intel.github.io/libva/
Other
664 stars 303 forks source link

win32: Replace fprintf with proper VA_TRACE function #695

Closed sivileri closed 1 year ago

sivileri commented 1 year ago

Closes #694

Tested with environment variable LIBVA_TRACE=1. Trace test when running vainfo:

[33752.134567][ctx       none]==========va_TraceInitialize
[33752.134574][ctx       none]==========    VA-API vendor string: Mesa Gallium driver 23.1.0-devel for D3D12 (NVIDIA GeForce RTX 4090)
[33752.134575][ctx       none]=========vaInitialize ret = VA_STATUS_SUCCESS, success (no error) 
[33752.134679][ctx       none]=========vaQueryConfigProfiles ret = VA_STATUS_SUCCESS, success (no error) 
[33752.134696][ctx       none]=========vaQueryConfigEntrypoints ret = VA_STATUS_SUCCESS, success (no error) 
[33752.134700][ctx       none]=========vaQueryConfigEntrypoints ret = VA_STATUS_SUCCESS, success (no error) 
[33752.134704][ctx       none]=========vaQueryConfigEntrypoints ret = VA_STATUS_SUCCESS, success (no error) 
[33752.134712][ctx       none]=========vaQueryConfigEntrypoints ret = VA_STATUS_SUCCESS, success (no error) 
[33752.134716][ctx       none]=========vaQueryConfigEntrypoints ret = VA_STATUS_SUCCESS, success (no error) 
[33752.134719][ctx       none]=========vaQueryConfigEntrypoints ret = VA_STATUS_SUCCESS, success (no error) 
[33752.134720][ctx       none]=========vaQueryConfigEntrypoints ret = VA_STATUS_SUCCESS, success (no error) 
[33752.134725][ctx       none]=========vaQueryConfigEntrypoints ret = VA_STATUS_SUCCESS, success (no error) 
[33752.134726][ctx       none]=========vaQueryConfigEntrypoints ret = VA_STATUS_SUCCESS, success (no error) 
[33752.160574][ctx       none]==========va_TraceTerminate
[33752.160603][ctx       none]=========vaTerminate ret = VA_STATUS_SUCCESS, success (no error) 
sivileri commented 1 year ago

This fixes #694 which fix is to be picked up by https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/4114#note_1814706

CC @XinfengZhang @dvrogozh @seungha-yang

evelikov commented 1 year ago

None of the other four (or six if we also count nvctl and fglrx) backends have any debug or info messages. There are some error messages, which are very hard to reach in reality.

@sivileri do we need the fprint/trace within the win32 driver in the first place? This PR adds additional API/ABI which varies across platforms - which is rarely a good sign.

sivileri commented 1 year ago

None of the other four (or six if we also count nvctl and fglrx) backends have any debug or info messages. There are some error messages, which are very hard to reach in reality.

@sivileri do we need the fprint/trace within the win32 driver in the first place? This PR adds additional API/ABI which varies across platforms - which is rarely a good sign.

I was following what this existing backend is doing as reference to get the va_trace* functions available from the backend, based on the discussion on #694 to add a trace message instead of just printing. https://github.com/intel/libva/blob/97cbc87c94351c62a47d87e776488a9317ad398b/va/x11/va_x11.c#L290

Knowing which adapter LUID and associated driver was loaded is important in my opinion for tracing/debugging purposes, and looked like using the tracing library was the preferred method in this project to log events like such, but I'd be okay with printing instead if that's cleaner. To solve #694 then I'd surround the printing in the backend with #if DEBUG guards so it's not so noisy for apps like gstreamer.

evelikov commented 1 year ago

That is tracing at the API level, since it is a platform specific API (which is bad on it's own right). The x11 backend does not print "found driver X", so the win32 shouldn't do either?

sivileri commented 1 year ago

I still think it's important to have trace information about the adapter/driver in the traces, but agree maybe there's a better way to do this that's not backend specific and doesn't add cross library binary dependencies. vaQueryVendorString should provide this information.

I modified the approach of this PR to solve the issue, dropping the win32 backend print messages and now logging vaQueryVendorString (if available) from the driver, in va_TraceInitialize

sivileri commented 1 year ago

@XinfengZhang could we please merge this one?