Re: [Piglit] [RFC 1/3] framework: introduce interface for external buffers
On 02/26/2013 05:15 AM, Topi Pohjolainen wrote:
> In order to test the OES_EGL_image_external with planar formats such
> as YUV420 or NV12, one needs a way for creating buffers that can be
> passed to EGL and filling them with YUV-data for the GL-stack to
> sample.
> By the nature the extension in question deals with platform specific
> buffers and hence the idea here is to push the details down into the
> platform specific logic leaving the tests themselves platform
> independent.
>
> Signed-off-by: Topi Pohjolainen
> ---
> tests/util/piglit-framework-gl.c | 29
>
> tests/util/piglit-framework-gl.h | 21 ++
> .../util/piglit-framework-gl/piglit_gl_framework.h | 23
> 3 files changed, 73 insertions(+)
> +
> +void *
> +piglit_create_ext_420_buf(unsigned w, unsigned h, bool swap_vu,
> + const void *y, const void *u, const void *v)
> +{
> + if (!gl_fw->create_external_420_buffer)
> + return NULL;
> +
> + return gl_fw->create_external_420_buffer(gl_fw, w, h, swap_vu, y, u, v);
> +}
Later, in patch 3, you write the test so that, if piglit_create_ext_420_buf()
fails then the test reports PIGLIT_FAIL. But the convention in piglit is to
report
PIGLIT_SKIP when the platform is incapable of running the test.
There are two choices to handle this:
1. Here, in piglit_create_ext_420_buf(), report PIGLIT_SKIP if it fails
to create the buffer.
2. In the testfile itself, report PIGLIT_SKIP if piglit_create_ext_420()
fails.
> +
> +void *
> +piglit_create_ext_nv12_buf(unsigned w, unsigned h, const void *y,
> + const void *uv)
> +{
> + if (!gl_fw->create_external_nv12_buffer)
> + return NULL;
> +
> + return gl_fw->create_external_nv12_buffer(gl_fw, w, h, y, uv);
> +}
Same comments for nv12.
> +/**
> + * Create YUV420 or YV12 (YVU420) formatted buffer of the given size and
> + * set it contents according to the given planes. Here there is no
> + * padding in the planes y-plane having stride equaling the given width
> + * and u/v-planes having stride equaling half of the width. By setting
> + * the 'swap_vu' to true, the order of u/v planes is swapped
> + * corresponding to YV12 (YVU420) format.
> + */
> +void *piglit_create_ext_420_buf(unsigned w, unsigned h, bool swap_vu,
> + const void *y, const void *u, const void *v);
> +
> +/**
> + * Create NV12 formatted buffer of the given size and set it contents
> + * according to the given planes. Here there is no padding in the planes
> + * both y-plane and uv-plane having stride equaling the given width.
> + */
> +void *piglit_create_ext_nv12_buf(unsigned w, unsigned h,
> + const void *y, const void *uv);
> +
> +void piglit_destroy_ext_buf(void *buf);
For these three functions, please comment that "ext" is short for "external".
That confused me.
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [RFC 1/3] framework: introduce interface for external buffers
On 03/01/2013 10:57 AM, Chad Versace wrote:
> On 02/26/2013 05:15 AM, Topi Pohjolainen wrote:
>> In order to test the OES_EGL_image_external with planar formats such
>> as YUV420 or NV12, one needs a way for creating buffers that can be
>> passed to EGL and filling them with YUV-data for the GL-stack to
>> sample.
>> By the nature the extension in question deals with platform specific
>> buffers and hence the idea here is to push the details down into the
>> platform specific logic leaving the tests themselves platform
>> independent.
>>
>> Signed-off-by: Topi Pohjolainen
>> ---
>> tests/util/piglit-framework-gl.c | 29
>>
>> tests/util/piglit-framework-gl.h | 21 ++
>> .../util/piglit-framework-gl/piglit_gl_framework.h | 23
>> 3 files changed, 73 insertions(+)
>> +void *
>> +piglit_create_ext_420_buf(unsigned w, unsigned h, bool swap_vu,
>> +const void *y, const void *u, const void *v)
>> +{
>> +if (!gl_fw->create_external_420_buffer)
>> +return NULL;
>> +
>> +return gl_fw->create_external_420_buffer(gl_fw, w, h, swap_vu, y, u, v);
>> +}
>
> Later, in patch 3, you write the test so that, if piglit_create_ext_420_buf()
> fails then the test reports PIGLIT_FAIL. But the convention in piglit is to
> report
> PIGLIT_SKIP when the platform is incapable of running the test.
>
> There are two choices to handle this:
> 1. Here, in piglit_create_ext_420_buf(), report PIGLIT_SKIP if it fails
> to create the buffer.
>
> 2. In the testfile itself, report PIGLIT_SKIP if piglit_create_ext_420()
> fails.
My opinion here has changed after I thought about it more. I think that the
piglit_gbm_framework_create() should set gl_fw->create_external_420_buffer
only if the platform is capable of creating a yuv420 buffer.
If a test calls piglit_create_ext_420_buf(), but the function is unable to
create
the buffer because the platform doesn't support it (that is, because
gl_fw->create_external_420_buffer is NULL), then the test should report
PIGLIT_SKIP.
That is the convention in piglit; if a test attempts to test something that the
platform is incapable of doing, then it reports PIGLIT_SKIP. In this case,
the best place to report PIGLIT_SKIP is here inside piglit_create_ext_420_buf().
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [RFC 1/3] framework: introduce interface for external buffers
On Fri, Mar 01, 2013 at 11:25:50AM -0800, Chad Versace wrote:
> On 03/01/2013 10:57 AM, Chad Versace wrote:
> > On 02/26/2013 05:15 AM, Topi Pohjolainen wrote:
> >> In order to test the OES_EGL_image_external with planar formats such
> >> as YUV420 or NV12, one needs a way for creating buffers that can be
> >> passed to EGL and filling them with YUV-data for the GL-stack to
> >> sample.
> >> By the nature the extension in question deals with platform specific
> >> buffers and hence the idea here is to push the details down into the
> >> platform specific logic leaving the tests themselves platform
> >> independent.
> >>
> >> Signed-off-by: Topi Pohjolainen
> >> ---
> >> tests/util/piglit-framework-gl.c | 29
> >>
> >> tests/util/piglit-framework-gl.h | 21 ++
> >> .../util/piglit-framework-gl/piglit_gl_framework.h | 23
> >> 3 files changed, 73 insertions(+)
>
>
> >> +void *
> >> +piglit_create_ext_420_buf(unsigned w, unsigned h, bool swap_vu,
> >> + const void *y, const void *u, const void *v)
> >> +{
> >> + if (!gl_fw->create_external_420_buffer)
> >> + return NULL;
> >> +
> >> + return gl_fw->create_external_420_buffer(gl_fw, w, h, swap_vu, y, u, v);
> >> +}
> >
> > Later, in patch 3, you write the test so that, if
> > piglit_create_ext_420_buf()
> > fails then the test reports PIGLIT_FAIL. But the convention in piglit is to
> > report
> > PIGLIT_SKIP when the platform is incapable of running the test.
> >
> > There are two choices to handle this:
> > 1. Here, in piglit_create_ext_420_buf(), report PIGLIT_SKIP if it fails
> > to create the buffer.
> >
> > 2. In the testfile itself, report PIGLIT_SKIP if piglit_create_ext_420()
> > fails.
>
>
> My opinion here has changed after I thought about it more. I think that the
> piglit_gbm_framework_create() should set gl_fw->create_external_420_buffer
> only if the platform is capable of creating a yuv420 buffer.
>
> If a test calls piglit_create_ext_420_buf(), but the function is unable to
> create
> the buffer because the platform doesn't support it (that is, because
> gl_fw->create_external_420_buffer is NULL), then the test should report
> PIGLIT_SKIP.
> That is the convention in piglit; if a test attempts to test something that
> the
> platform is incapable of doing, then it reports PIGLIT_SKIP. In this case,
> the best place to report PIGLIT_SKIP is here inside
> piglit_create_ext_420_buf().
Thanks Chad, I'll change it accordingly.
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit
