On Thu, Apr 14, 2016 at 04:53:39PM +0300, Pekka Paalanen wrote: > On Wed, 13 Apr 2016 03:25:11 -0700 > Bryce Harrington <br...@osg.samsung.com> wrote: > > > refactor configuration API of headless-backend > > > > Signed-off-by: Bryce Harrington <br...@osg.samsung.com> > > Hi Bryce, > > did you write this from scratch, or did you start from Benoit's patch? > > Just asking, since I see Benoit posting v0 and v1 for headless, but > this patch does not credit him at all.
Rats, my mistake. No I did start from Benoit's patch; his credit just must have gotten lost in all the rebase madness. I'll try and figure out how to restore the attribution for v6. Thanks for catching, Bryce > > --- > > v5: > > - update to current trunk > > - fixed typo 'struct weston_wayland_backend_config' > > - dropped unused variables > > - dropped weston_headless_backend_config_create() in favor of > > directly zalloc'ing the object > > - dropped weston_headless_backend_load() in favor of the more > > generalized load_backend_new(). > > - dropped typedef from header > > - restored use of 'backend_init' entry point > > - backend_init() takes a base weston_backend_config object > > - renamed 'param' to 'config' in a few places for consistency > > - renamed 'headless_options' variable to 'options for consistency > > - version the base struct > > - free config on error > > - don't free config during backend_init normal operations > > - adjust header ordering > > - make header guard naming consistent with other headers > > - light reformatting for code style and consistency with other > > backend config patches > > > > Makefile.am | 1 + > > src/compositor-headless.c | 69 > > ++++++++++++++++++++--------------------------- > > src/compositor-headless.h | 51 +++++++++++++++++++++++++++++++++++ > > src/main.c | 45 +++++++++++++++++++++++++++++-- > > 4 files changed, 124 insertions(+), 42 deletions(-) > > create mode 100644 src/compositor-headless.h > > > > diff --git a/Makefile.am b/Makefile.am > > index fde6280..8a5e004 100644 > > --- a/Makefile.am > > +++ b/Makefile.am > > @@ -73,6 +73,7 @@ weston_SOURCES = \ > > src/compositor.c \ > > src/compositor.h \ > > src/compositor-drm.h \ > > + src/compositor-headless.h \ > > src/compositor-x11.h \ > > src/input.c \ > > src/data-device.c \ > > Like the x11 patch, missing two more places to add the header to: for > installing and the module build. > > > diff --git a/src/compositor-headless.c b/src/compositor-headless.c > > index 16b5c39..0087347 100644 > > --- a/src/compositor-headless.c > > +++ b/src/compositor-headless.c > > @@ -31,33 +31,29 @@ > > #include <sys/time.h> > > #include <stdbool.h> > > > > -#include "shared/helpers.h" > > #include "compositor.h" > > +#include "compositor-headless.h" > > +#include "shared/helpers.h" > > #include "pixman-renderer.h" > > #include "presentation-time-server-protocol.h" > > > > struct headless_backend { > > struct weston_backend base; > > struct weston_compositor *compositor; > > + > > struct weston_seat fake_seat; > > bool use_pixman; > > }; > > > > struct headless_output { > > struct weston_output base; > > + > > struct weston_mode mode; > > struct wl_event_source *finish_frame_timer; > > uint32_t *image_buf; > > pixman_image_t *image; > > }; > > > > -struct headless_parameters { > > - int width; > > - int height; > > - int use_pixman; > > - uint32_t transform; > > -}; > > - > > static void > > headless_output_start_repaint_loop(struct weston_output *output) > > { > > @@ -120,7 +116,7 @@ headless_output_destroy(struct weston_output > > *output_base) > > > > static int > > headless_backend_create_output(struct headless_backend *b, > > - struct headless_parameters *param) > > + struct weston_headless_backend_config *config) > > { > > struct weston_compositor *c = b->compositor; > > struct headless_output *output; > > @@ -132,15 +128,15 @@ headless_backend_create_output(struct > > headless_backend *b, > > > > output->mode.flags = > > WL_OUTPUT_MODE_CURRENT | WL_OUTPUT_MODE_PREFERRED; > > - output->mode.width = param->width; > > - output->mode.height = param->height; > > + output->mode.width = config->width; > > + output->mode.height = config->height; > > output->mode.refresh = 60000; > > wl_list_init(&output->base.mode_list); > > wl_list_insert(&output->base.mode_list, &output->mode.link); > > > > output->base.current_mode = &output->mode; > > - weston_output_init(&output->base, c, 0, 0, param->width, > > - param->height, param->transform, 1); > > + weston_output_init(&output->base, c, 0, 0, config->width, > > + config->height, config->transform, 1); > > > > output->base.make = "weston"; > > output->base.model = "headless"; > > @@ -158,15 +154,15 @@ headless_backend_create_output(struct > > headless_backend *b, > > output->base.switch_mode = NULL; > > > > if (b->use_pixman) { > > - output->image_buf = malloc(param->width * param->height * 4); > > + output->image_buf = malloc(config->width * config->height * 4); > > if (!output->image_buf) > > return -1; > > > > output->image = pixman_image_create_bits(PIXMAN_x8r8g8b8, > > - param->width, > > - param->height, > > + config->width, > > + config->height, > > output->image_buf, > > - param->width * 4); > > + config->width * 4); > > > > if (pixman_renderer_output_create(&output->base) < 0) > > return -1; > > @@ -217,7 +213,7 @@ headless_destroy(struct weston_compositor *ec) > > > > static struct headless_backend * > > headless_backend_create(struct weston_compositor *compositor, > > - struct headless_parameters *param, > > + struct weston_headless_backend_config *config, > > const char *display_name) > > { > > struct headless_backend *b; > > @@ -236,11 +232,11 @@ headless_backend_create(struct weston_compositor > > *compositor, > > b->base.destroy = headless_destroy; > > b->base.restore = headless_restore; > > > > - b->use_pixman = param->use_pixman; > > + b->use_pixman = config->use_pixman; > > if (b->use_pixman) { > > pixman_renderer_init(compositor); > > } > > - if (headless_backend_create_output(b, param) < 0) > > + if (headless_backend_create_output(b, config) < 0) > > goto err_input; > > > > if (!b->use_pixman && noop_renderer_init(compositor) < 0) > > @@ -260,33 +256,26 @@ err_free: > > WL_EXPORT int > > backend_init(struct weston_compositor *compositor, > > int *argc, char *argv[], > > - struct weston_config *config, > > + struct weston_config *wc, > > struct weston_backend_config *config_base) > > { > > - int width = 1024, height = 640; > > - char *display_name = NULL; > > - struct headless_parameters param = { 0, }; > > - const char *transform = "normal"; > > struct headless_backend *b; > > + struct weston_headless_backend_config *config; > > + char *display_name = NULL; > > > > - const struct weston_option headless_options[] = { > > - { WESTON_OPTION_INTEGER, "width", 0, &width }, > > - { WESTON_OPTION_INTEGER, "height", 0, &height }, > > - { WESTON_OPTION_BOOLEAN, "use-pixman", 0, ¶m.use_pixman }, > > - { WESTON_OPTION_STRING, "transform", 0, &transform }, > > - }; > > - > > - parse_options(headless_options, > > - ARRAY_LENGTH(headless_options), argc, argv); > > - > > - param.width = width; > > - param.height = height; > > + if (config_base == NULL || > > + config_base->struct_version != 1 || > > + config_base->struct_size > sizeof(struct > > weston_headless_backend_config)) > > + return -1; > > > > - if (weston_parse_transform(transform, ¶m.transform) < 0) > > - weston_log("Invalid transform \"%s\"\n", transform); > > + config = zalloc(sizeof(struct weston_headless_backend_config)); > > + if (config == NULL) > > + return -1; > > + memcpy(config, config_base, config_base->struct_size); > > > > - b = headless_backend_create(compositor, ¶m, display_name); > > + b = headless_backend_create(compositor, config, display_name); > > if (b == NULL) > > return -1; > > Missing free(config), though might as well just allocate on the stack > instead of zalloc(). > > > + > > return 0; > > } > > diff --git a/src/compositor-headless.h b/src/compositor-headless.h > > new file mode 100644 > > index 0000000..dcd7cb6 > > --- /dev/null > > +++ b/src/compositor-headless.h > > @@ -0,0 +1,51 @@ > > +/* > > + * Copyright © 2016 Benoit Gschwind > > + * > > + * Permission is hereby granted, free of charge, to any person obtaining > > + * a copy of this software and associated documentation files (the > > + * "Software"), to deal in the Software without restriction, including > > + * without limitation the rights to use, copy, modify, merge, publish, > > + * distribute, sublicense, and/or sell copies of the Software, and to > > + * permit persons to whom the Software is furnished to do so, subject to > > + * the following conditions: > > + * > > + * The above copyright notice and this permission notice (including the > > + * next paragraph) shall be included in all copies or substantial > > + * portions of the Software. > > + * > > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, > > + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF > > + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND > > + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS > > + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN > > + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN > > + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE > > + * SOFTWARE. > > + */ > > + > > +#ifndef _WESTON_COMPOSITOR_HEADLESS_H > > +#define _WESTON_COMPOSITOR_HEADLESS_H > > Ah, Quentin gave me the reference I was wishing for: > https://www.securecoding.cert.org/confluence/display/c/DCL37-C.+Do+not+declare+or+define+a+reserved+identifier#DCL37-C.Donotdeclareordefineareservedidentifier-NoncompliantCodeExample%28HeaderGuard%29 > > Let's not add any more reserved macro names. That is, let's drop the > leading underscore. > > > + > > +#ifdef __cplusplus > > +extern "C" { > > +#endif > > + > > +#include "compositor.h" > > + > > +struct weston_headless_backend_config { > > + struct weston_backend_config base; > > + > > + int width; > > + int height; > > + > > + /** Whether to use the pixman renderer instead of the OpenGL ES > > renderer. */ > > + int use_pixman; > > + > > + uint32_t transform; > > +}; > > + > > +#ifdef __cplusplus > > +} > > +#endif > > + > > +#endif /* _WESTON_COMPOSITOR_HEADLESS_H */ > > diff --git a/src/main.c b/src/main.c > > index a72a9d7..334464b 100644 > > --- a/src/main.c > > +++ b/src/main.c > > @@ -48,6 +48,7 @@ > > #include "version.h" > > > > #include "compositor-drm.h" > > +#include "compositor-headless.h" > > #include "compositor-x11.h" > > > > static struct wl_list child_process_list; > > @@ -765,6 +766,46 @@ load_drm_backend(struct weston_compositor *c, const > > char *backend, > > } > > > > static int > > +load_headless_backend(struct weston_compositor *c, char const * backend, > > + int *argc, char **argv, struct weston_config *wc) > > +{ > > + struct weston_headless_backend_config *config; > > + int ret = 0; > > + const char *transform = "normal"; > > + > > + config = zalloc(sizeof(struct weston_headless_backend_config)); > > + if (config == NULL) > > + return -1; > > + > > + config->width = 1024; > > + config->height = 640; > > + > > + const struct weston_option options[] = { > > + { WESTON_OPTION_INTEGER, "width", 0, &config->width }, > > + { WESTON_OPTION_INTEGER, "height", 0, &config->height }, > > + { WESTON_OPTION_BOOLEAN, "use-pixman", 0, &config->use_pixman }, > > + { WESTON_OPTION_STRING, "transform", 0, &transform }, > > + }; > > + > > + parse_options(options, ARRAY_LENGTH(options), argc, argv); > > + > > + if (weston_parse_transform(transform, &config->transform) < 0) > > + weston_log("Invalid transform \"%s\"\n", transform); > > + > > + config->base.struct_version = 1; > > + config->base.struct_size = sizeof(struct > > weston_headless_backend_config); > > + > > + /* load the actual wayland backend and configure it */ > > + if (load_backend_new(c, backend, > > + (struct weston_backend_config *)config) < 0) { > > + ret = -1; > > + free(config); > > + } > > I think free(config) should be unconditional. Though, could just as > well put it on stack instead of zalloc(). > > > + > > + return ret; > > +} > > + > > +static int > > weston_x11_backend_config_append_output_config(struct > > weston_x11_backend_config *config, > > struct > > weston_x11_backend_output_config *output_config) { > > struct weston_x11_backend_output_config *new_outputs; > > @@ -906,6 +947,8 @@ load_backend(struct weston_compositor *compositor, > > const char *backend, > > { > > if (strstr(backend, "drm-backend.so")) > > return load_drm_backend(compositor, backend, argc, argv, > > config); > > + else if (strstr(backend, "headless-backend.so")) > > + return load_headless_backend(compositor, backend, argc, argv, > > config); > > else if (strstr(backend, "x11-backend.so")) > > return load_x11_backend(compositor, backend, argc, argv, > > config); > > #if 0 > > @@ -913,8 +956,6 @@ load_backend(struct weston_compositor *compositor, > > const char *backend, > > return load_wayland_backend(compositor, backend, argc, argv, > > config); > > else if (strstr(backend, "fbdev-backend.so")) > > return load_fbdev_backend(compositor, backend, argc, argv, > > config); > > - else if (strstr(backend, "headless-backend.so")) > > - return load_headless_backend(compositor, backend, argc, argv, > > config); > > else if (strstr(backend, "rpi-backend.so")) > > return load_rpi_backend(compositor, backend, argc, argv, > > config); > > else if (strstr(backend, "rdp-backend.so")) > > > With all the issues above fixed and credits re-checked: > Reviewed-by: Pekka Paalanen <pekka.paala...@collabora.co.uk> > > > Thanks, > pq > -----BEGIN PGP SIGNATURE----- > Version: GnuPG v2 > > iQIVAwUBVw+g6iNf5bQRqqqnAQid8hAAkIqtUWCuUQgbYdTSX66jJrbj2MOfZvYV > zKfncSqpc01AW5Z/awQpCnQT9aYFngP7WaQN5aTFEggP4pl3CohQKGQF7NK9BiC+ > 2nQaUwJUKkTcpgOYmYPZLOGo6lt4u/mjdg+adUwqdCQxgP91wLpncop1bIPWxGQl > I3eENLJ/7LH5/Wrdf8DcI8y1VbyMcMjaEvBRZPDvCjhYBOYvgNKb+OwwCfVnNF+H > HTMOoEkFKoZTIIDkXsw4ZeYpCPF+ugP+GVGL4hBK4DWg9ZvdBF0jSWJ/iMT9vNzm > BAMn9NI2E5nqAMAW7MhrQiUfP4eRvaMxk6CKTma12l00UWjud09LSlbGahDHvbCx > wqHXpTKnWK6loED0Uk4BBQMljaP5tCmjfkkNE508JDaHmggC0LoaHFqcOVNRo2op > H5P0PX0AJ7kw1/uWjA1pkBbLGwrb2balQaEbpOw3WgbBa+L03+2JdkvRRrIxzTML > EL31Rd9jbKFAGUSeUChFf2FDlwJwTjdFyJstuDAGV1GyZvDd07eACq0k1CXGa+Ji > liEJLV9WcUQpX8EL9ucZ3lToqGeNWWwPn8MLeFc7Ee2zUgPZswWFXnUBVxIPf3Y+ > CnQg+JZgwCUzoEP363Q6w7jVDFev5FmcxScyJ0w+6wYHEhD+oA6Ola5Xg16Fwzr0 > PIu627PB8S8= > =vOGU > -----END PGP SIGNATURE----- _______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel