> -----Original Message-----
> From: Libva [mailto:libva-boun...@lists.freedesktop.org] On Behalf Of Eoff,
> Ullysses A
> Sent: Wednesday, October 26, 2016 6:53 PM
> To: Charles, Daniel <daniel.char...@intel.com>; libva@lists.freedesktop.org
> Subject: Re: [Libva] [PATCH 2/2 v2][libva-intel-driver] i965_test_config:
> return properly unsupported profile
>
> > -----Original Message-----
> > From: Libva [mailto:libva-boun...@lists.freedesktop.org] On Behalf Of
> > Daniel Charles
> > Sent: Wednesday, October 26, 2016 3:46 PM
> > To: libva@lists.freedesktop.org
> > 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.
> >
> > Signed-off-by: Daniel Charles <daniel.char...@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)
>
> 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;
> > -
> > - 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();
>
> 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();
>
> > }
> >
> > 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))
>
> 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);
> >
> > - 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))
>
> 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
> > Libva@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/libva
> _______________________________________________
> Libva mailing list
> Libva@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/libva
_______________________________________________
Libva mailing list
Libva@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libva
