Port vainfo and tests for libva-win32 #283

sivileri · 2022-09-29T15:24:05Z

This MR ports vainfo and the gtest suite for Windows using libva-win32

Requires: intel/libva#621

evelikov · 2022-09-29T16:08:08Z

As a draft this is great. For the final PR please consider:

keeping the getopt a separate commit with references (source and version/git commit) in the commit message
adding support for non-default (NULL) display
run through style-unify, if you haven't already
porting at least some of the following - decode, encode, videoprocess and tests - hopefully they should be a matter of a trivial compat_win32.h header

The final pieces don't make sense to port IMHO:

vendor/intel - there is no intel driver for win32 (yet at least)
putsurface - the API is not implemented in va-win32, rightfully so IMHO

sivileri · 2022-09-29T17:31:04Z

@evelikov thanks for taking a look, please see inline.

As a draft this is great. For the final PR please consider:

keeping the getopt a separate commit with references (source and version/git commit) in the commit message

[sivileri] Will do.

run through style-unify, if you haven't already

[sivileri] Got it. I ran it through the files I changed, no changes needed.

adding support for non-default (NULL) display

porting at least some of the following - decode, encode, videoprocess and tests - hopefully they should be a matter of a trivial compat_win32.h header

[sivileri] I tried doing this earlier today, I could use a similar win32_compat like the one used for the libva repo for some of the it like the time.h functions and variadic macros, but in here the changes need to be a bit more invasive and will change significant parts of the existing code, the biggest reason being the explicit initialization with <fieldname> : <value> that's used for structs in the tests and doesn't build with MSVC, some tests using pthreads among others and being pure C limits the usage of some features like COM RAII for D3D code, etc.

To avoid doing such a disruptive change on existing code, and considering there are open source samples for VA on Windows here that do adapter enumeration and selection, encode, video processing, importing/exporting of surfaces and interop with D3D12 to present surfaces to screen, maybe we can add those to this repo and/or link them here to the project where those live? I'll work on porting a decode example in that sample app for completeness.

Edit: Added decode sample based on llibva-utils sample

The final pieces don't make sense to port IMHO:

vendor/intel - there is no intel driver for win32 (yet at least)

putsurface - the API is not implemented in va-win32, rightfully so IMHO

[sivileri] Sounds good.

evelikov · 2022-09-29T19:00:33Z

the biggest reason being the explicit initialization with : that's used for structs in the tests and doesn't build with MSVC

Off the top of my head, tests/gtest should just work with MSVC, do shout if it does not. Actual tests like test_va_api_get_create_config.cpp should really be fixed to use standard C++ by either dropping the <fieldname> : or using C++20 designated initialises (the ones we got with C99).

some tests using pthreads among others and being pure C limits the usage of some features like COM RAII for D3D code, etc.

Not familiar with the details off the top of my head. If you have references I might come-up with some half-crazy idea :-)

... maybe we can add those to this repo and/or link them here to the project where those live? I'll work on porting a decode example in that sample app for completeness.

No idea if copying these into the repo will try. Although having at least one actual example ported (apart from vainfo of course) - decode or others, would be amazing IMHO.

common/va_display_win32.c

getopt/getopt.h

evelikov · 2022-09-30T09:18:51Z

Glancing at the in-repo tools - they have been bit-rotting since 2016 when they were split out of libva itself. In libva we've had a dozen APIs and even more enum additions, while this repo has predominantly seen static analyser fixes. There is one notable exception - porting the tests to GTest \o/

Now I see why you saw more value in writing fresh HelloWorld demos instead of digging into the graveyard 👍

Personally I would strongly recommend porting the tests and not worrying about the rest. They should be trivial to fix, plus the Mesa CI should be using them already (or is about to soon).

sivileri · 2022-09-30T14:38:58Z

Glancing at the in-repo tools - they have been bit-rotting since 2016 when they were split out of libva itself. In libva we've had a dozen APIs and even more enum additions, while this repo has predominantly seen static analyser fixes. There is one notable exception - porting the tests to GTest \o/

Now I see why you saw more value in writing fresh HelloWorld demos instead of digging into the graveyard 👍

Personally I would strongly recommend porting the tests and not worrying about the rest. They should be trivial to fix, plus the Mesa CI should be using them already (or is about to soon).

I went ahead and ported the tests too now, I was able to build and run them.

test/test_va_api_fixture.cpp

evelikov · 2022-10-01T08:44:22Z

Incoming some nitpicks and a question for the mid-long run. Overall I'm in favour of the PR and don't see anything being off.

meson.build

test/test_utils.h

test/test_va_api_fixture.cpp

meson.build

test/test_va_api_fixture.cpp

dvrogozh · 2022-10-05T23:12:50Z

let's merge this already - hint maintainers, hint

We are waiting for @XinfengZhang to try libva and libva-utils changes. This won't happen earlier than next week due to holidays in China.

meson.build

sivileri · 2022-10-13T15:03:22Z

@dvrogozh @XinfengZhang now that intel/libva#621 is merged, could you please enable the workflows execution? I'll work on making them work with the libva-win32 dependency from libva/master

sivileri · 2022-10-13T17:59:42Z

@dvrogozh @XinfengZhang now that intel/libva#621 is merged, could you please enable the workflows execution? I'll work on making them work with the libva-win32 dependency from libva/master

There we go: https://github.com/sivileri/libva-utils/actions/runs/3244558566/jobs/5320888674 :) Please let me know if I can do anything else before we can merge this MR too.

dvrogozh · 2022-10-13T18:06:12Z

now that intel/libva#621 is merged, could you please enable the workflows execution?

Approved. But mind that this is a manual action which needs to be done each time till you won't have at least 1 commit merged:). So, if you will push an update it will need a new approval.

meson.build

test/test_va_api_fixture.cpp

dvrogozh · 2022-10-13T18:46:23Z

freebsd build fails (you need to open "test" stage to see actual error):

ld: error: undefined symbol: va_display_hooks_win32

dvrogozh

looks good

What did you do to fix freebsd build? Can you point to the change, please?

evelikov · 2022-10-16T08:02:29Z

What did you do to fix freebsd build? Can you point to the change, please?

The "Compare" button shows them - aka DHAVE_VA_WIN32=1 was incorrectly set.

sivileri · 2022-10-17T11:53:49Z

looks good

What did you do to fix freebsd build? Can you point to the change, please?

Thanks! DHAVE_VA_WIN32=1 was being always set disregarding of the dependency check. Here's the fix from the latest push:
https://github.com/intel/libva-utils/compare/b9d961ecd78b5c742aa834abbb4e3c775cb70e9d..88cd38ba4834c6da42d7084444fc6adbab9affdc

dvrogozh · 2022-10-17T14:58:26Z

Thank you for pointing out. I am fine with the change. @XinfengZhang : can you, please, take a look and merge?

XinfengZhang · 2022-10-19T14:18:33Z

meson.build

-subdir('vendor/intel/sfcsample')
+
+if not use_win32
+  subdir('decode')


is it possible to make sample codes work on windows?

For most of them we don't have driver support yet to test it (ie. mpeg2, jpeg decode, vp8/vp9 encode and most VPP effects tested). I think the ones we could port and test today are: encode H264/HEVC and VPP scaling and color conversion (only NV12, P010 and RGBX supported in driver at the moment).

From those we could port and test at the moment, cross-platform porting is needed, the most notable port needed would be pthreads for the encode samples.

As we're also in the process of writing new samples for Windows that cover those same Decode/Encode/VPP scenarios implemented by the driver today, also showing D3D12/Libva interoperability and adapter enumeration/selection, could we maybe point to those samples instead in the documentation? The goal is to upstream the open source samples to DirectX-Graphics-Samples when finished.

You can find the in-progress samples in: H264 decode sample similar to intel/sfcsample and VPP Color Conversion + H264 encode that show encode and color conversion

test/test_utils.h

Signed-off-by: Sil Vilerino <sivileri@microsoft.com>

Added windows.yml and EnterDevShell.ps1 for MSVC/mingw building with ninja/meson backend. Signed-off-by: Sil Vilerino <sivileri@microsoft.com>

sivileri mentioned this pull request Sep 29, 2022

va: Add Win32 Display/node and meson Windows build intel/libva#621

Merged

sivileri force-pushed the va_win_dev branch from b28f64b to 2525817 Compare September 29, 2022 17:15

sivileri force-pushed the va_win_dev branch from 2525817 to ee9b724 Compare September 29, 2022 21:18

dvrogozh reviewed Sep 30, 2022

View reviewed changes

common/va_display_win32.c Show resolved Hide resolved

getopt/getopt.h Show resolved Hide resolved

sivileri force-pushed the va_win_dev branch from ee9b724 to aad3399 Compare September 30, 2022 14:37

sivileri changed the title ~~Port vainfo for libva-win32~~ Port vainfo and tests for libva-win32 Sep 30, 2022

sivileri force-pushed the va_win_dev branch from aad3399 to 4e9b024 Compare September 30, 2022 14:44

sivileri commented Sep 30, 2022

View reviewed changes

test/test_va_api_fixture.cpp Outdated Show resolved Hide resolved

evelikov reviewed Oct 1, 2022

View reviewed changes

meson.build Outdated Show resolved Hide resolved

evelikov reviewed Oct 1, 2022

View reviewed changes

meson.build Outdated Show resolved Hide resolved

evelikov reviewed Oct 1, 2022

View reviewed changes

test/test_utils.h Show resolved Hide resolved

evelikov reviewed Oct 1, 2022

View reviewed changes

test/test_va_api_fixture.cpp Outdated Show resolved Hide resolved

sivileri force-pushed the va_win_dev branch from 4e9b024 to deb9c59 Compare October 4, 2022 12:54

sivileri marked this pull request as ready for review October 4, 2022 12:58

dvrogozh requested changes Oct 5, 2022

View reviewed changes

meson.build Show resolved Hide resolved

test/test_va_api_fixture.cpp Outdated Show resolved Hide resolved

test/test_va_api_fixture.cpp Outdated Show resolved Hide resolved

test/test_va_api_fixture.cpp Show resolved Hide resolved

sivileri force-pushed the va_win_dev branch from deb9c59 to d42b80a Compare October 6, 2022 13:22

sivileri requested a review from dvrogozh October 6, 2022 14:15

dvrogozh reviewed Oct 6, 2022

View reviewed changes

meson.build Outdated Show resolved Hide resolved

sivileri force-pushed the va_win_dev branch from d42b80a to b8c0cd4 Compare October 11, 2022 14:02

sivileri requested a review from dvrogozh October 13, 2022 15:03

sivileri force-pushed the va_win_dev branch 2 times, most recently from 0f391c8 to d46dd7e Compare October 13, 2022 15:25

sivileri force-pushed the va_win_dev branch 6 times, most recently from d114d7e to b9d961e Compare October 13, 2022 17:54

dvrogozh requested changes Oct 13, 2022

View reviewed changes

meson.build Outdated Show resolved Hide resolved

test/test_va_api_fixture.cpp Outdated Show resolved Hide resolved

sivileri force-pushed the va_win_dev branch from b9d961e to 88cd38b Compare October 13, 2022 19:42

sivileri requested a review from dvrogozh October 13, 2022 19:49

dvrogozh approved these changes Oct 15, 2022

View reviewed changes

dvrogozh requested a review from XinfengZhang October 15, 2022 02:33

XinfengZhang approved these changes Oct 19, 2022

View reviewed changes

sivileri added 6 commits October 21, 2022 09:12

Import OpenBSD getopt for MSVC from mesa/f9bb5323

4d965ae

Signed-off-by: Sil Vilerino <sivileri@microsoft.com>

vainfo: add va-win32 support

e70eb3f

Signed-off-by: Sil Vilerino <sivileri@microsoft.com>

tests: remove non C++ standard constructs

9e92b99

Signed-off-by: Sil Vilerino <sivileri@microsoft.com>

tests: Remove C++20 constructs in tests project targetting C++11

451268c

Signed-off-by: Sil Vilerino <sivileri@microsoft.com>

tests: enable building on Windows

b444a23

Signed-off-by: Sil Vilerino <sivileri@microsoft.com>

ci: Add msvc/mingw Windows CI

8aef416

Added windows.yml and EnterDevShell.ps1 for MSVC/mingw building with ninja/meson backend. Signed-off-by: Sil Vilerino <sivileri@microsoft.com>

sivileri force-pushed the va_win_dev branch from 88cd38b to 8aef416 Compare October 21, 2022 13:13

XinfengZhang merged commit 6bc3ec4 into intel:master Oct 23, 2022

sivileri deleted the va_win_dev branch February 7, 2023 13:28

Port vainfo and tests for libva-win32 #283

Port vainfo and tests for libva-win32 #283

Uh oh!

Conversation

sivileri commented Sep 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

evelikov commented Sep 29, 2022

Uh oh!

sivileri commented Sep 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

evelikov commented Sep 29, 2022

Uh oh!

Uh oh!

Uh oh!

evelikov commented Sep 30, 2022

Uh oh!

sivileri commented Sep 30, 2022

Uh oh!

Uh oh!

evelikov commented Oct 1, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dvrogozh commented Oct 5, 2022

Uh oh!

Uh oh!

sivileri commented Oct 13, 2022

Uh oh!

sivileri commented Oct 13, 2022

Uh oh!

dvrogozh commented Oct 13, 2022

Uh oh!

Uh oh!

Uh oh!

dvrogozh commented Oct 13, 2022

Uh oh!

dvrogozh left a comment

Choose a reason for hiding this comment

Uh oh!

evelikov commented Oct 16, 2022

Uh oh!

sivileri commented Oct 17, 2022

Uh oh!

dvrogozh commented Oct 17, 2022

Uh oh!

XinfengZhang Oct 19, 2022

Choose a reason for hiding this comment

Uh oh!

sivileri Oct 19, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

sivileri commented Sep 29, 2022 •

edited

Loading

sivileri commented Sep 29, 2022 •

edited

Loading