Commit 07952847 authored by runge's avatar runge

x11vnc: add uinput support for full input into linux fb device (e.g. qt-embed).

parent 8cda6096
2006-07-08 Karl Runge <runge@karlrunge.com>
* configure.ac: add <linux/uinput.h> for linux console.
2006-07-04 Karl Runge <runge@karlrunge.com>
* configure.ac: add getspnam.
......
......@@ -62,6 +62,8 @@ AH_TEMPLATE(HAVE_IRIX_XREADDISPLAY, [IRIX XReadDisplay available])
AH_TEMPLATE(HAVE_FBPM, [FBPM extension build environment present])
AH_TEMPLATE(HAVE_LINUX_VIDEODEV_H, [video4linux build environment present])
AH_TEMPLATE(HAVE_LINUX_FB_H, [linux fb device build environment present])
AH_TEMPLATE(HAVE_LINUX_INPUT_H, [linux/input.h present])
AH_TEMPLATE(HAVE_LINUX_UINPUT_H, [linux uinput device build environment present])
AC_ARG_WITH(xkeyboard,
[ --without-xkeyboard disable xkeyboard extension support],,)
......@@ -83,6 +85,8 @@ AC_ARG_WITH(v4l,
[ --without-v4l disable video4linux support],,)
AC_ARG_WITH(fbdev,
[ --without-fbdev disable linux fb device support],,)
AC_ARG_WITH(uinput,
[ --without-uinput disable linux uinput device support],,)
if test "$X_CFLAGS" != "-DX_DISPLAY_MISSING"; then
AC_CHECK_LIB(X11, XGetImage, HAVE_X="true",
......@@ -237,6 +241,14 @@ if test "$X_CFLAGS" != "-DX_DISPLAY_MISSING"; then
AC_CHECK_HEADER(linux/fb.h,
[AC_DEFINE(HAVE_LINUX_FB_H)],,)
fi
if test "x$with_uinput" != "xno"; then
AC_CHECK_HEADER(linux/input.h,
[AC_DEFINE(HAVE_LINUX_INPUT_H) HAVE_LINUX_INPUT_H="true"],,)
if test "x$HAVE_LINUX_INPUT_H" = "xtrue"; then
AC_CHECK_HEADER(linux/uinput.h,
[AC_DEFINE(HAVE_LINUX_UINPUT_H)],, [#include <linux/input.h>])
fi
fi
X_LIBS="$X_LIBS $X_PRELIBS -lX11 $X_EXTRA_LIBS"
fi
......
2006-07-08 Karl Runge <runge@karlrunge.com>
* x11vnc: add uinput support (-pipeinput UINPUT:...) for full
mouse and key input to linux console (e.g. for qt-embedded apps)
add -allinput for handleEventsEagerly.
2006-07-04 Karl Runge <runge@karlrunge.com>
* x11vnc: 2nd -accept popup with WAIT, and UNIX: info for unixpw
login. Use RFB_CLIENT_ON_HOLD for -unixpw. -unixpw white arrow
......
......@@ -13,7 +13,7 @@ endif
if HAVE_X
bin_PROGRAMS=x11vnc
x11vnc_SOURCES = 8to24.c cleanup.c connections.c cursor.c gui.c help.c inet.c keyboard.c linuxfb.c options.c pm.c pointer.c rates.c remote.c scan.c screen.c selection.c solid.c sslcmds.c sslhelper.c unixpw.c user.c userinput.c util.c v4l.c win_utils.c x11vnc.c x11vnc_defs.c xdamage.c xevents.c xinerama.c xkb_bell.c xrandr.c xrecord.c xwrappers.c 8to24.h allowed_input_t.h blackout_t.h cleanup.h connections.h cursor.h enums.h gui.h help.h inet.h keyboard.h linuxfb.h options.h params.h pm.h pointer.h rates.h remote.h scan.h screen.h scrollevent_t.h selection.h solid.h sslcmds.h sslhelper.h ssltools.h tkx11vnc.h unixpw.h user.h userinput.h util.h v4l.h win_utils.h winattr_t.h x11vnc.h xdamage.h xevents.h xinerama.h xkb_bell.h xrandr.h xrecord.h xwrappers.h
x11vnc_SOURCES = 8to24.c cleanup.c connections.c cursor.c gui.c help.c inet.c keyboard.c linuxfb.c options.c pm.c pointer.c rates.c remote.c scan.c screen.c selection.c solid.c sslcmds.c sslhelper.c uinput.c unixpw.c user.c userinput.c util.c v4l.c win_utils.c x11vnc.c x11vnc_defs.c xdamage.c xevents.c xinerama.c xkb_bell.c xrandr.c xrecord.c xwrappers.c 8to24.h allowed_input_t.h blackout_t.h cleanup.h connections.h cursor.h enums.h gui.h help.h inet.h keyboard.h linuxfb.h options.h params.h pm.h pointer.h rates.h remote.h scan.h screen.h scrollevent_t.h selection.h solid.h sslcmds.h sslhelper.h ssltools.h tkx11vnc.h uinput.h unixpw.h user.h userinput.h util.h v4l.h win_utils.h winattr_t.h x11vnc.h xdamage.h xevents.h xinerama.h xkb_bell.h xrandr.h xrecord.h xwrappers.h
INCLUDES=@X_CFLAGS@
x11vnc_LDADD=$(LDADD) @X_LIBS@ $(LD_CYGIPC)
endif
......
This diff is collapsed.
This diff is collapsed.
......@@ -2461,6 +2461,7 @@ void get_allowed_input(rfbClientPtr client, allowed_input_t *input) {
str = "KMBC";
}
}
if (0) fprintf(stderr, "GAI: %s - %s\n", str, cd->input);
while (*str) {
if (*str == 'K') {
......@@ -2485,9 +2486,10 @@ static void pipe_keyboard(rfbBool down, rfbKeySym keysym, rfbClientPtr client) {
if (pipeinput_int == PIPEINPUT_VID) {
v4l_key_command(down, keysym, client);
}
if (pipeinput_int == PIPEINPUT_CONS) {
} else if (pipeinput_int == PIPEINPUT_CONS) {
console_key_command(down, keysym, client);
} else if (pipeinput_int == PIPEINPUT_UINPUT) {
uinput_key_command(down, keysym, client);
}
if (pipeinput_fh == NULL) {
return;
......@@ -2495,7 +2497,7 @@ static void pipe_keyboard(rfbBool down, rfbKeySym keysym, rfbClientPtr client) {
if (! view_only) {
get_allowed_input(client, &input);
if (input.motion || input.button) {
if (input.keystroke) {
can_input = 1; /* XXX distinguish later */
}
}
......
......@@ -6,6 +6,7 @@
#include "xinerama.h"
#include "screen.h"
#include "pointer.h"
#include "allowed_input_t.h"
#if LIBVNCSERVER_HAVE_SYS_IOCTL_H
#include <sys/ioctl.h>
......@@ -21,7 +22,8 @@ void console_pointer_command(int mask, int x, int y, rfbClientPtr client);
char *console_guess(char *str, int *fd) {
char *q, *in = strdup(str);
char *atparms = NULL, *file = NULL;
int tty = -1;
int do_input, have_uinput, tty = -1;
if (strstr(in, "/dev/fb") == in) {
free(in);
in = (char *) malloc(strlen("cons:") + strlen(str) + 1);
......@@ -67,22 +69,43 @@ char *console_guess(char *str, int *fd) {
}
rfbLog("console_guess: file is %s\n", file);
if (!strcmp(in, "cons") || !strcmp(in, "console")) {
do_input = 1;
if (pipeinput_str) {
have_uinput = 0;
do_input = 0;
} else {
have_uinput = check_uinput();
}
if (!strcmp(in, "consx") || !strcmp(in, "consolex")) {
do_input = 0;
} else if (!strcmp(in, "cons") || !strcmp(in, "console")) {
/* current active VT: */
tty = 0;
if (! have_uinput) {
tty = 0;
}
} else {
int n;
if (sscanf(in, "cons%d", &n) == 1) {
tty = n;
have_uinput = 0;
} else if (sscanf(in, "console%d", &n) != 1) {
tty = n;
have_uinput = 0;
}
}
if (tty >=0 && tty < 64) {
if (pipeinput_str == NULL) {
if (do_input) {
if (tty >=0 && tty < 64) {
pipeinput_str = (char *) malloc(10);
sprintf(pipeinput_str, "CONS%d", tty);
rfbLog("console_guess: file pipeinput %s\n", pipeinput_str);
rfbLog("console_guess: file pipeinput %s\n",
pipeinput_str);
initialize_pipeinput();
} else if (have_uinput) {
pipeinput_str = strdup("UINPUT");
rfbLog("console_guess: file pipeinput %s\n",
pipeinput_str);
initialize_pipeinput();
}
}
......@@ -143,10 +166,20 @@ char *console_guess(char *str, int *fd) {
void console_key_command(rfbBool down, rfbKeySym keysym, rfbClientPtr client) {
static int control = 0, alt = 0;
allowed_input_t input;
if (debug_keyboard) fprintf(stderr, "console_key_command: %d %s\n", (int) keysym, down ? "down" : "up");
if (pipeinput_cons_fd < 0) {
return;
}
if (view_only) {
return;
}
get_allowed_input(client, &input);
if (! input.keystroke) {
return;
}
/* From LinuxVNC.c: */
if (keysym == XK_Control_L || keysym == XK_Control_R) {
......@@ -250,6 +283,7 @@ void console_key_command(rfbBool down, rfbKeySym keysym, rfbClientPtr client) {
}
void console_pointer_command(int mask, int x, int y, rfbClientPtr client) {
/* do not forget viewonly perms */
if (mask || x || y || client) {}
}
......@@ -290,6 +290,7 @@ int naptile = 4; /* tile change threshold per poll to take a nap */
int napfac = 4; /* time = napfac*waitms, cut load with extra waits */
int napmax = 1500; /* longest nap in ms. */
int ui_skip = 10; /* see watchloop. negative means ignore input */
int all_input = 0;
#if LIBVNCSERVER_HAVE_FBPM
......
......@@ -219,6 +219,7 @@ extern int naptile;
extern int napfac;
extern int napmax;
extern int ui_skip;
extern int all_input;
extern int watch_fbpm;
......
......@@ -35,8 +35,11 @@
#define MAXN 256
#define PIPEINPUT_NONE 0x0
#define PIPEINPUT_VID 0x1
#define PIPEINPUT_CONS 0x2
#define PIPEINPUT_NONE 0x0
#define PIPEINPUT_VID 0x1
#define PIPEINPUT_CONS 0x2
#define PIPEINPUT_UINPUT 0x3
#define MAX_BUTTONS 5
#endif /* _X11VNC_PARAMS_H */
......@@ -43,7 +43,6 @@ typedef struct ptrremap {
#ifdef LIBVNCSERVER_HAVE_LIBPTHREAD
MUTEX(pointerMutex);
#endif
#define MAX_BUTTONS 5
#define MAX_BUTTON_EVENTS 50
static prtremap_t pointer_map[MAX_BUTTONS+1][MAX_BUTTON_EVENTS];
......@@ -522,9 +521,10 @@ static void pipe_pointer(int mask, int x, int y, rfbClientPtr client) {
if (pipeinput_int == PIPEINPUT_VID) {
v4l_pointer_command(mask, x, y, client);
}
if (pipeinput_int == PIPEINPUT_CONS) {
} else if (pipeinput_int == PIPEINPUT_CONS) {
console_pointer_command(mask, x, y, client);
} else if (pipeinput_int == PIPEINPUT_UINPUT) {
uinput_pointer_command(mask, x, y, client);
}
if (pipeinput_fh == NULL) {
return;
......@@ -536,6 +536,7 @@ static void pipe_pointer(int mask, int x, int y, rfbClientPtr client) {
can_input = 1; /* XXX distinguish later */
}
}
if (cd) {
uid = cd->uid;
}
......@@ -594,6 +595,7 @@ void pointer(int mask, int x, int y, rfbClientPtr client) {
if (mask >= 0) {
got_pointer_calls++;
}
get_allowed_input(client, &input);
if (debug_pointer && mask >= 0) {
static int show_motion = -1;
......@@ -631,7 +633,7 @@ void pointer(int mask, int x, int y, rfbClientPtr client) {
y = nfix(y, dpy_y);
}
if (pipeinput_fh != NULL && mask >= 0) {
if ((pipeinput_fh != NULL || pipeinput_int) && mask >= 0) {
pipe_pointer(mask, x, y, client);
if (! pipeinput_tee) {
if (! view_only || raw_fb) { /* raw_fb hack */
......@@ -639,7 +641,7 @@ void pointer(int mask, int x, int y, rfbClientPtr client) {
got_keyboard_input++;
last_pointer_client = client;
}
if (view_only && raw_fb) {
if (input.motion) {
/* raw_fb hack track button state */
button_mask_prev = button_mask;
button_mask = mask;
......@@ -659,7 +661,6 @@ void pointer(int mask, int x, int y, rfbClientPtr client) {
* mask = -1 is a special case call from scan_for_updates()
* to flush the event queue; there is no real pointer event.
*/
get_allowed_input(client, &input);
if (! input.motion && ! input.button) {
return;
}
......@@ -863,7 +864,7 @@ void pointer(int mask, int x, int y, rfbClientPtr client) {
}
void initialize_pipeinput(void) {
char *p;
char *p = NULL;
if (pipeinput_fh != NULL) {
rfbLog("closing pipeinput stream: %p\n", pipeinput_fh);
......@@ -882,7 +883,11 @@ void initialize_pipeinput(void) {
}
/* look for options: tee, reopen, ... */
p = strchr(pipeinput_str, ':');
if (strstr(pipeinput_str, "UINPUT") == pipeinput_str) {
;
} else {
p = strchr(pipeinput_str, ':');
}
if (p != NULL) {
char *str, *opt, *q;
int got = 0;
......@@ -919,8 +924,7 @@ if (0) fprintf(stderr, "initialize_pipeinput: %s -- %s\n", pipeinput_str, p);
if (!strcmp(p, "VID")) {
pipeinput_int = PIPEINPUT_VID;
return;
}
if (strstr(p, "CONS") == p) {
} else if (strstr(p, "CONS") == p) {
int tty = 0, n;
char dev[32];
if (sscanf(p, "CONS%d", &n) == 1) {
......@@ -940,6 +944,14 @@ if (0) fprintf(stderr, "initialize_pipeinput: %s -- %s\n", pipeinput_str, p);
rfbLogPerror("open");
}
return;
} else if (strstr(p, "UINPUT") == p) {
char *q = strchr(p, ':');
if (q) {
parse_uinput_str(q+1);
}
pipeinput_int = PIPEINPUT_UINPUT;
initialize_uinput();
return;
}
set_child_info();
......@@ -990,16 +1002,16 @@ if (0) fprintf(stderr, "initialize_pipeinput: %s -- %s\n", pipeinput_str, p);
"# It can be:\n"
"#\n"
"# None (nothing to report)\n"
"# ButtonPress-N (this event will cause button-1 to be pressed) \n"
"# ButtonRelease-N (this event will cause button-1 to be released) \n"
"# ButtonPress-N (this event will cause button-N to be pressed) \n"
"# ButtonRelease-N (this event will cause button-N to be released) \n"
"#\n"
"# if two more more buttons change state in one event they are listed\n"
"# separated by commas.\n"
"#\n"
"# One might parse a Pointer line with:\n"
"#\n"
"# int client, x, y, mask; char *hint;\n"
"# sscanf(line, \"Pointer %d %d %d %s\", &client, &x, &y, &mask, &hint);\n"
"# int client, x, y, mask; char hint[100];\n"
"# sscanf(line, \"Pointer %d %d %d %d %s\", &client, &x, &y, &mask, hint);\n"
"#\n"
"#\n"
"# Keysym events (keyboard presses and releases) come in the form:\n"
......@@ -1023,8 +1035,8 @@ if (0) fprintf(stderr, "initialize_pipeinput: %s -- %s\n", pipeinput_str, p);
"#\n"
"# One might parse a Keysym line with:\n"
"#\n"
"# int client, down, keysym; char *name, *hint;\n"
"# sscanf(line, \"Keysym %d %d %s %s\", &client, &down, &keysym, &name, &hint);\n"
"# int client, down, keysym; char name[100], hint[100];\n"
"# sscanf(line, \"Keysym %d %d %d %s %s\", &client, &down, &keysym, name, hint);\n"
"#\n"
"# The <hint> value is currently just None, KeyPress, or KeyRelease.\n"
"#\n"
......@@ -1041,6 +1053,20 @@ if (0) fprintf(stderr, "initialize_pipeinput: %s -- %s\n", pipeinput_str, p);
"# info will be enough for most purposes (having identical keyboards on\n"
"# both sides helps).\n"
"#\n"
"# Parsing example for perl:\n"
"#\n"
"# while (<>) {\n"
"# chomp;\n"
"# if (/^Pointer/) {\n"
"# my ($p, $client, $x, $y, $mask, $hint) = split(' ', $_, 6);\n"
"# do_pointer($client, $x, $y, $mask, $hint);\n"
"# } elsif (/^Keysym/) {\n"
"# my ($k, $client, $down, $keysym, $name, $hint) = split(' ', $_, 6);\n"
"# do_keysym($client, $down, $keysym, $name, $hint);\n"
"# }\n"
"# }\n"
"#\n"
"#\n"
"# Here comes your stream. The following token will always indicate the\n"
"# end of this informational text:\n"
"# END_OF_TOP\n"
......
......@@ -24,6 +24,7 @@
#include "keyboard.h"
#include "selection.h"
#include "unixpw.h"
#include "uinput.h"
int send_remote_cmd(char *cmd, int query, int wait);
int do_remote_query(char *remote_cmd, char *query_cmd, int remote_sync,
......@@ -3029,6 +3030,21 @@ char *process_remote_cmd(char *cmd, int stringonly) {
rfbLog("remote_cmd: setting input_skip %d\n", is);
ui_skip = is;
} else if (!strcmp(p, "allinput")) {
if (query) {
snprintf(buf, bufn, "ans=%s:%d", p, all_input);
goto qry;
}
all_input = 1;
rfbLog("enabled allinput\n");
} else if (!strcmp(p, "noallinput")) {
if (query) {
snprintf(buf, bufn, "ans=%s:%d", p, !all_input);
goto qry;
}
all_input = 0;
rfbLog("disabled allinput\n");
} else if (strstr(p, "input") == p) {
int doit = 1;
COLON_CHECK("input:")
......@@ -3436,6 +3452,28 @@ char *process_remote_cmd(char *cmd, int stringonly) {
do_new_fb(1);
raw_fb_back_to_X = 0;
} else if (strstr(p, "uinput_accel") == p) {
COLON_CHECK("uinput_accel:")
if (query) {
snprintf(buf, bufn, "ans=%s%s%s", p, co,
NONUL(get_uinput_accel()));
goto qry;
}
p += strlen("uinput_accel:");
rfbLog("set_uinput_accel: %s\n", p);
set_uinput_accel(p);
} else if (strstr(p, "uinput_reset") == p) {
COLON_CHECK("uinput_reset:")
p += strlen("uinput_reset:");
if (query) {
snprintf(buf, bufn, "ans=%s%s%d", p, co,
get_uinput_reset());
goto qry;
}
rfbLog("set_uinput_reset: %s\n", p);
set_uinput_reset(atoi(p));
} else if (strstr(p, "progressive") == p) {
int f;
COLON_CHECK("progressive:")
......
......@@ -574,10 +574,12 @@ void set_raw_fb_params(int restore) {
}
} else {
/* Normal case: */
if (! view_only) {
#if 0
if (! view_only && ! pipeinput_str) {
if (! quiet) rfbLog(" rawfb: setting view_only\n");
view_only = 1;
}
#endif
if (watch_selection) {
if (! quiet) rfbLog(" rawfb: turning off "
"watch_selection\n");
......@@ -609,10 +611,6 @@ void set_raw_fb_params(int restore) {
}
multiple_cursors_mode = strdup("arrow");
}
if (0 && use_snapfb) {
if (! quiet) rfbLog(" rawfb: turning off use_snapfb\n");
use_snapfb = 0;
}
if (using_shm) {
if (! quiet) rfbLog(" rawfb: turning off using_shm\n");
using_shm = 0;
......@@ -963,7 +961,8 @@ if (db) fprintf(stderr, "initialize_raw_fb reset\n");
rawfb_dev_video = 1;
} else if (strstr(str, "dev/video")) {
rawfb_dev_video = 1;
} else if (strstr(str, "cons") == str || strstr(str, "/dev/fb") == str) {
} else if (strstr(str, "cons") == str || strstr(str, "fb") == str ||
strstr(str, "/dev/fb") == str) {
char *str2 = console_guess(str, &raw_fb_fd);
if (str2 == NULL) {
rfbLog("console_guess failed for: %s\n", str);
......
......@@ -178,6 +178,8 @@ Screen
visual:
rawfb:
pipeinput:
uinput_accel:
uinput_reset:
24to32
=GAL LOFF
......@@ -317,6 +319,7 @@ Permissions
Tuning
=D-C:0,1,2,3,4 pointer_mode:
input_skip:
allinput
=D nodragging
-- D
speeds:
......
......@@ -189,6 +189,8 @@ char gui_code[] = "";
" visual:\n"
" rawfb:\n"
" pipeinput:\n"
" uinput_accel:\n"
" uinput_reset:\n"
" 24to32\n"
" =GAL LOFF\n"
"\n"
......@@ -328,6 +330,7 @@ char gui_code[] = "";
"Tuning\n"
" =D-C:0,1,2,3,4 pointer_mode:\n"
" input_skip:\n"
" allinput\n"
" =D nodragging\n"
" -- D\n"
" speeds:\n"
......
This diff is collapsed.
#ifndef _X11VNC_UINPUT_H
#define _X11VNC_UINPUT_H
/* -- uinput.h -- */
extern int check_uinput(void);
extern int initialize_uinput(void);
extern int set_uinput_accel(char *str);
extern void set_uinput_reset(int ms);
extern char *get_uinput_accel();
extern int get_uinput_reset();
extern void parse_uinput_str(char *str);
extern void uinput_pointer_command(int mask, int x, int y, rfbClientPtr client);
extern void uinput_key_command(int down, int keysym, rfbClientPtr client);
#endif /* _X11VNC_UINPUT_H */
......@@ -446,7 +446,24 @@ void rfbCFD(long usec) {
}
if (! use_threads) {
rfbCheckFds(screen, usec);
if (0 && all_input) {
static int cnt = 0;
int f = 1;
while (rfbCheckFds(screen, usec) > 0) {
if (f) {
cnt++;
f = 0;
}
fprintf(stderr, "-%d", cnt);
}
} else {
if (all_input) {
screen->handleEventsEagerly = TRUE;
} else {
screen->handleEventsEagerly = FALSE;
}
rfbCheckFds(screen, usec);
}
}
if (unixpw && unixpw_in_progress && !uip0) {
......
......@@ -6,6 +6,7 @@
#include "xinerama.h"
#include "screen.h"
#include "connections.h"
#include "allowed_input_t.h"
#if LIBVNCSERVER_HAVE_LINUX_VIDEODEV_H
#if LIBVNCSERVER_HAVE_SYS_IOCTL_H
......@@ -556,12 +557,22 @@ static void v4l_fmt(char *fmt) {
}
void v4l_key_command(rfbBool down, rfbKeySym keysym, rfbClientPtr client) {
allowed_input_t input;
if (raw_fb_fd < 0) {
return;
}
if (! down) {
return;
}
if (view_only) {
return;
}
get_allowed_input(client, &input);
if (! input.keystroke) {
return;
}
if (keysym == XK_b) {
v4l_br(-1);
} else if (keysym == XK_B) {
......@@ -608,6 +619,7 @@ void v4l_key_command(rfbBool down, rfbKeySym keysym, rfbClientPtr client) {
void v4l_pointer_command(int mask, int x, int y, rfbClientPtr client) {
/* do not forget viewonly perms */
if (mask || x || y || client) {}
}
......
This diff is collapsed.
......@@ -2073,6 +2073,10 @@ int main(int argc, char* argv[]) {
CHECK_ARGC
ui_skip = atoi(argv[++i]);
if (! ui_skip) ui_skip = 1;
} else if (!strcmp(arg, "-allinput")) {
all_input = 1;
} else if (!strcmp(arg, "-noallinput")) {
all_input = 0;
} else if (!strcmp(arg, "-speeds")) {
CHECK_ARGC
speeds_str = strdup(argv[++i]);
......
......@@ -15,7 +15,7 @@ int xtrap_base_event_type = 0;
int xdamage_base_event_type = 0;
/* date +'lastmod: %Y-%m-%d' */
char lastmod[] = "0.8.2 lastmod: 2006-07-04";
char lastmod[] = "0.8.2 lastmod: 2006-07-08";
/* X display info */
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment