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