[Libva] [PATCH 0/2 v2][libva-intel-driver] report unsupported profile

Discussion:

Daniel Charles

2016-10-26 22:46:17 UTC

This is a re-send of patch 1/2 and a new patch that modifies the current
unittests to properly test the unsupported profile condition.

As mentioned in the commit messages, UNSUPPORTED_PROFILE will be returned
when a queried profile is not supported by any entrypoint.

If squashing the patches is preferred, let me know and I'll resubmit.

v2: do not change the checks for specific h/w, i.e. IS_SKL as it will
change the purpose of the test

Daniel Charles (2):
i965_validate_config: return unsupported profile
i965_test_config: return properly unsupported profile

src/i965_drv_video.c | 49 +++++++++++++++++++++++++++++++++--------
test/i965_avce_config_test.cpp | 25 +++++++++++++++------
test/i965_jpege_config_test.cpp | 17 +++++++-------
3 files changed, 66 insertions(+), 25 deletions(-)

--
2.5.5

Daniel Charles

2016-10-26 22:46:18 UTC

Permalink

When all the profiles are not supported return
VA_STATUS_ERROR_UNSUPPORTED_PROFILE instead of
VA_STATUS_ERROR_UNSUPPORTED_ENTRYPOINT.

Also change the style on the code modified to be common
on all cases

Signed-off-by: Daniel Charles <***@intel.com>
---
src/i965_drv_video.c | 49 ++++++++++++++++++++++++++++++++++++++++---------
1 file changed, 40 insertions(+), 9 deletions(-)

diff --git a/src/i965_drv_video.c b/src/i965_drv_video.c
index 7f2146e..e17c295 100644
--- a/src/i965_drv_video.c
+++ b/src/i965_drv_video.c
@@ -711,6 +711,8 @@ i965_validate_config(VADriverContextP ctx, VAProfile profile,
if ((HAS_MPEG2_DECODING(i965) && entrypoint == VAEntrypointVLD) ||
(HAS_MPEG2_ENCODING(i965) && entrypoint == VAEntrypointEncSlice)) {
va_status = VA_STATUS_SUCCESS;
+ } else if (!HAS_MPEG2_DECODING(i965) && !HAS_MPEG2_ENCODING(i965)){
+ va_status = VA_STATUS_ERROR_UNSUPPORTED_PROFILE;
} else {
va_status = VA_STATUS_ERROR_UNSUPPORTED_ENTRYPOINT;
}
@@ -723,6 +725,9 @@ i965_validate_config(VADriverContextP ctx, VAProfile profile,
(HAS_H264_ENCODING(i965) && entrypoint == VAEntrypointEncSlice) ||
(HAS_LP_H264_ENCODING(i965) && entrypoint == VAEntrypointEncSliceLP)) {
va_status = VA_STATUS_SUCCESS;
+ } else if (!HAS_H264_DECODING(i965) && !HAS_H264_ENCODING(i965) &&
+ !HAS_LP_H264_ENCODING(i965)){
+ va_status = VA_STATUS_ERROR_UNSUPPORTED_PROFILE;
} else {
va_status = VA_STATUS_ERROR_UNSUPPORTED_ENTRYPOINT;
}
@@ -733,6 +738,8 @@ i965_validate_config(VADriverContextP ctx, VAProfile profile,
case VAProfileVC1Advanced:
if (HAS_VC1_DECODING(i965) && entrypoint == VAEntrypointVLD) {
va_status = VA_STATUS_SUCCESS;
+ } else if (!HAS_VC1_DECODING(i965)) {
+ va_status = VA_STATUS_ERROR_UNSUPPORTED_PROFILE;
} else {
va_status = VA_STATUS_ERROR_UNSUPPORTED_ENTRYPOINT;
}
@@ -741,6 +748,8 @@ i965_validate_config(VADriverContextP ctx, VAProfile profile,
case VAProfileNone:
if (HAS_VPP(i965) && VAEntrypointVideoProc == entrypoint) {
va_status = VA_STATUS_SUCCESS;
+ } else if (!HAS_VPP(i965)){
+ va_status = VA_STATUS_ERROR_UNSUPPORTED_PROFILE;
} else {
va_status = VA_STATUS_ERROR_UNSUPPORTED_ENTRYPOINT;
}
@@ -750,6 +759,8 @@ i965_validate_config(VADriverContextP ctx, VAProfile profile,
if ((HAS_JPEG_DECODING(i965) && entrypoint == VAEntrypointVLD) ||
(HAS_JPEG_ENCODING(i965) && entrypoint == VAEntrypointEncPicture)) {
va_status = VA_STATUS_SUCCESS;
+ } else if (!HAS_JPEG_DECODING(i965) && !HAS_JPEG_ENCODING(i965)){
+ va_status = VA_STATUS_ERROR_UNSUPPORTED_PROFILE;
} else {
va_status = VA_STATUS_ERROR_UNSUPPORTED_ENTRYPOINT;
}
@@ -759,6 +770,8 @@ i965_validate_config(VADriverContextP ctx, VAProfile profile,
if ((HAS_VP8_DECODING(i965) && entrypoint == VAEntrypointVLD) ||
(HAS_VP8_ENCODING(i965) && entrypoint == VAEntrypointEncSlice)) {
va_status = VA_STATUS_SUCCESS;
+ } else if (!HAS_VP8_DECODING(i965) && !HAS_VP8_ENCODING(i965)){
+ va_status = VA_STATUS_ERROR_UNSUPPORTED_PROFILE;
} else {
va_status = VA_STATUS_ERROR_UNSUPPORTED_ENTRYPOINT;
}
@@ -768,8 +781,12 @@ i965_validate_config(VADriverContextP ctx, VAProfile profile,
case VAProfileH264StereoHigh:
if ((HAS_H264_MVC_DECODING_PROFILE(i965, profile) &&
entrypoint == VAEntrypointVLD) ||
- (HAS_H264_MVC_ENCODING(i965) && entrypoint == VAEntrypointEncSlice)) {
+ (HAS_H264_MVC_ENCODING(i965) &&
+ entrypoint == VAEntrypointEncSlice)) {
va_status = VA_STATUS_SUCCESS;
+ } else if(!HAS_H264_MVC_DECODING_PROFILE(i965, profile) &&
+ !HAS_H264_MVC_ENCODING(i965)) {
+ va_status = VA_STATUS_ERROR_UNSUPPORTED_PROFILE;
} else {
va_status = VA_STATUS_ERROR_UNSUPPORTED_ENTRYPOINT;
}
@@ -778,32 +795,46 @@ i965_validate_config(VADriverContextP ctx, VAProfile profile,

case VAProfileHEVCMain:
if ((HAS_HEVC_DECODING(i965) && (entrypoint == VAEntrypointVLD))||
- (HAS_HEVC_ENCODING(i965) && (entrypoint == VAEntrypointEncSlice)))
+ (HAS_HEVC_ENCODING(i965) && (entrypoint == VAEntrypointEncSlice))) {
va_status = VA_STATUS_SUCCESS;
- else
+ } else if (!HAS_HEVC_DECODING(i965) && !HAS_HEVC_ENCODING(i965)) {
+ va_status = VA_STATUS_ERROR_UNSUPPORTED_PROFILE;
+ } else {
va_status = VA_STATUS_ERROR_UNSUPPORTED_ENTRYPOINT;
+ }

break;

case VAProfileHEVCMain10:
if ((HAS_HEVC10_DECODING(i965) && (entrypoint == VAEntrypointVLD))||
- (HAS_HEVC10_ENCODING(i965) && (entrypoint == VAEntrypointEncSlice)))
+ (HAS_HEVC10_ENCODING(i965) &&
+ (entrypoint == VAEntrypointEncSlice))) {
va_status = VA_STATUS_SUCCESS;
- else
+ } else if (!HAS_HEVC10_DECODING(i965) && !HAS_HEVC10_ENCODING(i965)) {
+ va_status = VA_STATUS_ERROR_UNSUPPORTED_PROFILE;
+ } else {
va_status = VA_STATUS_ERROR_UNSUPPORTED_ENTRYPOINT;
+ }

break;

case VAProfileVP9Profile0:
case VAProfileVP9Profile2:
- if ((HAS_VP9_DECODING_PROFILE(i965, profile)) && (entrypoint == VAEntrypointVLD))
+ if ((HAS_VP9_DECODING_PROFILE(i965, profile)) &&
+ (entrypoint == VAEntrypointVLD)) {
va_status = VA_STATUS_SUCCESS;
- else if ((HAS_VP9_ENCODING(i965)) && (entrypoint == VAEntrypointEncSlice))
+ } else if ((HAS_VP9_ENCODING(i965)) &&
+ (entrypoint == VAEntrypointEncSlice)) {
va_status = VA_STATUS_SUCCESS;
- else if ((profile == VAProfileVP9Profile0) && i965->wrapper_pdrvctx)
+ } else if (profile == VAProfileVP9Profile0 && i965->wrapper_pdrvctx) {
va_status = VA_STATUS_SUCCESS;
- else
+ } else if(!HAS_VP9_DECODING_PROFILE(i965, profile) &&
+ !HAS_VP9_ENCODING(i965) && !i965->wrapper_pdrvctx) {
+ va_status = VA_STATUS_ERROR_UNSUPPORTED_PROFILE;
+ } else {
va_status = VA_STATUS_ERROR_UNSUPPORTED_ENTRYPOINT;
+ }
+
break;

default:

--
2.5.5

Daniel Charles

2016-10-26 22:46:19 UTC

Permalink

jpege and avce config tests to check against all supported
entrypoints for a profile. UNSUPPORTED_PROFILE is expected
when no entrypoints are available for a given profile, else
expect UNSUPPORTED_ENTRYPOINT.

Signed-off-by: Daniel Charles <***@intel.com>
---
test/i965_avce_config_test.cpp | 25 ++++++++++++++++++-------
test/i965_jpege_config_test.cpp | 17 ++++++++---------
2 files changed, 26 insertions(+), 16 deletions(-)

diff --git a/test/i965_avce_config_test.cpp b/test/i965_avce_config_test.cpp
index b30abbc..a6db97d 100644
--- a/test/i965_avce_config_test.cpp
+++ b/test/i965_avce_config_test.cpp
@@ -45,8 +45,13 @@ VAStatus HasEncodeSupport()
struct i965_driver_data *i965(*env);
EXPECT_PTR(i965);

- return HAS_H264_ENCODING(i965) ? VA_STATUS_SUCCESS :
- EntrypointNotSupported();
+ if (HAS_H264_ENCODING(i965))
+ return VA_STATUS_SUCCESS;
+ else if (!HAS_H264_ENCODING(i965) && !HAS_H264_DECODING(i965)
+ && !HAS_LP_H264_ENCODING(i965))
+ return ProfileNotSupported();
+ else
+ return EntrypointNotSupported();
}

VAStatus HasLPEncodeSupport()
@@ -59,9 +64,11 @@ VAStatus HasLPEncodeSupport()

if (IS_SKL(i965->intel.device_info))
return VA_STATUS_SUCCESS;
-
- return HAS_LP_H264_ENCODING(i965) ? VA_STATUS_SUCCESS :
- EntrypointNotSupported();
+ else if (!HAS_H264_ENCODING(i965) && !HAS_H264_DECODING(i965)
+ && !HAS_LP_H264_ENCODING(i965))
+ return ProfileNotSupported();
+ else
+ return EntrypointNotSupported();
}

VAStatus HasMVCEncodeSupport()
@@ -72,8 +79,12 @@ VAStatus HasMVCEncodeSupport()
struct i965_driver_data *i965(*env);
EXPECT_PTR(i965);

- return HAS_H264_MVC_ENCODING(i965) ? VA_STATUS_SUCCESS :
- EntrypointNotSupported();
+ if (HAS_H264_MVC_ENCODING(i965))
+ return VA_STATUS_SUCCESS;
+ else if (!HAS_H264_MVC_ENCODING(i965) && !HAS_H264_MVC_DECODING(i965))
+ return ProfileNotSupported();
+ else
+ return EntrypointNotSupported();
}

static const std::vector<ConfigTestInput> inputs = {
diff --git a/test/i965_jpege_config_test.cpp b/test/i965_jpege_config_test.cpp
index 924eccb..fdf98b6 100644
--- a/test/i965_jpege_config_test.cpp
+++ b/test/i965_jpege_config_test.cpp
@@ -27,11 +27,6 @@
namespace JPEG {
namespace Encode {

-VAStatus EntrypointNotSupported()
-{
- return VA_STATUS_ERROR_UNSUPPORTED_ENTRYPOINT;
-}
-
VAStatus HasEncodeSupport()
{
I965TestEnvironment *env(I965TestEnvironment::instance());
@@ -40,14 +35,18 @@ VAStatus HasEncodeSupport()
struct i965_driver_data *i965(*env);
EXPECT_PTR(i965);

- return HAS_JPEG_ENCODING(i965) ? VA_STATUS_SUCCESS :
- EntrypointNotSupported();
+ if (HAS_JPEG_ENCODING(i965))
+ return VA_STATUS_SUCCESS;
+ else if (!HAS_JPEG_ENCODING(i965) && !HAS_JPEG_DECODING(i965))
+ return VA_STATUS_ERROR_UNSUPPORTED_PROFILE;
+ else
+ return VA_STATUS_ERROR_UNSUPPORTED_ENTRYPOINT;
}

static const std::vector<ConfigTestInput> inputs = {
{VAProfileJPEGBaseline, VAEntrypointEncPicture, &HasEncodeSupport},
- {VAProfileJPEGBaseline, VAEntrypointEncSlice, &EntrypointNotSupported},
- {VAProfileJPEGBaseline, VAEntrypointEncSliceLP, &EntrypointNotSupported},
+ {VAProfileJPEGBaseline, VAEntrypointEncSlice, &HasEncodeSupport},
+ {VAProfileJPEGBaseline, VAEntrypointEncSliceLP, &HasEncodeSupport},
};

INSTANTIATE_TEST_CASE_P(

--
2.5.5

Eoff, Ullysses A

2016-10-27 01:53:13 UTC

Permalink

-----Original Message-----
Sent: Wednesday, October 26, 2016 3:46 PM
Subject: [Libva] [PATCH 2/2 v2][libva-intel-driver] i965_test_config: return properly unsupported profile
jpege and avce config tests to check against all supported
entrypoints for a profile. UNSUPPORTED_PROFILE is expected
when no entrypoints are available for a given profile, else
expect UNSUPPORTED_ENTRYPOINT.
---
test/i965_avce_config_test.cpp | 25 ++++++++++++++++++-------
test/i965_jpege_config_test.cpp | 17 ++++++++---------
2 files changed, 26 insertions(+), 16 deletions(-)
diff --git a/test/i965_avce_config_test.cpp b/test/i965_avce_config_test.cpp
index b30abbc..a6db97d 100644
--- a/test/i965_avce_config_test.cpp
+++ b/test/i965_avce_config_test.cpp
@@ -45,8 +45,13 @@ VAStatus HasEncodeSupport()
struct i965_driver_data *i965(*env);
EXPECT_PTR(i965);
- EntrypointNotSupported();
+ if (HAS_H264_ENCODING(i965))
+ return VA_STATUS_SUCCESS;
+ else if (!HAS_H264_ENCODING(i965) && !HAS_H264_DECODING(i965)

At this point, we already know !HAS_H264_ENCODING(i965) is true because
the first condition is false if we get here.

+ && !HAS_LP_H264_ENCODING(i965))
+ return ProfileNotSupported();
+ else
+ return EntrypointNotSupported();
}
VAStatus HasLPEncodeSupport()
@@ -59,9 +64,11 @@ VAStatus HasLPEncodeSupport()
if (IS_SKL(i965->intel.device_info))
return VA_STATUS_SUCCESS;
-
- EntrypointNotSupported();
+ else if (!HAS_H264_ENCODING(i965) && !HAS_H264_DECODING(i965)
+ && !HAS_LP_H264_ENCODING(i965))
+ return ProfileNotSupported();
+ else
+ return EntrypointNotSupported();

Need to return VA_STATUS_SUCCESS when HAS_LP_H264_ENCODING(i965)
is true. Maybe it's better not to use "else if ... else" here. Perhaps...

if (IS_SKL(i965->intel.device_info))
return VA_STATUS_SUCCESS;
if (HAS_LP_H264_ENCODING(i965))
return VA_STATUS_SUCCESS;
// we already know HAS_LP_H264_ENCODING is false here
if (!HAS_H264_DECODING(i965) && !HAS_LP_H264_ENCODING(i965))
return ProfileNotSupported();
return EntrypointNoSupported();

}
VAStatus HasMVCEncodeSupport()
@@ -72,8 +79,12 @@ VAStatus HasMVCEncodeSupport()
struct i965_driver_data *i965(*env);
EXPECT_PTR(i965);
- EntrypointNotSupported();
+ if (HAS_H264_MVC_ENCODING(i965))
+ return VA_STATUS_SUCCESS;
+ else if (!HAS_H264_MVC_ENCODING(i965) && !HAS_H264_MVC_DECODING(i965))

Should this be !HAS_H264_MVC_DECODING_PROFILE(i965) instead of
!HAS_H264_MVC_DECODING(i965)? You might want to confirm.

+ return ProfileNotSupported();
+ else
+ return EntrypointNotSupported();
}
static const std::vector<ConfigTestInput> inputs = {

Shouldn't there be modifications here for the "EntrypointNotSupported"
input cases? For example,

{VAProfileH264ConstrainedBaseline, VAEntrypointEncPicture, &EntrypointNotSupported},

...could be either ProfileNotSupported or EntrypointNotSupported now because
of patch 1.

diff --git a/test/i965_jpege_config_test.cpp b/test/i965_jpege_config_test.cpp
index 924eccb..fdf98b6 100644
--- a/test/i965_jpege_config_test.cpp
+++ b/test/i965_jpege_config_test.cpp
@@ -27,11 +27,6 @@
namespace JPEG {
namespace Encode {
-VAStatus EntrypointNotSupported()
-{
- return VA_STATUS_ERROR_UNSUPPORTED_ENTRYPOINT;
-}
-
VAStatus HasEncodeSupport()
{
I965TestEnvironment *env(I965TestEnvironment::instance());
@@ -40,14 +35,18 @@ VAStatus HasEncodeSupport()
struct i965_driver_data *i965(*env);
EXPECT_PTR(i965);
- EntrypointNotSupported();
+ if (HAS_JPEG_ENCODING(i965))
+ return VA_STATUS_SUCCESS;
+ else if (!HAS_JPEG_ENCODING(i965) && !HAS_JPEG_DECODING(i965))

At this point, we already know !HAS_JPEG_ENCODING(i965) is true... so no
need to check it.

+ return VA_STATUS_ERROR_UNSUPPORTED_PROFILE;
+ else
+ return VA_STATUS_ERROR_UNSUPPORTED_ENTRYPOINT;
}
static const std::vector<ConfigTestInput> inputs = {
{VAProfileJPEGBaseline, VAEntrypointEncPicture, &HasEncodeSupport},
- {VAProfileJPEGBaseline, VAEntrypointEncSlice, &EntrypointNotSupported},
- {VAProfileJPEGBaseline, VAEntrypointEncSliceLP, &EntrypointNotSupported},
+ {VAProfileJPEGBaseline, VAEntrypointEncSlice, &HasEncodeSupport},
+ {VAProfileJPEGBaseline, VAEntrypointEncSliceLP, &HasEncodeSupport},

EncSlice and EncSlicLP are not supported on any platform for JPEG. This will result in
VA_STATUS_SUCCESS if HAS_JPEG_ENCODING == true, regardless of entrypoint.
Perhaps create a new function (e.g. NotSupported) to return either unsupported
profile or unsupported entrypoint depending on the conditions you've introduced
from the previous patch. Keep in mind, the callback functions don't tell the whole
story. It is here, where we define the inputs, which completes the "actual"
expectation we want for the profile/entrypoint combination.

};
INSTANTIATE_TEST_CASE_P(
--
2.5.5
_______________________________________________
Libva mailing list
https://lists.freedesktop.org/mailman/listinfo/libva

Eoff, Ullysses A

2016-10-27 15:34:26 UTC

Permalink

-----Original Message-----
Sent: Wednesday, October 26, 2016 6:53 PM
Subject: Re: [Libva] [PATCH 2/2 v2][libva-intel-driver] i965_test_config: return properly unsupported profile

At this point, we already know !HAS_H264_ENCODING(i965) is true because
the first condition is false if we get here.

Need to return VA_STATUS_SUCCESS when HAS_LP_H264_ENCODING(i965)
is true. Maybe it's better not to use "else if ... else" here. Perhaps...
if (IS_SKL(i965->intel.device_info))
return VA_STATUS_SUCCESS;
if (HAS_LP_H264_ENCODING(i965))
return VA_STATUS_SUCCESS;
// we already know HAS_LP_H264_ENCODING is false here
if (!HAS_H264_DECODING(i965) && !HAS_LP_H264_ENCODING(i965))
return ProfileNotSupported();

Oops, I meant the following condition here...

if (!HAS_H264_DECODING(i965) && !HAS _H264_ENCODING(i965))
return ProfileNotSupported();

return EntrypointNoSupported();

Should this be !HAS_H264_MVC_DECODING_PROFILE(i965) instead of
!HAS_H264_MVC_DECODING(i965)? You might want to confirm.

+ return ProfileNotSupported();
+ else
+ return EntrypointNotSupported();
}
static const std::vector<ConfigTestInput> inputs = {

Shouldn't there be modifications here for the "EntrypointNotSupported"
input cases? For example,
{VAProfileH264ConstrainedBaseline, VAEntrypointEncPicture, &EntrypointNotSupported},
...could be either ProfileNotSupported or EntrypointNotSupported now because
of patch 1.

At this point, we already know !HAS_JPEG_ENCODING(i965) is true... so no
need to check it.

};
INSTANTIATE_TEST_CASE_P(
--
2.5.5
_______________________________________________
Libva mailing list
https://lists.freedesktop.org/mailman/listinfo/libva

_______________________________________________
Libva mailing list
https://lists.freedesktop.org/mailman/listinfo/libva