Is it Ok to get into this catch? I mean even now it can be just exited correctly?

The same comment can be applied to all the catch blocks below

it's OK here: swap_surface can throw exception in case of no available "free" surface is present in pool. This lack-of-resource state processing (like std::bad_alloc for new allocation ) allows us to go on async-waiting result state and put off decoder queuing job until one or more async result completed and resource freeing

The issue of bad_alloc is that it can be handled explicitly - here we handle all the exceptions uniformly and this bad-alloc / general error difference is not clear to me

This is the reason why I adhered my point about using abort() in GAPI_Assert instead of throwing exception:
because abort() means the library itself problem which is not supposed to be handed by user but introducing BUG to fix. Otherwise exception has recoverable semantic

In current catch block we could face with two different kind of exception:

swap_surface ->get_free_surface() -> find KEY in pool (exception means library problem and supposed to be abort() with BUG/Issue creation)

and the second

swap_surface ->get_free_surface()-> pool.find_free() -> no free surface at the time (resource unavailable at moment)

The second is expected to be processed.
No other exception types is throwing.

So, according to idea that GAPI_Assert has no abort inside - we need to catch only second case. So it make sense to find out what types of exception is throwing by GAPI_Assert and sort out different types in pool.find_free

should we put false here as second argument to not complain on file not found error? Or I am wrong?

maybe yes, i will add it

looks like false is confusing here. it added but has reverted

From my perspective:

1. if user didn't configured EXTRA path in here initTestDataPathSilent() then SkipException is OK here
2. IF user did configured EXTRA path here, then IT MUST contain requested test files. So findDataFile must request it availability

Please align it with the latest news from @mpashchenkov

asked a detailed question in @mpashchenkov PR by #20995 (comment)

#20995 (comment)
So:
test must fail if no env OPENCV_TEST_DATA_PATH is set (in case of test requires external test data)
test must fail if OPENCV_TEST_DATA_PATH set and filepath is incorrect.

Looks like i need to remove my initTestDataPathSilent check

Don't you find using std::tie(src, type) = params; better?

as for me this transforms into the same 3 lines

The idea behind using std::tie instead of get<N> is type safety

i'm not sure that it is important in UTs, because an one tuple type is for a single one test and when you changed the tuple set - it had caused by the reason that you intended to change test by itself.
If a some common tuple set (hypothetically) was changed by adding new type for a some specific single new one test then it is more convenient for me to do not brake ALL tests which depended from common tuple set by adding std::ignore for all of them (in case if i have 100500 legacy unit tests).

Additionally, AND which is more important for me: please compare

std::string path;
...
std::tie(path,...) = GetParams();
path = findDataFile(path);
...

vs

auto path = findDataFile(std::get<0>(param));

because an one tuple type is for a single one test and when you changed the tuple set - it had caused by the reason that you intended to change test by itself.

Not always, unfortunately - sometimes these parameters are introduced only for particular tests in the suite and the rest is not updated.

Why there are difference between instantiations? std::string is not deducible?

first function deduces template argument as const char* which uses as type for construction cv::variant later.
cv::variant is pretty different from std::variant and cannot support construct_as semantic for std::string (At the moment when current PR introduced). Thus explicit template instantiation requested here due to limited functionality of cv::variant

it is still more tricky to read !int()
Is this some common way to represent logic statement in some famous libraries (likes STD)?
Just a minor question, not to address!

I wonder when you asked - as for me it's well known expression, especially in C in the same way as if (num %2) and so on

+1 to Asya's point, get_lock_count() == 0 is more straight to understand

ok will change by majority request

sorry for such comment!
qecode -> decode

thanks

object

thanks

different order of operands in comparison against if above, however, such order is mostly required for == comparison...

is it intended?

it was written in different times of day :)

we usually use std::tie instead of std::get<>

hmmm, wondering why it isn't under ifdef ?

it makes sense.
What is the current order for UT: to skip or to disable ?

added #ifdef HAVE_ONEVPL in the beginning

disable (put under ifdef)

I'd start with capital and usually we use these tags before comment:
NB, FIXME, TODO

sorry, what is NB ?

Nota bene

The idea behind using std::tie instead of get<N> is type safety

to be honest I'm not sure how well it maps to the idea of a performance test this way.

TEST_CYCLE() measures the same thing again and again to calculate its statistic values. Here you actually decode different frames, but measure it as the same random value - not sure if it is correct.

Maybe running the whole file under the same TEST_CYCLE would make sense w.r.t stats, but then this test can take foerever...

I's true, because different frames take up different decode duration but...

It's supposed to be compared in performance metrics against VideoCap on the same input streams (and frames) AND find out hotspots by using Vtune metrics in overall pipeline. I means auxiliary code around on direct MFX decode api

Wrapping TEST_CYCLE for all case and to incorporate a source creation also will measure MFX library/DX11 CoCreate load/unload and so on and doesn't give me relative result performance in comparison with VideoCapdecode api

Wrapping TEST_CYCLE for all case and to incorporate a source creation also will measure MFX library/DX11 CoCreate load/unload and so on and doesn't give me relative result performance in comparison with VideoCapdecode api

This makes sense, but can it be done only once before the TEST_CYCLE() body kicks in? Is there a way to decompose it to prologue/epilogue so that initialization/deinitialization for every individual run is left out of profile?

I believe this is the method which throws SkipTestException so we don't need to test it on our own

It receives initial PATH and return modified PATH according to the OPENCV_TEST_DAT_PATH prefix but for only single case if file is found by path in result.
So, if path doesn't find then no modified PATH returned and no exception was throws in case of second false params.
So i was confused by changing second params to false
Let's remove false here - false means optional, so when requested file doesn't found it returns non-modified path and throw Skip otherwise.

My suggestions:
if initTestDataPathSilent is success, that means OPENCV EXTRA is present that MUST provide test video file by requested path otherwise configuration is incorrect. so false must be removed

Please align with @mpashchenkov

Nota bene

The issue of bad_alloc is that it can be handled explicitly - here we handle all the exceptions uniformly and this bad-alloc / general error difference is not clear to me

here too

and here too

disable (put under ifdef)