video(1) and modesetting driver

classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|

video(1) and modesetting driver

Raphael Graf-2
Here is an attempt to make video(1) work with the modesetting driver.
See https://marc.info/?l=openbsd-bugs&m=152231686416039&w=2

The general idea:
If there is no common encoding for input (device) and output (Xv), the
encoding is converted to something supported by the output.
No conversion is done if the output is sent to file (options -o/-O).

The option -e is now used as the input format. The output format
is selected automatically (if output goes to Xv).

The following encodings are supported: yuy2, yuyv, uyuy, yv12

yuy2 and yuyv are treated as two different encodings, they share the same
pixelformat, but have different ids. (This behaviour is different from the
current description of '-e' in the manpage)


I have tested on two lenovo laptops using both drivers (modesetting and intel).

On my laptop (lenovo X1 Carbon), with modesetting diver:
yuyv is converted to yv12 before sending to Xv.

On the same laptop With intel driver:
yuyv is converted to yuy2 (no-op)




Index: video.c
===================================================================
RCS file: /cvs/xenocara/app/video/video.c,v
retrieving revision 1.26
diff -u -p -u -p -r1.26 video.c
--- video.c 4 Jan 2019 17:45:00 -0000 1.26
+++ video.c 16 Jan 2019 11:30:21 -0000
@@ -38,6 +38,8 @@
 #include <X11/extensions/Xvlib.h>
 #include <X11/Xatom.h>

+#define V_V4L2_PIX_FMT_YUY2    v4l2_fourcc('Y', 'U', 'Y', '2')
+
 /* Xv(3) adaptor properties */
 struct xv_adap {
  char name[128];
@@ -133,20 +135,23 @@ struct dev {
 /* video encodingss */
 struct encodings {
  char *name;
+ int id;
  int bpp;
- int xv_id;
- int dev_id;
 #define SW_DEV 0x1
 #define SW_XV 0x2
 #define SW_MASK (SW_DEV | SW_XV)
  int flags;
 } encs[] = {
 #define ENC_YUY2 0
- { "yuy2", 16, -1, -1, 0 },
-#define ENC_UYVY 1
- { "uyvy", 16, -1, -1, 0 },
-#define ENC_LAST 2
- { NULL, 0, 0, 0, 0 }
+ { "yuy2", V_V4L2_PIX_FMT_YUY2, 16, 0 },
+#define ENC_YUYV 1
+ { "yuyv", V4L2_PIX_FMT_YUYV, 16, 0 },
+#define ENC_UYVY 2
+ { "uyvy", V4L2_PIX_FMT_UYVY,  16, 0 },
+#define ENC_YV12 3
+ { "yv12", V4L2_PIX_FMT_YVU420, 12, 0 },
+#define ENC_LAST 4
+ { NULL, 0, 0, 0 }
 };

 struct video {
@@ -163,8 +168,10 @@ struct video {
  int iofile_fd;
  char *sz_str;
 #define CONV_SWAP 0x1
+#define CONV_YV12 0x2
  int conv_type;
- int enc;
+ int enc_in;
+ int enc_out;
  int full_screen;
  int net_wm;
  int width;
@@ -216,6 +223,7 @@ int stream(struct video *);
 void got_frame(int);
 void got_shutdown(int);
 int find_enc(char *);
+int find_by_id(int);
 void usage(void);

 static volatile sig_atomic_t play, shutdown, hold, wout;
@@ -242,6 +250,17 @@ find_enc(char *name)
 }

 int
+find_enc_by_id(int id)
+{
+ int i;
+
+ for (i = 0; i < ENC_LAST; i++)
+ if (encs[i].id == id)
+ break;
+ return i;
+}
+
+int
 xv_get_info(struct video *vid)
 {
  struct xdsp *x = &vid->xdsp;
@@ -251,7 +270,6 @@ xv_get_info(struct video *vid)
  struct xv_adap *adap;
  unsigned int nenc, p;
  int num_xvformats, nadaps, i, j, ret;
- char fmtName[5];

  if ((x->dpy = XOpenDisplay(NULL)) == NULL) {
  warnx("cannot open display %s", XDisplayName(NULL));
@@ -312,16 +330,9 @@ xv_get_info(struct video *vid)
     &num_xvformats);
  adap->nfmts = 0;
  for (j = 0; j < num_xvformats; j++) {
- snprintf(fmtName, sizeof(fmtName), "%c%c%c%c",
-    xvformats[j].id & 0xff,
-    (xvformats[j].id >> 8) & 0xff,
-    (xvformats[j].id >> 16) & 0xff,
-    (xvformats[j].id >> 24) & 0xff);
- if (!strcmp(fmtName, "YUY2")) {
- encs[ENC_YUY2].xv_id = xvformats[j].id;
- adap->fmts[adap->nfmts++] = xvformats[j].id;
- } else if (!strcmp(fmtName, "UYVY")) {
- encs[ENC_UYVY].xv_id = xvformats[j].id;
+ int enc = find_enc_by_id(xvformats[j].id);
+ if (enc < ENC_LAST) {
+ encs[enc].flags |= SW_XV;
  adap->fmts[adap->nfmts++] = xvformats[j].id;
  }
  if (adap->nfmts >= MAX_FMTS)
@@ -369,20 +380,20 @@ xv_sel_adap(struct video *vid)
  x->cur_adap = i;
  adap = &x->adaps[i];
  for (i = 0; i < adap->nfmts; i++) {
- if (adap->fmts[i] == encs[vid->enc].xv_id)
+ if (adap->fmts[i] == encs[vid->enc_out].id)
  break;
  }
  if (i >= adap->nfmts) {
  warnx("Xv adaptor '%d' doesn't support %s",
     x->adaps[x->cur_adap].adap_index,
-    encs[vid->enc].name);
+    encs[vid->enc_out].name);
  return 0;
  }
  }
  for (i = 0; i < x->nadaps && x->cur_adap == -1; i++) {
  adap = &x->adaps[i];
  for (j = 0; j < adap->nfmts; j++) {
- if (adap->fmts[j] == encs[vid->enc].xv_id) {
+ if (adap->fmts[j] == encs[vid->enc_out].id) {
  x->cur_adap = i;
  break;
  }
@@ -445,7 +456,7 @@ xv_dump_info(struct video *vid)

  fprintf(stderr, "  encodings: ");
  for (i = 0, j = 0; i < ENC_LAST; i++) {
- if (encs[i].xv_id != -1 && !(encs[i].flags & SW_XV)) {
+ if (encs[i].id != -1 && (encs[i].flags & SW_XV)) {
  if (j)
  fprintf(stderr, ", ");
  fprintf(stderr, "%s", encs[i].name);
@@ -502,7 +513,7 @@ xv_init(struct video *vid)

  resize_window(vid, 0);

- x->xv_image = XvCreateImage(x->dpy, x->port, encs[vid->enc].xv_id,
+ x->xv_image = XvCreateImage(x->dpy, x->port, encs[vid->enc_out].id,
     vid->frame_buffer, vid->width, vid->height);

  return 1;
@@ -780,36 +791,20 @@ dev_get_encs(struct video *vid)
  fmtdesc.index = 0;
  fmtdesc.type = d->buf_type;
  while (ioctl(d->fd, VIDIOC_ENUM_FMT, &fmtdesc) >= 0) {
- if (fmtdesc.pixelformat == V4L2_PIX_FMT_YUYV) {
- i = find_enc("yuy2");
- if (i < ENC_LAST)
- encs[i].dev_id = fmtdesc.pixelformat;
- }
- if (fmtdesc.pixelformat == V4L2_PIX_FMT_UYVY) {
- i = find_enc("uyvy");
- if (i < ENC_LAST)
- encs[i].dev_id = fmtdesc.pixelformat;
+ i = find_enc_by_id(fmtdesc.pixelformat);
+ if (i < ENC_LAST) {
+ encs[i].flags |= SW_DEV;
  }
  fmtdesc.index++;
  }
  for (i = 0; encs[i].name; i++) {
- if (encs[i].dev_id != -1)
+ if (encs[i].flags & SW_DEV)
  break;
  }
  if (i >= ENC_LAST) {
  warnx("%s has no usable YUV encodings", d->path);
  return 0;
  }
- if (encs[ENC_YUY2].dev_id == -1 &&
-    encs[ENC_UYVY].dev_id != -1) {
- encs[ENC_YUY2].dev_id = encs[ENC_UYVY].dev_id;
- encs[ENC_YUY2].flags |= SW_DEV;
- }
- if (encs[ENC_UYVY].dev_id == -1 &&
-    encs[ENC_YUY2].dev_id != -1) {
- encs[ENC_UYVY].dev_id = encs[ENC_YUY2].dev_id;
- encs[ENC_UYVY].flags |= SW_DEV;
- }

  return 1;
 }
@@ -824,7 +819,7 @@ dev_get_sizes(struct video *vid)

  nsizes = 0;
  fsize.index = 0;
- fsize.pixel_format = encs[vid->enc].dev_id;
+ fsize.pixel_format = encs[vid->enc_in].id;
  while (ioctl(d->fd, VIDIOC_ENUM_FRAMESIZES, &fsize) == 0) {
  switch (fsize.type) {
  case V4L2_FRMSIZE_TYPE_DISCRETE:
@@ -915,7 +910,7 @@ dev_get_rates(struct video *vid)

  for (i = 0; i < d->nsizes; i++) {
  bzero(&ival, sizeof(ival));
- ival.pixel_format = encs[vid->enc].dev_id;
+ ival.pixel_format = encs[vid->enc_in].id;
  ival.width = d->sizes[i].w;
  ival.height = d->sizes[i].h;
  ival.index = 0;
@@ -1072,7 +1067,7 @@ dev_dump_info(struct video *vid)

  fprintf(stderr, "  encodings: ");
  for (i = 0, j = 0; i < ENC_LAST; i++) {
- if (encs[i].dev_id != -1 && !(encs[i].flags & SW_DEV)) {
+ if (encs[i].flags & SW_DEV) {
  if (j)
  fprintf(stderr, ", ");
  fprintf(stderr, "%s", encs[i].name);
@@ -1136,7 +1131,7 @@ dev_init(struct video *vid)
  fmt.type = d->buf_type;
  fmt.fmt.pix.width = vid->width;
  fmt.fmt.pix.height = vid->height;
- fmt.fmt.pix.pixelformat = encs[vid->enc].dev_id;
+ fmt.fmt.pix.pixelformat = encs[vid->enc_in].id;
  fmt.fmt.pix.field = V4L2_FIELD_ANY;
  if (ioctl(d->fd, VIDIOC_S_FMT, &fmt) < 0) {
  warn("VIDIOC_S_FMT");
@@ -1290,57 +1285,67 @@ choose_enc(struct video *vid)
 {
  int i;

- if (vid->enc < 0) {
- for (i = 0; vid->enc < 0 && i < ENC_LAST; i++) {
- if ((vid->mode & M_IN_DEV) && (vid->mode & M_OUT_XV)) {
- if (encs[i].dev_id != -1 &&
-    encs[i].xv_id != -1 &&
-    (encs[i].flags & SW_MASK) == 0)
- vid->enc = i;
- } else if (vid->mode & M_IN_DEV) {
- if (encs[i].dev_id != -1 &&
-    (encs[i].flags & SW_MASK) == 0)
- vid->enc = i;
- } else if (vid->mode & M_OUT_XV) {
- if (encs[i].xv_id != -1 &&
-    (encs[i].flags & SW_MASK) == 0)
- vid->enc = i;
- }
- }
- for (i = 0; vid->enc < 0 && i < ENC_LAST; i++) {
- if ((vid->mode & M_IN_DEV) && (vid->mode & M_OUT_XV)) {
- if (encs[i].dev_id != -1 &&
-    encs[i].xv_id != -1)
- vid->enc = i;
- } else if (vid->mode & M_IN_DEV) {
- if (encs[i].dev_id != -1)
- vid->enc = i;
- } else if (vid->mode & M_OUT_XV) {
- if (encs[i].xv_id != -1)
- vid->enc = i;
+ if (vid->enc_in < 0) {
+ for (i = 0; vid->enc_in < 0 && i < ENC_LAST; i++) {
+ if (encs[i].flags & SW_DEV)
+ vid->enc_in = i;
+ }
+ }
+
+ if (vid->enc_out < 0) {
+ for (i = 0; vid->enc_out < 0 && i < ENC_LAST; i++) {
+ if (encs[i].flags & SW_XV)
+ vid->enc_out = i;
+ }
+ }
+
+ if ((vid->mode & M_IN_DEV) && (vid->mode & M_OUT_XV)) {
+ for (i = 0; vid->enc_in < 0 && i < ENC_LAST; i++) {
+ if ((encs[i].flags & SW_DEV) &&
+    (encs[i].flags & SW_XV)) {
+ vid->enc_in = i;
+ vid->enc_out = i;
  }
  }
  }
- if (vid->enc < 0) {
+
+ if (vid->enc_in < 0 && vid->mode & M_IN_FILE)
+ vid->enc_in = ENC_YUY2;
+
+ if (vid->mode & M_OUT_XV && vid->enc_in != vid->enc_out) {
+ /* check if conversion is possible */
+ int enc_in = (vid->enc_in == ENC_YUYV) ? ENC_YUY2 : vid->enc_in;
+ int enc_out = (vid->enc_out == ENC_YUYV) ? ENC_YUY2 : vid->enc_out;
+
+ if (enc_in == enc_out)
+ vid->conv_type = 0; /* same pixelformat, different id */
+ else if ((enc_in == ENC_YUY2 && enc_out == ENC_UYVY) ||
+    (enc_in == ENC_UYVY && enc_out == ENC_YUY2))
+ vid->conv_type = CONV_SWAP;
+ else if (enc_in == ENC_YUY2 && enc_out == ENC_YV12)
+ vid->conv_type = CONV_YV12;
+ else {
+ warn("can't convert from %s to %s",
+    encs[vid->enc_in].name, encs[vid->enc_out].name);
+ return 0;
+ }
+ }
+
+ if (vid->enc_in < 0) {
  warnx("could not find a usable encoding");
  return 0;
  }
- if ((vid->mode & M_IN_DEV) && encs[vid->enc].dev_id == -1) {
+ if ((vid->mode & M_IN_DEV) && !(encs[vid->enc_in].flags & SW_DEV)) {
  warnx("device %s can't supply %s", vid->dev.path,
-    encs[vid->enc].name);
+    encs[vid->enc_in].name);
  return 0;
  }
- if ((vid->mode & M_OUT_XV) && encs[vid->enc].xv_id == -1) {
+ if ((vid->mode & M_OUT_XV) && !(encs[vid->enc_out].flags & SW_XV)) {
  warnx("Xv adaptor %d can't display %s",
     vid->xdsp.adaps[vid->xdsp.cur_adap].adap_index,
-    encs[vid->enc].name);
+    encs[vid->enc_out].name);
  return 0;
  }
- if (((vid->mode & M_IN_DEV) &&
-    (encs[vid->enc].flags & SW_MASK) == SW_DEV) ||
-    ((vid->mode & M_OUT_XV) &&
-    (encs[vid->enc].flags & SW_MASK) == SW_XV))
- vid->conv_type = CONV_SWAP;

  return 1;
 }
@@ -1469,14 +1474,14 @@ setup(struct video *vid)
  if (!parse_size(vid) || !choose_size(vid))
  return 0;

- vid->bpf = vid->width * vid->height * encs[vid->enc].bpp / NBBY;
+ vid->bpf = (vid->width * vid->height * encs[vid->enc_in].bpp) / NBBY;

  if (vid->verbose > 0) {
  if (vid->mode & M_IN_DEV)
  dev_dump_info(vid);
  if (vid->mode & M_OUT_XV)
  xv_dump_info(vid);
- fprintf(stderr, "using %s encoding\n", encs[vid->enc].name);
+ fprintf(stderr, "using %s encoding\n", encs[vid->enc_in].name);
  fprintf(stderr, "using frame size %dx%d (%d bytes)\n",
     vid->width, vid->height, vid->bpf);
  if (vid->fps)
@@ -1491,12 +1496,7 @@ setup(struct video *vid)
  }

  if (vid->conv_type) {
- if (vid->conv_type == CONV_SWAP) {
- vid->conv_bufsz = vid->bpf;
- } else {
- warnx("invalid conversion type");
- return 0;
- }
+ vid->conv_bufsz = (vid->width * vid->height * encs[vid->enc_out].bpp) / NBBY;
  if ((vid->conv_buffer = calloc(1, vid->conv_bufsz)) == NULL) {
  warn("conv_buffer");
  return 0;
@@ -1539,8 +1539,9 @@ ioctl_input(struct video *vid)
  }

  /* copy frame buffer */
- if (buf.bytesused > vid->bpf)
+ if (buf.bytesused > vid->bpf) {
  return 0;
+ }
  memcpy(vid->frame_buffer, vid->mmap_buffer[buf.index], buf.bytesused);

  /* requeue buffer */
@@ -1632,6 +1633,29 @@ got_shutdown(int s)
  shutdown = 1;
 }

+int yuy2_to_yv12(uint8_t *src, uint8_t *dst, int width, int height) {
+ int i, j;
+ int iy = 0, iu, h1, h2, c;
+ int bpf = width * height * 2;
+
+ for (i = 0; i < bpf; i+= 2)
+ dst[iy++] = src[i]; //Y component
+
+ iu = iy + bpf / 8;
+ for (i = 0; i < height / 2; i++) { //row
+ c = i * width * 4;
+ for (j = 0; j < width / 2; j++) { //col
+ h1 = c + j * 4 + 1;
+ h2 = h1 + width * 2;
+ dst[iu++] = (src[h1] + src[h2]) / 2;
+ h1+= 2; h2+= 2;
+ dst[iy++] = (src[h1] + src[h2]) / 2;
+ }
+ }
+
+ return 0;
+}
+
 int
 stream(struct video *vid)
 {
@@ -1721,7 +1745,8 @@ stream(struct video *vid)
  src = vid->frame_buffer;
  if (vid->conv_type == CONV_SWAP) {
  swab(src, vid->conv_buffer, vid->bpf);
- src = vid->conv_buffer;
+ } else if (vid->conv_type == CONV_YV12) {
+ yuy2_to_yv12(src, vid->conv_buffer, vid->width, vid->height);
  }

  if ((vid->mode & M_OUT_FILE) && wout) {
@@ -1743,6 +1768,7 @@ stream(struct video *vid)
  }
  }
  if (vid->mode & M_OUT_XV) {
+ src = (vid->conv_type) ? vid->conv_buffer : vid->frame_buffer;
  x->xv_image->data = src;
  if (x->resized) {
  x->resized = 0;
@@ -1862,7 +1888,8 @@ main(int argc, char *argv[])
  x->cur_adap = -1;
  vid.dev.fd = vid.iofile_fd = -1;
  vid.mode = M_IN_DEV | M_OUT_XV;
- vid.enc = -1;
+ vid.enc_in = -1;
+ vid.enc_out = -1;
  vid.mmap_on = 1; /* mmap method is default */
  wout = 1;

@@ -1876,8 +1903,8 @@ main(int argc, char *argv[])
  }
  break;
  case 'e':
- vid.enc = find_enc(optarg);
- if (vid.enc >= ENC_LAST) {
+ vid.enc_in = find_enc(optarg);
+ if (vid.enc_in >= ENC_LAST) {
  warnx("encoding '%s' is invalid", optarg);
  errs++;
  }


Reply | Threaded
Open this post in threaded view
|

Re: video(1) and modesetting driver

Martin Pieuchot
Hello Raphael,

On 16/01/19(Wed) 12:41, Raphael Graf wrote:

> Here is an attempt to make video(1) work with the modesetting driver.
> See https://marc.info/?l=openbsd-bugs&m=152231686416039&w=2
>
> The general idea:
> If there is no common encoding for input (device) and output (Xv), the
> encoding is converted to something supported by the output.
> No conversion is done if the output is sent to file (options -o/-O).
>
> The option -e is now used as the input format. The output format
> is selected automatically (if output goes to Xv).
>
> The following encodings are supported: yuy2, yuyv, uyuy, yv12

Did you test all of them?

> yuy2 and yuyv are treated as two different encodings, they share the same
> pixelformat, but have different ids. (This behaviour is different from the
> current description of '-e' in the manpage)

What about fixing the manpage then? ;)

> I have tested on two lenovo laptops using both drivers (modesetting and intel).
>
> On my laptop (lenovo X1 Carbon), with modesetting diver:
> yuyv is converted to yv12 before sending to Xv.
>
> On the same laptop With intel driver:
> yuyv is converted to yuy2 (no-op)

Nice work, some comments below.

Make sure the code you're writing follows style(9) :o)

> Index: video.c
> ===================================================================
> RCS file: /cvs/xenocara/app/video/video.c,v
> retrieving revision 1.26
> diff -u -p -u -p -r1.26 video.c
> --- video.c 4 Jan 2019 17:45:00 -0000 1.26
> +++ video.c 16 Jan 2019 11:30:21 -0000
> @@ -38,6 +38,8 @@
>  #include <X11/extensions/Xvlib.h>
>  #include <X11/Xatom.h>
>
> +#define V_V4L2_PIX_FMT_YUY2    v4l2_fourcc('Y', 'U', 'Y', '2')

Why do we need this define and not the others?  Aren't we missing an
include?

> +
>  /* Xv(3) adaptor properties */
>  struct xv_adap {
>   char name[128];
> @@ -133,20 +135,23 @@ struct dev {
>  /* video encodingss */
>  struct encodings {
>   char *name;
> + int id;
>   int bpp;
> - int xv_id;
> - int dev_id;
>  #define SW_DEV 0x1
>  #define SW_XV 0x2
>  #define SW_MASK (SW_DEV | SW_XV)
>   int flags;
>  } encs[] = {
>  #define ENC_YUY2 0
> - { "yuy2", 16, -1, -1, 0 },
> -#define ENC_UYVY 1
> - { "uyvy", 16, -1, -1, 0 },
> -#define ENC_LAST 2
> - { NULL, 0, 0, 0, 0 }
> + { "yuy2", V_V4L2_PIX_FMT_YUY2, 16, 0 },
> +#define ENC_YUYV 1
> + { "yuyv", V4L2_PIX_FMT_YUYV, 16, 0 },
> +#define ENC_UYVY 2
> + { "uyvy", V4L2_PIX_FMT_UYVY,  16, 0 },
> +#define ENC_YV12 3
> + { "yv12", V4L2_PIX_FMT_YVU420, 12, 0 },
> +#define ENC_LAST 4
> + { NULL, 0, 0, 0 }
>  };
>
>  struct video {
> @@ -163,8 +168,10 @@ struct video {
>   int iofile_fd;
>   char *sz_str;
>  #define CONV_SWAP 0x1
> +#define CONV_YV12 0x2
>   int conv_type;
> - int enc;
> + int enc_in;
> + int enc_out;
>   int full_screen;
>   int net_wm;
>   int width;
> @@ -216,6 +223,7 @@ int stream(struct video *);
>  void got_frame(int);
>  void got_shutdown(int);
>  int find_enc(char *);
> +int find_by_id(int);
>  void usage(void);
>
>  static volatile sig_atomic_t play, shutdown, hold, wout;
> @@ -242,6 +250,17 @@ find_enc(char *name)
>  }
>
>  int
> +find_enc_by_id(int id)
> +{
> + int i;
> +
> + for (i = 0; i < ENC_LAST; i++)
> + if (encs[i].id == id)
> + break;
> + return i;
> +}
> +
> +int
>  xv_get_info(struct video *vid)
>  {
>   struct xdsp *x = &vid->xdsp;
> @@ -251,7 +270,6 @@ xv_get_info(struct video *vid)
>   struct xv_adap *adap;
>   unsigned int nenc, p;
>   int num_xvformats, nadaps, i, j, ret;
> - char fmtName[5];
>
>   if ((x->dpy = XOpenDisplay(NULL)) == NULL) {
>   warnx("cannot open display %s", XDisplayName(NULL));
> @@ -312,16 +330,9 @@ xv_get_info(struct video *vid)
>      &num_xvformats);
>   adap->nfmts = 0;
>   for (j = 0; j < num_xvformats; j++) {
> - snprintf(fmtName, sizeof(fmtName), "%c%c%c%c",
> -    xvformats[j].id & 0xff,
> -    (xvformats[j].id >> 8) & 0xff,
> -    (xvformats[j].id >> 16) & 0xff,
> -    (xvformats[j].id >> 24) & 0xff);
> - if (!strcmp(fmtName, "YUY2")) {
> - encs[ENC_YUY2].xv_id = xvformats[j].id;
> - adap->fmts[adap->nfmts++] = xvformats[j].id;
> - } else if (!strcmp(fmtName, "UYVY")) {
> - encs[ENC_UYVY].xv_id = xvformats[j].id;
> + int enc = find_enc_by_id(xvformats[j].id);
> + if (enc < ENC_LAST) {
> + encs[enc].flags |= SW_XV;
>   adap->fmts[adap->nfmts++] = xvformats[j].id;
>   }
>   if (adap->nfmts >= MAX_FMTS)
> @@ -369,20 +380,20 @@ xv_sel_adap(struct video *vid)
>   x->cur_adap = i;
>   adap = &x->adaps[i];
>   for (i = 0; i < adap->nfmts; i++) {
> - if (adap->fmts[i] == encs[vid->enc].xv_id)
> + if (adap->fmts[i] == encs[vid->enc_out].id)

What about a small diff that introduce `enc_out' and `enc_in' to
reduce the number of chunks in this one?

>   break;
>   }
>   if (i >= adap->nfmts) {
>   warnx("Xv adaptor '%d' doesn't support %s",
>      x->adaps[x->cur_adap].adap_index,
> -    encs[vid->enc].name);
> +    encs[vid->enc_out].name);
>   return 0;
>   }
>   }
>   for (i = 0; i < x->nadaps && x->cur_adap == -1; i++) {
>   adap = &x->adaps[i];
>   for (j = 0; j < adap->nfmts; j++) {
> - if (adap->fmts[j] == encs[vid->enc].xv_id) {
> + if (adap->fmts[j] == encs[vid->enc_out].id) {
>   x->cur_adap = i;
>   break;
>   }
> @@ -445,7 +456,7 @@ xv_dump_info(struct video *vid)
>
>   fprintf(stderr, "  encodings: ");
>   for (i = 0, j = 0; i < ENC_LAST; i++) {
> - if (encs[i].xv_id != -1 && !(encs[i].flags & SW_XV)) {
> + if (encs[i].id != -1 && (encs[i].flags & SW_XV)) {
>   if (j)
>   fprintf(stderr, ", ");
>   fprintf(stderr, "%s", encs[i].name);
> @@ -502,7 +513,7 @@ xv_init(struct video *vid)
>
>   resize_window(vid, 0);
>
> - x->xv_image = XvCreateImage(x->dpy, x->port, encs[vid->enc].xv_id,
> + x->xv_image = XvCreateImage(x->dpy, x->port, encs[vid->enc_out].id,
>      vid->frame_buffer, vid->width, vid->height);
>
>   return 1;
> @@ -780,36 +791,20 @@ dev_get_encs(struct video *vid)
>   fmtdesc.index = 0;
>   fmtdesc.type = d->buf_type;
>   while (ioctl(d->fd, VIDIOC_ENUM_FMT, &fmtdesc) >= 0) {
> - if (fmtdesc.pixelformat == V4L2_PIX_FMT_YUYV) {
> - i = find_enc("yuy2");
> - if (i < ENC_LAST)
> - encs[i].dev_id = fmtdesc.pixelformat;
> - }
> - if (fmtdesc.pixelformat == V4L2_PIX_FMT_UYVY) {
> - i = find_enc("uyvy");
> - if (i < ENC_LAST)
> - encs[i].dev_id = fmtdesc.pixelformat;
> + i = find_enc_by_id(fmtdesc.pixelformat);
> + if (i < ENC_LAST) {
> + encs[i].flags |= SW_DEV;
>   }
>   fmtdesc.index++;
>   }
>   for (i = 0; encs[i].name; i++) {
> - if (encs[i].dev_id != -1)
> + if (encs[i].flags & SW_DEV)
>   break;
>   }
>   if (i >= ENC_LAST) {
>   warnx("%s has no usable YUV encodings", d->path);
>   return 0;
>   }
> - if (encs[ENC_YUY2].dev_id == -1 &&
> -    encs[ENC_UYVY].dev_id != -1) {
> - encs[ENC_YUY2].dev_id = encs[ENC_UYVY].dev_id;
> - encs[ENC_YUY2].flags |= SW_DEV;
> - }
> - if (encs[ENC_UYVY].dev_id == -1 &&
> -    encs[ENC_YUY2].dev_id != -1) {
> - encs[ENC_UYVY].dev_id = encs[ENC_YUY2].dev_id;
> - encs[ENC_UYVY].flags |= SW_DEV;
> - }
>
>   return 1;
>  }
> @@ -824,7 +819,7 @@ dev_get_sizes(struct video *vid)
>
>   nsizes = 0;
>   fsize.index = 0;
> - fsize.pixel_format = encs[vid->enc].dev_id;
> + fsize.pixel_format = encs[vid->enc_in].id;
>   while (ioctl(d->fd, VIDIOC_ENUM_FRAMESIZES, &fsize) == 0) {
>   switch (fsize.type) {
>   case V4L2_FRMSIZE_TYPE_DISCRETE:
> @@ -915,7 +910,7 @@ dev_get_rates(struct video *vid)
>
>   for (i = 0; i < d->nsizes; i++) {
>   bzero(&ival, sizeof(ival));
> - ival.pixel_format = encs[vid->enc].dev_id;
> + ival.pixel_format = encs[vid->enc_in].id;
>   ival.width = d->sizes[i].w;
>   ival.height = d->sizes[i].h;
>   ival.index = 0;
> @@ -1072,7 +1067,7 @@ dev_dump_info(struct video *vid)
>
>   fprintf(stderr, "  encodings: ");
>   for (i = 0, j = 0; i < ENC_LAST; i++) {
> - if (encs[i].dev_id != -1 && !(encs[i].flags & SW_DEV)) {
> + if (encs[i].flags & SW_DEV) {
>   if (j)
>   fprintf(stderr, ", ");
>   fprintf(stderr, "%s", encs[i].name);
> @@ -1136,7 +1131,7 @@ dev_init(struct video *vid)
>   fmt.type = d->buf_type;
>   fmt.fmt.pix.width = vid->width;
>   fmt.fmt.pix.height = vid->height;
> - fmt.fmt.pix.pixelformat = encs[vid->enc].dev_id;
> + fmt.fmt.pix.pixelformat = encs[vid->enc_in].id;
>   fmt.fmt.pix.field = V4L2_FIELD_ANY;
>   if (ioctl(d->fd, VIDIOC_S_FMT, &fmt) < 0) {
>   warn("VIDIOC_S_FMT");
> @@ -1290,57 +1285,67 @@ choose_enc(struct video *vid)
>  {
>   int i;
>
> - if (vid->enc < 0) {
> - for (i = 0; vid->enc < 0 && i < ENC_LAST; i++) {
> - if ((vid->mode & M_IN_DEV) && (vid->mode & M_OUT_XV)) {
> - if (encs[i].dev_id != -1 &&
> -    encs[i].xv_id != -1 &&
> -    (encs[i].flags & SW_MASK) == 0)
> - vid->enc = i;
> - } else if (vid->mode & M_IN_DEV) {
> - if (encs[i].dev_id != -1 &&
> -    (encs[i].flags & SW_MASK) == 0)
> - vid->enc = i;
> - } else if (vid->mode & M_OUT_XV) {
> - if (encs[i].xv_id != -1 &&
> -    (encs[i].flags & SW_MASK) == 0)
> - vid->enc = i;
> - }
> - }
> - for (i = 0; vid->enc < 0 && i < ENC_LAST; i++) {
> - if ((vid->mode & M_IN_DEV) && (vid->mode & M_OUT_XV)) {
> - if (encs[i].dev_id != -1 &&
> -    encs[i].xv_id != -1)
> - vid->enc = i;
> - } else if (vid->mode & M_IN_DEV) {
> - if (encs[i].dev_id != -1)
> - vid->enc = i;
> - } else if (vid->mode & M_OUT_XV) {
> - if (encs[i].xv_id != -1)
> - vid->enc = i;
> + if (vid->enc_in < 0) {
> + for (i = 0; vid->enc_in < 0 && i < ENC_LAST; i++) {
> + if (encs[i].flags & SW_DEV)
> + vid->enc_in = i;
> + }
> + }
> +
> + if (vid->enc_out < 0) {
> + for (i = 0; vid->enc_out < 0 && i < ENC_LAST; i++) {
> + if (encs[i].flags & SW_XV)
> + vid->enc_out = i;
> + }
> + }
> +
> + if ((vid->mode & M_IN_DEV) && (vid->mode & M_OUT_XV)) {
> + for (i = 0; vid->enc_in < 0 && i < ENC_LAST; i++) {
> + if ((encs[i].flags & SW_DEV) &&
> +    (encs[i].flags & SW_XV)) {
> + vid->enc_in = i;
> + vid->enc_out = i;
>   }
>   }
>   }
> - if (vid->enc < 0) {
> +
> + if (vid->enc_in < 0 && vid->mode & M_IN_FILE)
> + vid->enc_in = ENC_YUY2;
> +
> + if (vid->mode & M_OUT_XV && vid->enc_in != vid->enc_out) {
> + /* check if conversion is possible */
> + int enc_in = (vid->enc_in == ENC_YUYV) ? ENC_YUY2 : vid->enc_in;
> + int enc_out = (vid->enc_out == ENC_YUYV) ? ENC_YUY2 : vid->enc_out;

Please define your variable first then do the assignment.  This will
also make your lines fit in 80 chars :)

> +
> + if (enc_in == enc_out)
> + vid->conv_type = 0; /* same pixelformat, different id */

Could we use a define for no conversion?  CONV_NONE?  This would
implicitly document your code so you can get rid of the comment above :)

> + else if ((enc_in == ENC_YUY2 && enc_out == ENC_UYVY) ||
> +    (enc_in == ENC_UYVY && enc_out == ENC_YUY2))
> + vid->conv_type = CONV_SWAP;
> + else if (enc_in == ENC_YUY2 && enc_out == ENC_YV12)
> + vid->conv_type = CONV_YV12;
> + else {
> + warn("can't convert from %s to %s",
> +    encs[vid->enc_in].name, encs[vid->enc_out].name);

Is it impossible to convert or is it unsupported?

> + return 0;
> + }
> + }
> +
> + if (vid->enc_in < 0) {
>   warnx("could not find a usable encoding");
>   return 0;
>   }
> - if ((vid->mode & M_IN_DEV) && encs[vid->enc].dev_id == -1) {
> + if ((vid->mode & M_IN_DEV) && !(encs[vid->enc_in].flags & SW_DEV)) {
>   warnx("device %s can't supply %s", vid->dev.path,
> -    encs[vid->enc].name);
> +    encs[vid->enc_in].name);
>   return 0;
>   }
> - if ((vid->mode & M_OUT_XV) && encs[vid->enc].xv_id == -1) {
> + if ((vid->mode & M_OUT_XV) && !(encs[vid->enc_out].flags & SW_XV)) {
>   warnx("Xv adaptor %d can't display %s",
>      vid->xdsp.adaps[vid->xdsp.cur_adap].adap_index,
> -    encs[vid->enc].name);
> +    encs[vid->enc_out].name);
>   return 0;
>   }
> - if (((vid->mode & M_IN_DEV) &&
> -    (encs[vid->enc].flags & SW_MASK) == SW_DEV) ||
> -    ((vid->mode & M_OUT_XV) &&
> -    (encs[vid->enc].flags & SW_MASK) == SW_XV))
> - vid->conv_type = CONV_SWAP;
>
>   return 1;
>  }
> @@ -1469,14 +1474,14 @@ setup(struct video *vid)
>   if (!parse_size(vid) || !choose_size(vid))
>   return 0;
>
> - vid->bpf = vid->width * vid->height * encs[vid->enc].bpp / NBBY;
> + vid->bpf = (vid->width * vid->height * encs[vid->enc_in].bpp) / NBBY;
>
>   if (vid->verbose > 0) {
>   if (vid->mode & M_IN_DEV)
>   dev_dump_info(vid);
>   if (vid->mode & M_OUT_XV)
>   xv_dump_info(vid);
> - fprintf(stderr, "using %s encoding\n", encs[vid->enc].name);
> + fprintf(stderr, "using %s encoding\n", encs[vid->enc_in].name);
>   fprintf(stderr, "using frame size %dx%d (%d bytes)\n",
>      vid->width, vid->height, vid->bpf);
>   if (vid->fps)
> @@ -1491,12 +1496,7 @@ setup(struct video *vid)
>   }
>
>   if (vid->conv_type) {
> - if (vid->conv_type == CONV_SWAP) {
> - vid->conv_bufsz = vid->bpf;
> - } else {
> - warnx("invalid conversion type");
> - return 0;
> - }
> + vid->conv_bufsz = (vid->width * vid->height * encs[vid->enc_out].bpp) / NBBY;

Please reformat this line to fit in 80 chars :)

>   if ((vid->conv_buffer = calloc(1, vid->conv_bufsz)) == NULL) {
>   warn("conv_buffer");
>   return 0;
> @@ -1539,8 +1539,9 @@ ioctl_input(struct video *vid)
>   }
>
>   /* copy frame buffer */
> - if (buf.bytesused > vid->bpf)
> + if (buf.bytesused > vid->bpf) {
>   return 0;
> + }
>   memcpy(vid->frame_buffer, vid->mmap_buffer[buf.index], buf.bytesused);
>
>   /* requeue buffer */
> @@ -1632,6 +1633,29 @@ got_shutdown(int s)
>   shutdown = 1;
>  }
>
> +int yuy2_to_yv12(uint8_t *src, uint8_t *dst, int width, int height) {

This function definition should respect style(9) :)

> + int i, j;
> + int iy = 0, iu, h1, h2, c;
> + int bpf = width * height * 2;
> +
> + for (i = 0; i < bpf; i+= 2)
> + dst[iy++] = src[i]; //Y component

Please do not use C++-style comments.  You could also pick better name
for your variables, if `i' represents the rows and `j' the columns then
use `row' and `col' ;)  What does `c' represent?  And the others?

> + iu = iy + bpf / 8;
> + for (i = 0; i < height / 2; i++) { //row
> + c = i * width * 4;
> + for (j = 0; j < width / 2; j++) { //col
> + h1 = c + j * 4 + 1;
> + h2 = h1 + width * 2;
> + dst[iu++] = (src[h1] + src[h2]) / 2;
> + h1+= 2; h2+= 2;
> + dst[iy++] = (src[h1] + src[h2]) / 2;
> + }
> + }
> +
> + return 0;
> +}
> +
>  int
>  stream(struct video *vid)
>  {
> @@ -1721,7 +1745,8 @@ stream(struct video *vid)
>   src = vid->frame_buffer;
>   if (vid->conv_type == CONV_SWAP) {
>   swab(src, vid->conv_buffer, vid->bpf);
> - src = vid->conv_buffer;
> + } else if (vid->conv_type == CONV_YV12) {
> + yuy2_to_yv12(src, vid->conv_buffer, vid->width, vid->height);
>   }
>
>   if ((vid->mode & M_OUT_FILE) && wout) {
> @@ -1743,6 +1768,7 @@ stream(struct video *vid)
>   }
>   }
>   if (vid->mode & M_OUT_XV) {
> + src = (vid->conv_type) ? vid->conv_buffer : vid->frame_buffer;

Please reformat this line to fit in 80 chars ;)

>   x->xv_image->data = src;
>   if (x->resized) {
>   x->resized = 0;
> @@ -1862,7 +1888,8 @@ main(int argc, char *argv[])
>   x->cur_adap = -1;
>   vid.dev.fd = vid.iofile_fd = -1;
>   vid.mode = M_IN_DEV | M_OUT_XV;
> - vid.enc = -1;
> + vid.enc_in = -1;
> + vid.enc_out = -1;
>   vid.mmap_on = 1; /* mmap method is default */
>   wout = 1;
>
> @@ -1876,8 +1903,8 @@ main(int argc, char *argv[])
>   }
>   break;
>   case 'e':
> - vid.enc = find_enc(optarg);
> - if (vid.enc >= ENC_LAST) {
> + vid.enc_in = find_enc(optarg);
> + if (vid.enc_in >= ENC_LAST) {
>   warnx("encoding '%s' is invalid", optarg);
>   errs++;
>   }
>
>

Reply | Threaded
Open this post in threaded view
|

Re: video(1) and modesetting driver

Raphael Graf-2
Hi Martin,
Thanks for your feedback!

On Wed, Jan 23, 2019 at 12:54:51PM -0200, Martin Pieuchot wrote:

> Hello Raphael,
>
> On 16/01/19(Wed) 12:41, Raphael Graf wrote:
> > Here is an attempt to make video(1) work with the modesetting driver.
> > See https://marc.info/?l=openbsd-bugs&m=152231686416039&w=2
> >
> > The general idea:
> > If there is no common encoding for input (device) and output (Xv), the
> > encoding is converted to something supported by the output.
> > No conversion is done if the output is sent to file (options -o/-O).
> >
> > The option -e is now used as the input format. The output format
> > is selected automatically (if output goes to Xv).
> >
> > The following encodings are supported: yuy2, yuyv, uyuy, yv12
>
> Did you test all of them?

Yes, I have tested all encodings with these two devices:

uvideo0 at uhub0 port 6 configuration 1 interface 0
"Chicony Electronics Co., Ltd. product 0x480c" rev 2.00/31.34 addr 2

uvideo0 at uhub1 port 8 configuration 1 interface 0 "SC20A38485AA4629RX
Integrated Camera" rev 2.00/0.21 addr 4

>
> > yuy2 and yuyv are treated as two different encodings, they share the same
> > pixelformat, but have different ids. (This behaviour is different from the
> > current description of '-e' in the manpage)
>
> What about fixing the manpage then? ;)

The new diff below contains the changes to the manpage.

>
> > I have tested on two lenovo laptops using both drivers (modesetting and intel).
> >
> > On my laptop (lenovo X1 Carbon), with modesetting diver:
> > yuyv is converted to yv12 before sending to Xv.
> >
> > On the same laptop With intel driver:
> > yuyv is converted to yuy2 (no-op)
>
> Nice work, some comments below.
>
> Make sure the code you're writing follows style(9) :o)
>
> > Index: video.c
> > ===================================================================
> > RCS file: /cvs/xenocara/app/video/video.c,v
> > retrieving revision 1.26
> > diff -u -p -u -p -r1.26 video.c
> > --- video.c 4 Jan 2019 17:45:00 -0000 1.26
> > +++ video.c 16 Jan 2019 11:30:21 -0000
> > @@ -38,6 +38,8 @@
> >  #include <X11/extensions/Xvlib.h>
> >  #include <X11/Xatom.h>
> >
> > +#define V_V4L2_PIX_FMT_YUY2    v4l2_fourcc('Y', 'U', 'Y', '2')
>
> Why do we need this define and not the others?  Aren't we missing an
> include?

Actually, I don't know why this define is missing in videoio.h.
This YUY2 format (id:0x32595559) is reported by Xv.
The uvideo(4) driver reports YUYV which has a define in videoio.h.
(YUY2 and YUYV are equivalent)

In the new diff, I have changed the variable names enc_in/enc_out to enc/enc_xv
as the second 'enc' is only used for Xv output.
I also fixed the style(9) issues and added a CONV_NONE define.

Note that I'm not sure if the algorithm used in the yuy2_to_yv12 function is
correct in respect to color theory..
(It is a conversion from 16 to 12 bits/pixel)

Sorry, it is still a big diff, feedback is welcome.



Index: video.1
===================================================================
RCS file: /cvs/xenocara/app/video/video.1,v
retrieving revision 1.13
diff -u -p -u -p -r1.13 video.1
--- video.1 4 Jun 2016 07:44:32 -0000 1.13
+++ video.1 25 Jan 2019 09:51:12 -0000
@@ -84,18 +84,19 @@ The default is 0, the first adaptor repo
 .It Fl e Ar encoding
 Lowercase FOURCC name of video encoding to use.
 Valid arguments are
+.Ql yuyv ,
+.Ql yuy2 ,
 .Ql uyvy
 and
-.Ql yuy2 .
+.Ql yv12 .
 The default is
-.Ql yuy2
-unless
-.Ar file
-is being used and only supports
-.Ql uyvy ,
-in which case
-.Ql uyvy
-will be used by default.
+.Ql yuyv .
+If reading from a
+.Xr video 4
+device and
+.Ql yuyv
+is not supported, the default is
+.Ql uyvy .
 .It Fl f Ar file
 .Xr video 4
 device from which frames will be read.
@@ -338,6 +339,6 @@ Note that with the first three commands,
 does not support 640x480 pixels sized frames, the largest frame size
 smaller than 640x480 will be used, and if
 .Ar /dev/video
-does not support yuy2 encoding, uyvy will be used.
+does not support yuyv encoding, uyvy will be used.
 .Sh SEE ALSO
 .Xr video 4
Index: video.c
===================================================================
RCS file: /cvs/xenocara/app/video/video.c,v
retrieving revision 1.27
diff -u -p -u -p -r1.27 video.c
--- video.c 22 Jan 2019 20:02:40 -0000 1.27
+++ video.c 25 Jan 2019 09:51:12 -0000
@@ -38,6 +38,8 @@
 #include <X11/extensions/Xvlib.h>
 #include <X11/Xatom.h>
 
+#define V_V4L2_PIX_FMT_YUY2    v4l2_fourcc('Y', 'U', 'Y', '2')
+
 /* Xv(3) adaptor properties */
 struct xv_adap {
  char name[128];
@@ -133,20 +135,23 @@ struct dev {
 /* video encodingss */
 struct encodings {
  char *name;
+ int id;
  int bpp;
- int xv_id;
- int dev_id;
 #define SW_DEV 0x1
 #define SW_XV 0x2
 #define SW_MASK (SW_DEV | SW_XV)
  int flags;
 } encs[] = {
 #define ENC_YUY2 0
- { "yuy2", 16, -1, -1, 0 },
-#define ENC_UYVY 1
- { "uyvy", 16, -1, -1, 0 },
-#define ENC_LAST 2
- { NULL, 0, 0, 0, 0 }
+ { "yuy2", V_V4L2_PIX_FMT_YUY2, 16, 0 },
+#define ENC_YUYV 1
+ { "yuyv", V4L2_PIX_FMT_YUYV, 16, 0 },
+#define ENC_UYVY 2
+ { "uyvy", V4L2_PIX_FMT_UYVY,  16, 0 },
+#define ENC_YV12 3
+ { "yv12", V4L2_PIX_FMT_YVU420, 12, 0 },
+#define ENC_LAST 4
+ { NULL, 0, 0, 0 }
 };
 
 struct video {
@@ -162,9 +167,12 @@ struct video {
  char iofile[FILENAME_MAX];
  int iofile_fd;
  char *sz_str;
+#define CONV_NONE 0x0
 #define CONV_SWAP 0x1
+#define CONV_YV12 0x2
  int conv_type;
  int enc;
+ int enc_xv;
  int full_screen;
  int net_wm;
  int width;
@@ -216,6 +224,7 @@ int stream(struct video *);
 void got_frame(int);
 void got_shutdown(int);
 int find_enc(char *);
+int find_by_id(int);
 void usage(void);
 
 static volatile sig_atomic_t play, shutdown, hold, wout;
@@ -242,6 +251,17 @@ find_enc(char *name)
 }
 
 int
+find_enc_by_id(int id)
+{
+ int i;
+
+ for (i = 0; i < ENC_LAST; i++)
+ if (encs[i].id == id)
+ break;
+ return i;
+}
+
+int
 xv_get_info(struct video *vid)
 {
  struct xdsp *x = &vid->xdsp;
@@ -250,8 +270,7 @@ xv_get_info(struct video *vid)
  XvEncodingInfo *xv_encs;
  struct xv_adap *adap;
  unsigned int nenc, p;
- int num_xvformats, nadaps, i, j, ret;
- char fmtName[5];
+ int num_xvformats, nadaps, i, j, ret, enc;
 
  if ((x->dpy = XOpenDisplay(NULL)) == NULL) {
  warnx("cannot open display %s", XDisplayName(NULL));
@@ -312,16 +331,9 @@ xv_get_info(struct video *vid)
     &num_xvformats);
  adap->nfmts = 0;
  for (j = 0; j < num_xvformats; j++) {
- snprintf(fmtName, sizeof(fmtName), "%c%c%c%c",
-    xvformats[j].id & 0xff,
-    (xvformats[j].id >> 8) & 0xff,
-    (xvformats[j].id >> 16) & 0xff,
-    (xvformats[j].id >> 24) & 0xff);
- if (!strcmp(fmtName, "YUY2")) {
- encs[ENC_YUY2].xv_id = xvformats[j].id;
- adap->fmts[adap->nfmts++] = xvformats[j].id;
- } else if (!strcmp(fmtName, "UYVY")) {
- encs[ENC_UYVY].xv_id = xvformats[j].id;
+ enc = find_enc_by_id(xvformats[j].id);
+ if (enc < ENC_LAST) {
+ encs[enc].flags |= SW_XV;
  adap->fmts[adap->nfmts++] = xvformats[j].id;
  }
  if (adap->nfmts >= MAX_FMTS)
@@ -369,20 +381,20 @@ xv_sel_adap(struct video *vid)
  x->cur_adap = i;
  adap = &x->adaps[i];
  for (i = 0; i < adap->nfmts; i++) {
- if (adap->fmts[i] == encs[vid->enc].xv_id)
+ if (adap->fmts[i] == encs[vid->enc_xv].id)
  break;
  }
  if (i >= adap->nfmts) {
  warnx("Xv adaptor '%d' doesn't support %s",
     x->adaps[x->cur_adap].adap_index,
-    encs[vid->enc].name);
+    encs[vid->enc_xv].name);
  return 0;
  }
  }
  for (i = 0; i < x->nadaps && x->cur_adap == -1; i++) {
  adap = &x->adaps[i];
  for (j = 0; j < adap->nfmts; j++) {
- if (adap->fmts[j] == encs[vid->enc].xv_id) {
+ if (adap->fmts[j] == encs[vid->enc_xv].id) {
  x->cur_adap = i;
  break;
  }
@@ -445,7 +457,7 @@ xv_dump_info(struct video *vid)
 
  fprintf(stderr, "  encodings: ");
  for (i = 0, j = 0; i < ENC_LAST; i++) {
- if (encs[i].xv_id != -1 && !(encs[i].flags & SW_XV)) {
+ if (encs[i].id != -1 && (encs[i].flags & SW_XV)) {
  if (j)
  fprintf(stderr, ", ");
  fprintf(stderr, "%s", encs[i].name);
@@ -502,7 +514,7 @@ xv_init(struct video *vid)
 
  resize_window(vid, 0);
 
- x->xv_image = XvCreateImage(x->dpy, x->port, encs[vid->enc].xv_id,
+ x->xv_image = XvCreateImage(x->dpy, x->port, encs[vid->enc_xv].id,
     vid->frame_buffer, vid->width, vid->height);
 
  return 1;
@@ -780,36 +792,20 @@ dev_get_encs(struct video *vid)
  fmtdesc.index = 0;
  fmtdesc.type = d->buf_type;
  while (ioctl(d->fd, VIDIOC_ENUM_FMT, &fmtdesc) >= 0) {
- if (fmtdesc.pixelformat == V4L2_PIX_FMT_YUYV) {
- i = find_enc("yuy2");
- if (i < ENC_LAST)
- encs[i].dev_id = fmtdesc.pixelformat;
- }
- if (fmtdesc.pixelformat == V4L2_PIX_FMT_UYVY) {
- i = find_enc("uyvy");
- if (i < ENC_LAST)
- encs[i].dev_id = fmtdesc.pixelformat;
+ i = find_enc_by_id(fmtdesc.pixelformat);
+ if (i < ENC_LAST) {
+ encs[i].flags |= SW_DEV;
  }
  fmtdesc.index++;
  }
  for (i = 0; encs[i].name; i++) {
- if (encs[i].dev_id != -1)
+ if (encs[i].flags & SW_DEV)
  break;
  }
  if (i >= ENC_LAST) {
  warnx("%s has no usable YUV encodings", d->path);
  return 0;
  }
- if (encs[ENC_YUY2].dev_id == -1 &&
-    encs[ENC_UYVY].dev_id != -1) {
- encs[ENC_YUY2].dev_id = encs[ENC_UYVY].dev_id;
- encs[ENC_YUY2].flags |= SW_DEV;
- }
- if (encs[ENC_UYVY].dev_id == -1 &&
-    encs[ENC_YUY2].dev_id != -1) {
- encs[ENC_UYVY].dev_id = encs[ENC_YUY2].dev_id;
- encs[ENC_UYVY].flags |= SW_DEV;
- }
 
  return 1;
 }
@@ -824,7 +820,7 @@ dev_get_sizes(struct video *vid)
 
  nsizes = 0;
  fsize.index = 0;
- fsize.pixel_format = encs[vid->enc].dev_id;
+ fsize.pixel_format = encs[vid->enc].id;
  while (ioctl(d->fd, VIDIOC_ENUM_FRAMESIZES, &fsize) == 0) {
  switch (fsize.type) {
  case V4L2_FRMSIZE_TYPE_DISCRETE:
@@ -915,7 +911,7 @@ dev_get_rates(struct video *vid)
 
  for (i = 0; i < d->nsizes; i++) {
  bzero(&ival, sizeof(ival));
- ival.pixel_format = encs[vid->enc].dev_id;
+ ival.pixel_format = encs[vid->enc].id;
  ival.width = d->sizes[i].w;
  ival.height = d->sizes[i].h;
  ival.index = 0;
@@ -1072,7 +1068,7 @@ dev_dump_info(struct video *vid)
 
  fprintf(stderr, "  encodings: ");
  for (i = 0, j = 0; i < ENC_LAST; i++) {
- if (encs[i].dev_id != -1 && !(encs[i].flags & SW_DEV)) {
+ if (encs[i].flags & SW_DEV) {
  if (j)
  fprintf(stderr, ", ");
  fprintf(stderr, "%s", encs[i].name);
@@ -1136,7 +1132,7 @@ dev_init(struct video *vid)
  fmt.type = d->buf_type;
  fmt.fmt.pix.width = vid->width;
  fmt.fmt.pix.height = vid->height;
- fmt.fmt.pix.pixelformat = encs[vid->enc].dev_id;
+ fmt.fmt.pix.pixelformat = encs[vid->enc].id;
  fmt.fmt.pix.field = V4L2_FIELD_ANY;
  if (ioctl(d->fd, VIDIOC_S_FMT, &fmt) < 0) {
  warn("VIDIOC_S_FMT");
@@ -1289,58 +1285,64 @@ int
 choose_enc(struct video *vid)
 {
  int i;
+ int enc, enc_xv;
 
  if (vid->enc < 0) {
  for (i = 0; vid->enc < 0 && i < ENC_LAST; i++) {
- if ((vid->mode & M_IN_DEV) && (vid->mode & M_OUT_XV)) {
- if (encs[i].dev_id != -1 &&
-    encs[i].xv_id != -1 &&
-    (encs[i].flags & SW_MASK) == 0)
- vid->enc = i;
- } else if (vid->mode & M_IN_DEV) {
- if (encs[i].dev_id != -1 &&
-    (encs[i].flags & SW_MASK) == 0)
- vid->enc = i;
- } else if (vid->mode & M_OUT_XV) {
- if (encs[i].xv_id != -1 &&
-    (encs[i].flags & SW_MASK) == 0)
- vid->enc = i;
- }
+ if (encs[i].flags & SW_DEV)
+ vid->enc = i;
+ }
+ }
+ if (vid->enc_xv < 0) {
+ for (i = 0; vid->enc_xv < 0 && i < ENC_LAST; i++) {
+ if (encs[i].flags & SW_XV)
+ vid->enc_xv = i;
  }
+ }
+ if ((vid->mode & M_IN_DEV) && (vid->mode & M_OUT_XV)) {
  for (i = 0; vid->enc < 0 && i < ENC_LAST; i++) {
- if ((vid->mode & M_IN_DEV) && (vid->mode & M_OUT_XV)) {
- if (encs[i].dev_id != -1 &&
-    encs[i].xv_id != -1)
- vid->enc = i;
- } else if (vid->mode & M_IN_DEV) {
- if (encs[i].dev_id != -1)
- vid->enc = i;
- } else if (vid->mode & M_OUT_XV) {
- if (encs[i].xv_id != -1)
- vid->enc = i;
+ if ((encs[i].flags & SW_DEV) &&
+    (encs[i].flags & SW_XV)) {
+ vid->enc = i;
+ vid->enc_xv = i;
  }
  }
  }
- if (vid->enc < 0) {
- warnx("could not find a usable encoding");
- return 0;
- }
- if ((vid->mode & M_IN_DEV) && encs[vid->enc].dev_id == -1) {
+ if (vid->enc < 0 && vid->mode & M_IN_FILE)
+ vid->enc = ENC_YUYV;
+ if ((vid->mode & M_IN_DEV) && !(encs[vid->enc].flags & SW_DEV)) {
  warnx("device %s can't supply %s", vid->dev.path,
     encs[vid->enc].name);
  return 0;
  }
- if ((vid->mode & M_OUT_XV) && encs[vid->enc].xv_id == -1) {
+ if (vid->mode & M_OUT_XV && vid->enc != vid->enc_xv) {
+ /* check if conversion is possible */
+ enc = (vid->enc == ENC_YUYV) ? ENC_YUY2 : vid->enc;
+ enc_xv = (vid->enc_xv == ENC_YUYV) ? ENC_YUY2 : vid->enc_xv;
+
+ if (enc == enc_xv)
+ vid->conv_type = CONV_NONE;
+ else if ((enc == ENC_YUY2 && enc_xv == ENC_UYVY) ||
+    (enc == ENC_UYVY && enc_xv == ENC_YUY2))
+ vid->conv_type = CONV_SWAP;
+ else if (enc == ENC_YUY2 && enc_xv == ENC_YV12)
+ vid->conv_type = CONV_YV12;
+ else {
+ warn("conversion from %s to %s is unsupported",
+    encs[vid->enc].name, encs[vid->enc_xv].name);
+ return 0;
+ }
+ }
+ if (vid->enc < 0) {
+ warnx("could not find a usable encoding");
+ return 0;
+ }
+ if ((vid->mode & M_OUT_XV) && !(encs[vid->enc_xv].flags & SW_XV)) {
  warnx("Xv adaptor %d can't display %s",
     vid->xdsp.adaps[vid->xdsp.cur_adap].adap_index,
-    encs[vid->enc].name);
+    encs[vid->enc_xv].name);
  return 0;
  }
- if (((vid->mode & M_IN_DEV) &&
-    (encs[vid->enc].flags & SW_MASK) == SW_DEV) ||
-    ((vid->mode & M_OUT_XV) &&
-    (encs[vid->enc].flags & SW_MASK) == SW_XV))
- vid->conv_type = CONV_SWAP;
 
  return 1;
 }
@@ -1491,12 +1493,8 @@ setup(struct video *vid)
  }
 
  if (vid->conv_type) {
- if (vid->conv_type == CONV_SWAP) {
- vid->conv_bufsz = vid->bpf;
- } else {
- warnx("invalid conversion type");
- return 0;
- }
+ vid->conv_bufsz =
+    (vid->width * vid->height * encs[vid->enc_xv].bpp) / NBBY;
  if ((vid->conv_buffer = calloc(1, vid->conv_bufsz)) == NULL) {
  warn("conv_buffer");
  return 0;
@@ -1633,6 +1631,34 @@ got_shutdown(int s)
 }
 
 int
+yuy2_to_yv12(uint8_t *src, uint8_t *dst, int width, int height)
+{
+ int row, col, c, o;
+ int iy = 0;
+ int iu = width * height;
+ int iv = iu + iu / 4;
+
+ for (row = 0; row < height * 2; row += 2) {
+ c = row * width;
+ for (col = 0; col < width * 2 ; col += 4) {
+ o = c + col;
+ dst[iy++] = src[o];
+ dst[iy++] = src[o + 2];
+ if ((row & 0x03)  == 0) {
+ o++;
+ dst[iv++] = (src[o] + src[o + width * 2]) / 2;
+ o += 2;
+ dst[iu++] = (src[o] + src[o + width * 2]) / 2;
+ if (o + width * 2 >= iu*2)
+ printf("hello\n");
+ }
+ }
+ }
+
+ return 0;
+}
+
+int
 stream(struct video *vid)
 {
  struct xdsp *x = &vid->xdsp;
@@ -1721,7 +1747,9 @@ stream(struct video *vid)
  src = vid->frame_buffer;
  if (vid->conv_type == CONV_SWAP) {
  swab(src, vid->conv_buffer, vid->bpf);
- src = vid->conv_buffer;
+ } else if (vid->conv_type == CONV_YV12) {
+ yuy2_to_yv12(src, vid->conv_buffer,
+    vid->width, vid->height);
  }
 
  if ((vid->mode & M_OUT_FILE) && wout) {
@@ -1743,6 +1771,8 @@ stream(struct video *vid)
  }
  }
  if (vid->mode & M_OUT_XV) {
+ src = (vid->conv_type) ?
+    vid->conv_buffer : vid->frame_buffer;
  x->xv_image->data = src;
  if (x->resized) {
  x->resized = 0;
@@ -1863,6 +1893,7 @@ main(int argc, char *argv[])
  vid.dev.fd = vid.iofile_fd = -1;
  vid.mode = M_IN_DEV | M_OUT_XV;
  vid.enc = -1;
+ vid.enc_xv = -1;
  vid.mmap_on = 1; /* mmap method is default */
  wout = 1;
 

Reply | Threaded
Open this post in threaded view
|

Re: video(1) and modesetting driver

Artturi Alm
On Fri, Jan 25, 2019 at 11:16:49AM +0100, Raphael Graf wrote:

> Hi Martin,
> Thanks for your feedback!
>
> On Wed, Jan 23, 2019 at 12:54:51PM -0200, Martin Pieuchot wrote:
> > Hello Raphael,
> >
> > On 16/01/19(Wed) 12:41, Raphael Graf wrote:
> > > Here is an attempt to make video(1) work with the modesetting driver.
> > > See https://marc.info/?l=openbsd-bugs&m=152231686416039&w=2
> > >
> > > The general idea:
> > > If there is no common encoding for input (device) and output (Xv), the
> > > encoding is converted to something supported by the output.
> > > No conversion is done if the output is sent to file (options -o/-O).
> > >
> > > The option -e is now used as the input format. The output format
> > > is selected automatically (if output goes to Xv).
> > >
> > > The following encodings are supported: yuy2, yuyv, uyuy, yv12
> >
> > Did you test all of them?
>
> Yes, I have tested all encodings with these two devices:
>
> uvideo0 at uhub0 port 6 configuration 1 interface 0
> "Chicony Electronics Co., Ltd. product 0x480c" rev 2.00/31.34 addr 2
>
> uvideo0 at uhub1 port 8 configuration 1 interface 0 "SC20A38485AA4629RX
> Integrated Camera" rev 2.00/0.21 addr 4
>
> >
> > > yuy2 and yuyv are treated as two different encodings, they share the same
> > > pixelformat, but have different ids. (This behaviour is different from the
> > > current description of '-e' in the manpage)
> >
> > What about fixing the manpage then? ;)
>
> The new diff below contains the changes to the manpage.
>
> >
> > > I have tested on two lenovo laptops using both drivers (modesetting and intel).
> > >
> > > On my laptop (lenovo X1 Carbon), with modesetting diver:
> > > yuyv is converted to yv12 before sending to Xv.
> > >
> > > On the same laptop With intel driver:
> > > yuyv is converted to yuy2 (no-op)
> >
> > Nice work, some comments below.
> >
> > Make sure the code you're writing follows style(9) :o)
> >
> > > Index: video.c
> > > ===================================================================
> > > RCS file: /cvs/xenocara/app/video/video.c,v
> > > retrieving revision 1.26
> > > diff -u -p -u -p -r1.26 video.c
> > > --- video.c 4 Jan 2019 17:45:00 -0000 1.26
> > > +++ video.c 16 Jan 2019 11:30:21 -0000
> > > @@ -38,6 +38,8 @@
> > >  #include <X11/extensions/Xvlib.h>
> > >  #include <X11/Xatom.h>
> > >
> > > +#define V_V4L2_PIX_FMT_YUY2    v4l2_fourcc('Y', 'U', 'Y', '2')
> >
> > Why do we need this define and not the others?  Aren't we missing an
> > include?
>
> Actually, I don't know why this define is missing in videoio.h.
> This YUY2 format (id:0x32595559) is reported by Xv.
> The uvideo(4) driver reports YUYV which has a define in videoio.h.
> (YUY2 and YUYV are equivalent)
>
> In the new diff, I have changed the variable names enc_in/enc_out to enc/enc_xv
> as the second 'enc' is only used for Xv output.
> I also fixed the style(9) issues and added a CONV_NONE define.
>
> Note that I'm not sure if the algorithm used in the yuy2_to_yv12 function is
> correct in respect to color theory..
> (It is a conversion from 16 to 12 bits/pixel)
>
> Sorry, it is still a big diff, feedback is welcome.
>
>

Hi,

got one question below.

-Artturi

[...snip...]

> @@ -1633,6 +1631,34 @@ got_shutdown(int s)
>  }
>  
>  int
> +yuy2_to_yv12(uint8_t *src, uint8_t *dst, int width, int height)
> +{
> + int row, col, c, o;
> + int iy = 0;
> + int iu = width * height;
> + int iv = iu + iu / 4;
> +
> + for (row = 0; row < height * 2; row += 2) {
> + c = row * width;
> + for (col = 0; col < width * 2 ; col += 4) {
> + o = c + col;
> + dst[iy++] = src[o];
> + dst[iy++] = src[o + 2];
> + if ((row & 0x03)  == 0) {
> + o++;
> + dst[iv++] = (src[o] + src[o + width * 2]) / 2;
> + o += 2;
> + dst[iu++] = (src[o] + src[o + width * 2]) / 2;
> + if (o + width * 2 >= iu*2)
> + printf("hello\n");

is that a placeholder for something relevant, or what does hello mean here?

> + }
> + }
> + }
> +
> + return 0;
> +}
> +
> +int
>  stream(struct video *vid)
>  {
>   struct xdsp *x = &vid->xdsp;

[...snip...]

Reply | Threaded
Open this post in threaded view
|

Re: video(1) and modesetting driver

Martin Pieuchot
In reply to this post by Raphael Graf-2
On 25/01/19(Fri) 11:16, Raphael Graf wrote:

> On Wed, Jan 23, 2019 at 12:54:51PM -0200, Martin Pieuchot wrote:
> > > Index: video.c
> > > ===================================================================
> > > RCS file: /cvs/xenocara/app/video/video.c,v
> > > retrieving revision 1.26
> > > diff -u -p -u -p -r1.26 video.c
> > > --- video.c 4 Jan 2019 17:45:00 -0000 1.26
> > > +++ video.c 16 Jan 2019 11:30:21 -0000
> > > @@ -38,6 +38,8 @@
> > >  #include <X11/extensions/Xvlib.h>
> > >  #include <X11/Xatom.h>
> > >
> > > +#define V_V4L2_PIX_FMT_YUY2    v4l2_fourcc('Y', 'U', 'Y', '2')
> >
> > Why do we need this define and not the others?  Aren't we missing an
> > include?
>
> Actually, I don't know why this define is missing in videoio.h.
> This YUY2 format (id:0x32595559) is reported by Xv.
> The uvideo(4) driver reports YUYV which has a define in videoio.h.
> (YUY2 and YUYV are equivalent)

I see.  I believe you're being too clever her.  XvListImageFormats(3)
returns you a list with an Id in FourCC format and you're using V4L2
defines to match these IDs.

I'd suggest you define only one encoding "yuy2" or "yuyv" and make the
matching function, find_enc_by_id() a bit more clever.  That would
explicitly document that Xv and V4L2 use different names for the same
format.  On top of that you'll get rid of your conversion code :)
 
> In the new diff, I have changed the variable names enc_in/enc_out to enc/enc_xv
> as the second 'enc' is only used for Xv output.
> I also fixed the style(9) issues and added a CONV_NONE define.

I find enc_in/enc_out more explicit, but let's keep it like that for
now.  This also makes the diff smaller :)

> Note that I'm not sure if the algorithm used in the yuy2_to_yv12 function is
> correct in respect to color theory..
> (It is a conversion from 16 to 12 bits/pixel)

I'm sure you can find some good documentation and/or example about that
on the Web or in the existing freesoftware ecosystem.

> Index: video.1
> ===================================================================
> RCS file: /cvs/xenocara/app/video/video.1,v
> retrieving revision 1.13
> diff -u -p -u -p -r1.13 video.1
> --- video.1 4 Jun 2016 07:44:32 -0000 1.13
> +++ video.1 25 Jan 2019 09:51:12 -0000
> @@ -84,18 +84,19 @@ The default is 0, the first adaptor repo
>  .It Fl e Ar encoding
>  Lowercase FOURCC name of video encoding to use.
>  Valid arguments are
> +.Ql yuyv ,
> +.Ql yuy2 ,
>  .Ql uyvy
>  and
> -.Ql yuy2 .
> +.Ql yv12 .
>  The default is
> -.Ql yuy2
> -unless
> -.Ar file
> -is being used and only supports
> -.Ql uyvy ,
> -in which case
> -.Ql uyvy
> -will be used by default.
> +.Ql yuyv .
> +If reading from a
> +.Xr video 4
> +device and
> +.Ql yuyv
> +is not supported, the default is
> +.Ql uyvy .
>  .It Fl f Ar file
>  .Xr video 4
>  device from which frames will be read.
> @@ -338,6 +339,6 @@ Note that with the first three commands,
>  does not support 640x480 pixels sized frames, the largest frame size
>  smaller than 640x480 will be used, and if
>  .Ar /dev/video
> -does not support yuy2 encoding, uyvy will be used.
> +does not support yuyv encoding, uyvy will be used.
>  .Sh SEE ALSO
>  .Xr video 4
> Index: video.c
> ===================================================================
> RCS file: /cvs/xenocara/app/video/video.c,v
> retrieving revision 1.27
> diff -u -p -u -p -r1.27 video.c
> --- video.c 22 Jan 2019 20:02:40 -0000 1.27
> +++ video.c 25 Jan 2019 09:51:12 -0000
> @@ -38,6 +38,8 @@
>  #include <X11/extensions/Xvlib.h>
>  #include <X11/Xatom.h>
>  
> +#define V_V4L2_PIX_FMT_YUY2    v4l2_fourcc('Y', 'U', 'Y', '2')
> +
>  /* Xv(3) adaptor properties */
>  struct xv_adap {
>   char name[128];
> @@ -133,20 +135,23 @@ struct dev {
>  /* video encodingss */
>  struct encodings {
>   char *name;
> + int id;
>   int bpp;
> - int xv_id;
> - int dev_id;
>  #define SW_DEV 0x1
>  #define SW_XV 0x2
>  #define SW_MASK (SW_DEV | SW_XV)
>   int flags;
>  } encs[] = {
>  #define ENC_YUY2 0
> - { "yuy2", 16, -1, -1, 0 },
> -#define ENC_UYVY 1
> - { "uyvy", 16, -1, -1, 0 },
> -#define ENC_LAST 2
> - { NULL, 0, 0, 0, 0 }
> + { "yuy2", V_V4L2_PIX_FMT_YUY2, 16, 0 },
> +#define ENC_YUYV 1
> + { "yuyv", V4L2_PIX_FMT_YUYV, 16, 0 },
> +#define ENC_UYVY 2
> + { "uyvy", V4L2_PIX_FMT_UYVY,  16, 0 },
> +#define ENC_YV12 3
> + { "yv12", V4L2_PIX_FMT_YVU420, 12, 0 },
> +#define ENC_LAST 4
> + { NULL, 0, 0, 0 }
>  };
>  
>  struct video {
> @@ -162,9 +167,12 @@ struct video {
>   char iofile[FILENAME_MAX];
>   int iofile_fd;
>   char *sz_str;
> +#define CONV_NONE 0x0
>  #define CONV_SWAP 0x1
> +#define CONV_YV12 0x2
>   int conv_type;
>   int enc;
> + int enc_xv;
>   int full_screen;
>   int net_wm;
>   int width;
> @@ -216,6 +224,7 @@ int stream(struct video *);
>  void got_frame(int);
>  void got_shutdown(int);
>  int find_enc(char *);
> +int find_by_id(int);
>  void usage(void);
>  
>  static volatile sig_atomic_t play, shutdown, hold, wout;
> @@ -242,6 +251,17 @@ find_enc(char *name)
>  }
>  
>  int
> +find_enc_by_id(int id)
> +{
> + int i;
> +
> + for (i = 0; i < ENC_LAST; i++)
> + if (encs[i].id == id)
> + break;
> + return i;
> +}
> +
> +int
>  xv_get_info(struct video *vid)
>  {
>   struct xdsp *x = &vid->xdsp;
> @@ -250,8 +270,7 @@ xv_get_info(struct video *vid)
>   XvEncodingInfo *xv_encs;
>   struct xv_adap *adap;
>   unsigned int nenc, p;
> - int num_xvformats, nadaps, i, j, ret;
> - char fmtName[5];
> + int num_xvformats, nadaps, i, j, ret, enc;
>  
>   if ((x->dpy = XOpenDisplay(NULL)) == NULL) {
>   warnx("cannot open display %s", XDisplayName(NULL));
> @@ -312,16 +331,9 @@ xv_get_info(struct video *vid)
>      &num_xvformats);
>   adap->nfmts = 0;
>   for (j = 0; j < num_xvformats; j++) {
> - snprintf(fmtName, sizeof(fmtName), "%c%c%c%c",
> -    xvformats[j].id & 0xff,
> -    (xvformats[j].id >> 8) & 0xff,
> -    (xvformats[j].id >> 16) & 0xff,
> -    (xvformats[j].id >> 24) & 0xff);
> - if (!strcmp(fmtName, "YUY2")) {
> - encs[ENC_YUY2].xv_id = xvformats[j].id;
> - adap->fmts[adap->nfmts++] = xvformats[j].id;
> - } else if (!strcmp(fmtName, "UYVY")) {
> - encs[ENC_UYVY].xv_id = xvformats[j].id;
> + enc = find_enc_by_id(xvformats[j].id);
> + if (enc < ENC_LAST) {
> + encs[enc].flags |= SW_XV;
>   adap->fmts[adap->nfmts++] = xvformats[j].id;
>   }
>   if (adap->nfmts >= MAX_FMTS)
> @@ -369,20 +381,20 @@ xv_sel_adap(struct video *vid)
>   x->cur_adap = i;
>   adap = &x->adaps[i];
>   for (i = 0; i < adap->nfmts; i++) {
> - if (adap->fmts[i] == encs[vid->enc].xv_id)
> + if (adap->fmts[i] == encs[vid->enc_xv].id)
>   break;
>   }
>   if (i >= adap->nfmts) {
>   warnx("Xv adaptor '%d' doesn't support %s",
>      x->adaps[x->cur_adap].adap_index,
> -    encs[vid->enc].name);
> +    encs[vid->enc_xv].name);
>   return 0;
>   }
>   }
>   for (i = 0; i < x->nadaps && x->cur_adap == -1; i++) {
>   adap = &x->adaps[i];
>   for (j = 0; j < adap->nfmts; j++) {
> - if (adap->fmts[j] == encs[vid->enc].xv_id) {
> + if (adap->fmts[j] == encs[vid->enc_xv].id) {
>   x->cur_adap = i;
>   break;
>   }
> @@ -445,7 +457,7 @@ xv_dump_info(struct video *vid)
>  
>   fprintf(stderr, "  encodings: ");
>   for (i = 0, j = 0; i < ENC_LAST; i++) {
> - if (encs[i].xv_id != -1 && !(encs[i].flags & SW_XV)) {
> + if (encs[i].id != -1 && (encs[i].flags & SW_XV)) {
>   if (j)
>   fprintf(stderr, ", ");
>   fprintf(stderr, "%s", encs[i].name);
> @@ -502,7 +514,7 @@ xv_init(struct video *vid)
>  
>   resize_window(vid, 0);
>  
> - x->xv_image = XvCreateImage(x->dpy, x->port, encs[vid->enc].xv_id,
> + x->xv_image = XvCreateImage(x->dpy, x->port, encs[vid->enc_xv].id,
>      vid->frame_buffer, vid->width, vid->height);
>  
>   return 1;
> @@ -780,36 +792,20 @@ dev_get_encs(struct video *vid)
>   fmtdesc.index = 0;
>   fmtdesc.type = d->buf_type;
>   while (ioctl(d->fd, VIDIOC_ENUM_FMT, &fmtdesc) >= 0) {
> - if (fmtdesc.pixelformat == V4L2_PIX_FMT_YUYV) {
> - i = find_enc("yuy2");
> - if (i < ENC_LAST)
> - encs[i].dev_id = fmtdesc.pixelformat;
> - }
> - if (fmtdesc.pixelformat == V4L2_PIX_FMT_UYVY) {
> - i = find_enc("uyvy");
> - if (i < ENC_LAST)
> - encs[i].dev_id = fmtdesc.pixelformat;
> + i = find_enc_by_id(fmtdesc.pixelformat);
> + if (i < ENC_LAST) {
> + encs[i].flags |= SW_DEV;
>   }
>   fmtdesc.index++;
>   }
>   for (i = 0; encs[i].name; i++) {
> - if (encs[i].dev_id != -1)
> + if (encs[i].flags & SW_DEV)
>   break;
>   }
>   if (i >= ENC_LAST) {
>   warnx("%s has no usable YUV encodings", d->path);
>   return 0;
>   }
> - if (encs[ENC_YUY2].dev_id == -1 &&
> -    encs[ENC_UYVY].dev_id != -1) {
> - encs[ENC_YUY2].dev_id = encs[ENC_UYVY].dev_id;
> - encs[ENC_YUY2].flags |= SW_DEV;
> - }
> - if (encs[ENC_UYVY].dev_id == -1 &&
> -    encs[ENC_YUY2].dev_id != -1) {
> - encs[ENC_UYVY].dev_id = encs[ENC_YUY2].dev_id;
> - encs[ENC_UYVY].flags |= SW_DEV;
> - }
>  
>   return 1;
>  }
> @@ -824,7 +820,7 @@ dev_get_sizes(struct video *vid)
>  
>   nsizes = 0;
>   fsize.index = 0;
> - fsize.pixel_format = encs[vid->enc].dev_id;
> + fsize.pixel_format = encs[vid->enc].id;
>   while (ioctl(d->fd, VIDIOC_ENUM_FRAMESIZES, &fsize) == 0) {
>   switch (fsize.type) {
>   case V4L2_FRMSIZE_TYPE_DISCRETE:
> @@ -915,7 +911,7 @@ dev_get_rates(struct video *vid)
>  
>   for (i = 0; i < d->nsizes; i++) {
>   bzero(&ival, sizeof(ival));
> - ival.pixel_format = encs[vid->enc].dev_id;
> + ival.pixel_format = encs[vid->enc].id;
>   ival.width = d->sizes[i].w;
>   ival.height = d->sizes[i].h;
>   ival.index = 0;
> @@ -1072,7 +1068,7 @@ dev_dump_info(struct video *vid)
>  
>   fprintf(stderr, "  encodings: ");
>   for (i = 0, j = 0; i < ENC_LAST; i++) {
> - if (encs[i].dev_id != -1 && !(encs[i].flags & SW_DEV)) {
> + if (encs[i].flags & SW_DEV) {
>   if (j)
>   fprintf(stderr, ", ");
>   fprintf(stderr, "%s", encs[i].name);
> @@ -1136,7 +1132,7 @@ dev_init(struct video *vid)
>   fmt.type = d->buf_type;
>   fmt.fmt.pix.width = vid->width;
>   fmt.fmt.pix.height = vid->height;
> - fmt.fmt.pix.pixelformat = encs[vid->enc].dev_id;
> + fmt.fmt.pix.pixelformat = encs[vid->enc].id;
>   fmt.fmt.pix.field = V4L2_FIELD_ANY;
>   if (ioctl(d->fd, VIDIOC_S_FMT, &fmt) < 0) {
>   warn("VIDIOC_S_FMT");
> @@ -1289,58 +1285,64 @@ int
>  choose_enc(struct video *vid)
>  {
>   int i;
> + int enc, enc_xv;
>  
>   if (vid->enc < 0) {
>   for (i = 0; vid->enc < 0 && i < ENC_LAST; i++) {
> - if ((vid->mode & M_IN_DEV) && (vid->mode & M_OUT_XV)) {
> - if (encs[i].dev_id != -1 &&
> -    encs[i].xv_id != -1 &&
> -    (encs[i].flags & SW_MASK) == 0)
> - vid->enc = i;
> - } else if (vid->mode & M_IN_DEV) {
> - if (encs[i].dev_id != -1 &&
> -    (encs[i].flags & SW_MASK) == 0)
> - vid->enc = i;
> - } else if (vid->mode & M_OUT_XV) {
> - if (encs[i].xv_id != -1 &&
> -    (encs[i].flags & SW_MASK) == 0)
> - vid->enc = i;
> - }
> + if (encs[i].flags & SW_DEV)
> + vid->enc = i;
> + }
> + }
> + if (vid->enc_xv < 0) {
> + for (i = 0; vid->enc_xv < 0 && i < ENC_LAST; i++) {
> + if (encs[i].flags & SW_XV)
> + vid->enc_xv = i;
>   }
> + }
> + if ((vid->mode & M_IN_DEV) && (vid->mode & M_OUT_XV)) {
>   for (i = 0; vid->enc < 0 && i < ENC_LAST; i++) {
> - if ((vid->mode & M_IN_DEV) && (vid->mode & M_OUT_XV)) {
> - if (encs[i].dev_id != -1 &&
> -    encs[i].xv_id != -1)
> - vid->enc = i;
> - } else if (vid->mode & M_IN_DEV) {
> - if (encs[i].dev_id != -1)
> - vid->enc = i;
> - } else if (vid->mode & M_OUT_XV) {
> - if (encs[i].xv_id != -1)
> - vid->enc = i;
> + if ((encs[i].flags & SW_DEV) &&
> +    (encs[i].flags & SW_XV)) {
> + vid->enc = i;
> + vid->enc_xv = i;
>   }
>   }
>   }
> - if (vid->enc < 0) {
> - warnx("could not find a usable encoding");
> - return 0;
> - }
> - if ((vid->mode & M_IN_DEV) && encs[vid->enc].dev_id == -1) {
> + if (vid->enc < 0 && vid->mode & M_IN_FILE)
> + vid->enc = ENC_YUYV;
> + if ((vid->mode & M_IN_DEV) && !(encs[vid->enc].flags & SW_DEV)) {
>   warnx("device %s can't supply %s", vid->dev.path,
>      encs[vid->enc].name);
>   return 0;
>   }
> - if ((vid->mode & M_OUT_XV) && encs[vid->enc].xv_id == -1) {
> + if (vid->mode & M_OUT_XV && vid->enc != vid->enc_xv) {
> + /* check if conversion is possible */
> + enc = (vid->enc == ENC_YUYV) ? ENC_YUY2 : vid->enc;
> + enc_xv = (vid->enc_xv == ENC_YUYV) ? ENC_YUY2 : vid->enc_xv;
> +
> + if (enc == enc_xv)
> + vid->conv_type = CONV_NONE;
> + else if ((enc == ENC_YUY2 && enc_xv == ENC_UYVY) ||
> +    (enc == ENC_UYVY && enc_xv == ENC_YUY2))
> + vid->conv_type = CONV_SWAP;
> + else if (enc == ENC_YUY2 && enc_xv == ENC_YV12)
> + vid->conv_type = CONV_YV12;
> + else {
> + warn("conversion from %s to %s is unsupported",
> +    encs[vid->enc].name, encs[vid->enc_xv].name);
> + return 0;
> + }
> + }
> + if (vid->enc < 0) {
> + warnx("could not find a usable encoding");
> + return 0;
> + }
> + if ((vid->mode & M_OUT_XV) && !(encs[vid->enc_xv].flags & SW_XV)) {
>   warnx("Xv adaptor %d can't display %s",
>      vid->xdsp.adaps[vid->xdsp.cur_adap].adap_index,
> -    encs[vid->enc].name);
> +    encs[vid->enc_xv].name);
>   return 0;
>   }
> - if (((vid->mode & M_IN_DEV) &&
> -    (encs[vid->enc].flags & SW_MASK) == SW_DEV) ||
> -    ((vid->mode & M_OUT_XV) &&
> -    (encs[vid->enc].flags & SW_MASK) == SW_XV))
> - vid->conv_type = CONV_SWAP;
>  
>   return 1;
>  }
> @@ -1491,12 +1493,8 @@ setup(struct video *vid)
>   }
>  
>   if (vid->conv_type) {
> - if (vid->conv_type == CONV_SWAP) {
> - vid->conv_bufsz = vid->bpf;
> - } else {
> - warnx("invalid conversion type");
> - return 0;
> - }
> + vid->conv_bufsz =
> +    (vid->width * vid->height * encs[vid->enc_xv].bpp) / NBBY;
>   if ((vid->conv_buffer = calloc(1, vid->conv_bufsz)) == NULL) {
>   warn("conv_buffer");
>   return 0;
> @@ -1633,6 +1631,34 @@ got_shutdown(int s)
>  }
>  
>  int
> +yuy2_to_yv12(uint8_t *src, uint8_t *dst, int width, int height)
> +{
> + int row, col, c, o;
> + int iy = 0;
> + int iu = width * height;
> + int iv = iu + iu / 4;
> +
> + for (row = 0; row < height * 2; row += 2) {
> + c = row * width;
> + for (col = 0; col < width * 2 ; col += 4) {
> + o = c + col;
> + dst[iy++] = src[o];
> + dst[iy++] = src[o + 2];
> + if ((row & 0x03)  == 0) {
> + o++;
> + dst[iv++] = (src[o] + src[o + width * 2]) / 2;
> + o += 2;
> + dst[iu++] = (src[o] + src[o + width * 2]) / 2;
> + if (o + width * 2 >= iu*2)
> + printf("hello\n");
> + }
> + }
> + }
> +
> + return 0;
> +}
> +
> +int
>  stream(struct video *vid)
>  {
>   struct xdsp *x = &vid->xdsp;
> @@ -1721,7 +1747,9 @@ stream(struct video *vid)
>   src = vid->frame_buffer;
>   if (vid->conv_type == CONV_SWAP) {
>   swab(src, vid->conv_buffer, vid->bpf);
> - src = vid->conv_buffer;
> + } else if (vid->conv_type == CONV_YV12) {
> + yuy2_to_yv12(src, vid->conv_buffer,
> +    vid->width, vid->height);
>   }
>  
>   if ((vid->mode & M_OUT_FILE) && wout) {
> @@ -1743,6 +1771,8 @@ stream(struct video *vid)
>   }
>   }
>   if (vid->mode & M_OUT_XV) {
> + src = (vid->conv_type) ?
> +    vid->conv_buffer : vid->frame_buffer;
>   x->xv_image->data = src;
>   if (x->resized) {
>   x->resized = 0;
> @@ -1863,6 +1893,7 @@ main(int argc, char *argv[])
>   vid.dev.fd = vid.iofile_fd = -1;
>   vid.mode = M_IN_DEV | M_OUT_XV;
>   vid.enc = -1;
> + vid.enc_xv = -1;
>   vid.mmap_on = 1; /* mmap method is default */
>   wout = 1;
>  
>

Reply | Threaded
Open this post in threaded view
|

Re: video(1) and modesetting driver

Raphael Graf-2
On Fri, Jan 25, 2019 at 09:09:29AM -0200, Martin Pieuchot wrote:

> On 25/01/19(Fri) 11:16, Raphael Graf wrote:
> > On Wed, Jan 23, 2019 at 12:54:51PM -0200, Martin Pieuchot wrote:
> > > > Index: video.c
> > > > ===================================================================
> > > > RCS file: /cvs/xenocara/app/video/video.c,v
> > > > retrieving revision 1.26
> > > > diff -u -p -u -p -r1.26 video.c
> > > > --- video.c 4 Jan 2019 17:45:00 -0000 1.26
> > > > +++ video.c 16 Jan 2019 11:30:21 -0000
> > > > @@ -38,6 +38,8 @@
> > > >  #include <X11/extensions/Xvlib.h>
> > > >  #include <X11/Xatom.h>
> > > >
> > > > +#define V_V4L2_PIX_FMT_YUY2    v4l2_fourcc('Y', 'U', 'Y', '2')
> > >
> > > Why do we need this define and not the others?  Aren't we missing an
> > > include?
> >
> > Actually, I don't know why this define is missing in videoio.h.
> > This YUY2 format (id:0x32595559) is reported by Xv.
> > The uvideo(4) driver reports YUYV which has a define in videoio.h.
> > (YUY2 and YUYV are equivalent)
>
> I see.  I believe you're being too clever her.  XvListImageFormats(3)
> returns you a list with an Id in FourCC format and you're using V4L2
> defines to match these IDs.
>
> I'd suggest you define only one encoding "yuy2" or "yuyv" and make the
> matching function, find_enc_by_id() a bit more clever.  That would
> explicitly document that Xv and V4L2 use different names for the same
> format.  On top of that you'll get rid of your conversion code :)
>  

I actually tried to do this, but it's not trivial.
The current code had only one entry for yuy2/yuyv in 'encs[]' and handled the
situation by having xv_id and dev_id in 'struct encodings':

struct encodings {
        char    *name;
        int      bpp;
        int      xv_id;
        int      dev_id;
        int      flags;
}  

But this approach does not work when 'bpp' changes for the output.
There is also the case of '-O' where output goes to 'Xv' and 'file'
simultanously, using different encodings for the two outputs.
When using the modesetting driver, Xv output needs yv12, and the file is
written in yuy2.

(It would be possible to add xv_bpp and dev_bpp to 'struct encodings', but this
would be a bit hacky, I think)

> > In the new diff, I have changed the variable names enc_in/enc_out to enc/enc_xv
> > as the second 'enc' is only used for Xv output.
> > I also fixed the style(9) issues and added a CONV_NONE define.
>
> I find enc_in/enc_out more explicit, but let's keep it like that for
> now.  This also makes the diff smaller :)
>
> > Note that I'm not sure if the algorithm used in the yuy2_to_yv12 function is
> > correct in respect to color theory..
> > (It is a conversion from 16 to 12 bits/pixel)
>
> I'm sure you can find some good documentation and/or example about that
> on the Web or in the existing freesoftware ecosystem.

I'll do some more research on this topic.

>
> > Index: video.1
> > ===================================================================
> > RCS file: /cvs/xenocara/app/video/video.1,v
> > retrieving revision 1.13
> > diff -u -p -u -p -r1.13 video.1
> > --- video.1 4 Jun 2016 07:44:32 -0000 1.13
> > +++ video.1 25 Jan 2019 09:51:12 -0000
> > @@ -84,18 +84,19 @@ The default is 0, the first adaptor repo
> >  .It Fl e Ar encoding
> >  Lowercase FOURCC name of video encoding to use.
> >  Valid arguments are
> > +.Ql yuyv ,
> > +.Ql yuy2 ,
> >  .Ql uyvy
> >  and
> > -.Ql yuy2 .
> > +.Ql yv12 .
> >  The default is
> > -.Ql yuy2
> > -unless
> > -.Ar file
> > -is being used and only supports
> > -.Ql uyvy ,
> > -in which case
> > -.Ql uyvy
> > -will be used by default.
> > +.Ql yuyv .
> > +If reading from a
> > +.Xr video 4
> > +device and
> > +.Ql yuyv
> > +is not supported, the default is
> > +.Ql uyvy .
> >  .It Fl f Ar file
> >  .Xr video 4
> >  device from which frames will be read.
> > @@ -338,6 +339,6 @@ Note that with the first three commands,
> >  does not support 640x480 pixels sized frames, the largest frame size
> >  smaller than 640x480 will be used, and if
> >  .Ar /dev/video
> > -does not support yuy2 encoding, uyvy will be used.
> > +does not support yuyv encoding, uyvy will be used.
> >  .Sh SEE ALSO
> >  .Xr video 4
> > Index: video.c
> > ===================================================================
> > RCS file: /cvs/xenocara/app/video/video.c,v
> > retrieving revision 1.27
> > diff -u -p -u -p -r1.27 video.c
> > --- video.c 22 Jan 2019 20:02:40 -0000 1.27
> > +++ video.c 25 Jan 2019 09:51:12 -0000
> > @@ -38,6 +38,8 @@
> >  #include <X11/extensions/Xvlib.h>
> >  #include <X11/Xatom.h>
> >  
> > +#define V_V4L2_PIX_FMT_YUY2    v4l2_fourcc('Y', 'U', 'Y', '2')
> > +
> >  /* Xv(3) adaptor properties */
> >  struct xv_adap {
> >   char name[128];
> > @@ -133,20 +135,23 @@ struct dev {
> >  /* video encodingss */
> >  struct encodings {
> >   char *name;
> > + int id;
> >   int bpp;
> > - int xv_id;
> > - int dev_id;
> >  #define SW_DEV 0x1
> >  #define SW_XV 0x2
> >  #define SW_MASK (SW_DEV | SW_XV)
> >   int flags;
> >  } encs[] = {
> >  #define ENC_YUY2 0
> > - { "yuy2", 16, -1, -1, 0 },
> > -#define ENC_UYVY 1
> > - { "uyvy", 16, -1, -1, 0 },
> > -#define ENC_LAST 2
> > - { NULL, 0, 0, 0, 0 }
> > + { "yuy2", V_V4L2_PIX_FMT_YUY2, 16, 0 },
> > +#define ENC_YUYV 1
> > + { "yuyv", V4L2_PIX_FMT_YUYV, 16, 0 },
> > +#define ENC_UYVY 2
> > + { "uyvy", V4L2_PIX_FMT_UYVY,  16, 0 },
> > +#define ENC_YV12 3
> > + { "yv12", V4L2_PIX_FMT_YVU420, 12, 0 },
> > +#define ENC_LAST 4
> > + { NULL, 0, 0, 0 }
> >  };
> >  
> >  struct video {
> > @@ -162,9 +167,12 @@ struct video {
> >   char iofile[FILENAME_MAX];
> >   int iofile_fd;
> >   char *sz_str;
> > +#define CONV_NONE 0x0
> >  #define CONV_SWAP 0x1
> > +#define CONV_YV12 0x2
> >   int conv_type;
> >   int enc;
> > + int enc_xv;
> >   int full_screen;
> >   int net_wm;
> >   int width;
> > @@ -216,6 +224,7 @@ int stream(struct video *);
> >  void got_frame(int);
> >  void got_shutdown(int);
> >  int find_enc(char *);
> > +int find_by_id(int);
> >  void usage(void);
> >  
> >  static volatile sig_atomic_t play, shutdown, hold, wout;
> > @@ -242,6 +251,17 @@ find_enc(char *name)
> >  }
> >  
> >  int
> > +find_enc_by_id(int id)
> > +{
> > + int i;
> > +
> > + for (i = 0; i < ENC_LAST; i++)
> > + if (encs[i].id == id)
> > + break;
> > + return i;
> > +}
> > +
> > +int
> >  xv_get_info(struct video *vid)
> >  {
> >   struct xdsp *x = &vid->xdsp;
> > @@ -250,8 +270,7 @@ xv_get_info(struct video *vid)
> >   XvEncodingInfo *xv_encs;
> >   struct xv_adap *adap;
> >   unsigned int nenc, p;
> > - int num_xvformats, nadaps, i, j, ret;
> > - char fmtName[5];
> > + int num_xvformats, nadaps, i, j, ret, enc;
> >  
> >   if ((x->dpy = XOpenDisplay(NULL)) == NULL) {
> >   warnx("cannot open display %s", XDisplayName(NULL));
> > @@ -312,16 +331,9 @@ xv_get_info(struct video *vid)
> >      &num_xvformats);
> >   adap->nfmts = 0;
> >   for (j = 0; j < num_xvformats; j++) {
> > - snprintf(fmtName, sizeof(fmtName), "%c%c%c%c",
> > -    xvformats[j].id & 0xff,
> > -    (xvformats[j].id >> 8) & 0xff,
> > -    (xvformats[j].id >> 16) & 0xff,
> > -    (xvformats[j].id >> 24) & 0xff);
> > - if (!strcmp(fmtName, "YUY2")) {
> > - encs[ENC_YUY2].xv_id = xvformats[j].id;
> > - adap->fmts[adap->nfmts++] = xvformats[j].id;
> > - } else if (!strcmp(fmtName, "UYVY")) {
> > - encs[ENC_UYVY].xv_id = xvformats[j].id;
> > + enc = find_enc_by_id(xvformats[j].id);
> > + if (enc < ENC_LAST) {
> > + encs[enc].flags |= SW_XV;
> >   adap->fmts[adap->nfmts++] = xvformats[j].id;
> >   }
> >   if (adap->nfmts >= MAX_FMTS)
> > @@ -369,20 +381,20 @@ xv_sel_adap(struct video *vid)
> >   x->cur_adap = i;
> >   adap = &x->adaps[i];
> >   for (i = 0; i < adap->nfmts; i++) {
> > - if (adap->fmts[i] == encs[vid->enc].xv_id)
> > + if (adap->fmts[i] == encs[vid->enc_xv].id)
> >   break;
> >   }
> >   if (i >= adap->nfmts) {
> >   warnx("Xv adaptor '%d' doesn't support %s",
> >      x->adaps[x->cur_adap].adap_index,
> > -    encs[vid->enc].name);
> > +    encs[vid->enc_xv].name);
> >   return 0;
> >   }
> >   }
> >   for (i = 0; i < x->nadaps && x->cur_adap == -1; i++) {
> >   adap = &x->adaps[i];
> >   for (j = 0; j < adap->nfmts; j++) {
> > - if (adap->fmts[j] == encs[vid->enc].xv_id) {
> > + if (adap->fmts[j] == encs[vid->enc_xv].id) {
> >   x->cur_adap = i;
> >   break;
> >   }
> > @@ -445,7 +457,7 @@ xv_dump_info(struct video *vid)
> >  
> >   fprintf(stderr, "  encodings: ");
> >   for (i = 0, j = 0; i < ENC_LAST; i++) {
> > - if (encs[i].xv_id != -1 && !(encs[i].flags & SW_XV)) {
> > + if (encs[i].id != -1 && (encs[i].flags & SW_XV)) {
> >   if (j)
> >   fprintf(stderr, ", ");
> >   fprintf(stderr, "%s", encs[i].name);
> > @@ -502,7 +514,7 @@ xv_init(struct video *vid)
> >  
> >   resize_window(vid, 0);
> >  
> > - x->xv_image = XvCreateImage(x->dpy, x->port, encs[vid->enc].xv_id,
> > + x->xv_image = XvCreateImage(x->dpy, x->port, encs[vid->enc_xv].id,
> >      vid->frame_buffer, vid->width, vid->height);
> >  
> >   return 1;
> > @@ -780,36 +792,20 @@ dev_get_encs(struct video *vid)
> >   fmtdesc.index = 0;
> >   fmtdesc.type = d->buf_type;
> >   while (ioctl(d->fd, VIDIOC_ENUM_FMT, &fmtdesc) >= 0) {
> > - if (fmtdesc.pixelformat == V4L2_PIX_FMT_YUYV) {
> > - i = find_enc("yuy2");
> > - if (i < ENC_LAST)
> > - encs[i].dev_id = fmtdesc.pixelformat;
> > - }
> > - if (fmtdesc.pixelformat == V4L2_PIX_FMT_UYVY) {
> > - i = find_enc("uyvy");
> > - if (i < ENC_LAST)
> > - encs[i].dev_id = fmtdesc.pixelformat;
> > + i = find_enc_by_id(fmtdesc.pixelformat);
> > + if (i < ENC_LAST) {
> > + encs[i].flags |= SW_DEV;
> >   }
> >   fmtdesc.index++;
> >   }
> >   for (i = 0; encs[i].name; i++) {
> > - if (encs[i].dev_id != -1)
> > + if (encs[i].flags & SW_DEV)
> >   break;
> >   }
> >   if (i >= ENC_LAST) {
> >   warnx("%s has no usable YUV encodings", d->path);
> >   return 0;
> >   }
> > - if (encs[ENC_YUY2].dev_id == -1 &&
> > -    encs[ENC_UYVY].dev_id != -1) {
> > - encs[ENC_YUY2].dev_id = encs[ENC_UYVY].dev_id;
> > - encs[ENC_YUY2].flags |= SW_DEV;
> > - }
> > - if (encs[ENC_UYVY].dev_id == -1 &&
> > -    encs[ENC_YUY2].dev_id != -1) {
> > - encs[ENC_UYVY].dev_id = encs[ENC_YUY2].dev_id;
> > - encs[ENC_UYVY].flags |= SW_DEV;
> > - }
> >  
> >   return 1;
> >  }
> > @@ -824,7 +820,7 @@ dev_get_sizes(struct video *vid)
> >  
> >   nsizes = 0;
> >   fsize.index = 0;
> > - fsize.pixel_format = encs[vid->enc].dev_id;
> > + fsize.pixel_format = encs[vid->enc].id;
> >   while (ioctl(d->fd, VIDIOC_ENUM_FRAMESIZES, &fsize) == 0) {
> >   switch (fsize.type) {
> >   case V4L2_FRMSIZE_TYPE_DISCRETE:
> > @@ -915,7 +911,7 @@ dev_get_rates(struct video *vid)
> >  
> >   for (i = 0; i < d->nsizes; i++) {
> >   bzero(&ival, sizeof(ival));
> > - ival.pixel_format = encs[vid->enc].dev_id;
> > + ival.pixel_format = encs[vid->enc].id;
> >   ival.width = d->sizes[i].w;
> >   ival.height = d->sizes[i].h;
> >   ival.index = 0;
> > @@ -1072,7 +1068,7 @@ dev_dump_info(struct video *vid)
> >  
> >   fprintf(stderr, "  encodings: ");
> >   for (i = 0, j = 0; i < ENC_LAST; i++) {
> > - if (encs[i].dev_id != -1 && !(encs[i].flags & SW_DEV)) {
> > + if (encs[i].flags & SW_DEV) {
> >   if (j)
> >   fprintf(stderr, ", ");
> >   fprintf(stderr, "%s", encs[i].name);
> > @@ -1136,7 +1132,7 @@ dev_init(struct video *vid)
> >   fmt.type = d->buf_type;
> >   fmt.fmt.pix.width = vid->width;
> >   fmt.fmt.pix.height = vid->height;
> > - fmt.fmt.pix.pixelformat = encs[vid->enc].dev_id;
> > + fmt.fmt.pix.pixelformat = encs[vid->enc].id;
> >   fmt.fmt.pix.field = V4L2_FIELD_ANY;
> >   if (ioctl(d->fd, VIDIOC_S_FMT, &fmt) < 0) {
> >   warn("VIDIOC_S_FMT");
> > @@ -1289,58 +1285,64 @@ int
> >  choose_enc(struct video *vid)
> >  {
> >   int i;
> > + int enc, enc_xv;
> >  
> >   if (vid->enc < 0) {
> >   for (i = 0; vid->enc < 0 && i < ENC_LAST; i++) {
> > - if ((vid->mode & M_IN_DEV) && (vid->mode & M_OUT_XV)) {
> > - if (encs[i].dev_id != -1 &&
> > -    encs[i].xv_id != -1 &&
> > -    (encs[i].flags & SW_MASK) == 0)
> > - vid->enc = i;
> > - } else if (vid->mode & M_IN_DEV) {
> > - if (encs[i].dev_id != -1 &&
> > -    (encs[i].flags & SW_MASK) == 0)
> > - vid->enc = i;
> > - } else if (vid->mode & M_OUT_XV) {
> > - if (encs[i].xv_id != -1 &&
> > -    (encs[i].flags & SW_MASK) == 0)
> > - vid->enc = i;
> > - }
> > + if (encs[i].flags & SW_DEV)
> > + vid->enc = i;
> > + }
> > + }
> > + if (vid->enc_xv < 0) {
> > + for (i = 0; vid->enc_xv < 0 && i < ENC_LAST; i++) {
> > + if (encs[i].flags & SW_XV)
> > + vid->enc_xv = i;
> >   }
> > + }
> > + if ((vid->mode & M_IN_DEV) && (vid->mode & M_OUT_XV)) {
> >   for (i = 0; vid->enc < 0 && i < ENC_LAST; i++) {
> > - if ((vid->mode & M_IN_DEV) && (vid->mode & M_OUT_XV)) {
> > - if (encs[i].dev_id != -1 &&
> > -    encs[i].xv_id != -1)
> > - vid->enc = i;
> > - } else if (vid->mode & M_IN_DEV) {
> > - if (encs[i].dev_id != -1)
> > - vid->enc = i;
> > - } else if (vid->mode & M_OUT_XV) {
> > - if (encs[i].xv_id != -1)
> > - vid->enc = i;
> > + if ((encs[i].flags & SW_DEV) &&
> > +    (encs[i].flags & SW_XV)) {
> > + vid->enc = i;
> > + vid->enc_xv = i;
> >   }
> >   }
> >   }
> > - if (vid->enc < 0) {
> > - warnx("could not find a usable encoding");
> > - return 0;
> > - }
> > - if ((vid->mode & M_IN_DEV) && encs[vid->enc].dev_id == -1) {
> > + if (vid->enc < 0 && vid->mode & M_IN_FILE)
> > + vid->enc = ENC_YUYV;
> > + if ((vid->mode & M_IN_DEV) && !(encs[vid->enc].flags & SW_DEV)) {
> >   warnx("device %s can't supply %s", vid->dev.path,
> >      encs[vid->enc].name);
> >   return 0;
> >   }
> > - if ((vid->mode & M_OUT_XV) && encs[vid->enc].xv_id == -1) {
> > + if (vid->mode & M_OUT_XV && vid->enc != vid->enc_xv) {
> > + /* check if conversion is possible */
> > + enc = (vid->enc == ENC_YUYV) ? ENC_YUY2 : vid->enc;
> > + enc_xv = (vid->enc_xv == ENC_YUYV) ? ENC_YUY2 : vid->enc_xv;
> > +
> > + if (enc == enc_xv)
> > + vid->conv_type = CONV_NONE;
> > + else if ((enc == ENC_YUY2 && enc_xv == ENC_UYVY) ||
> > +    (enc == ENC_UYVY && enc_xv == ENC_YUY2))
> > + vid->conv_type = CONV_SWAP;
> > + else if (enc == ENC_YUY2 && enc_xv == ENC_YV12)
> > + vid->conv_type = CONV_YV12;
> > + else {
> > + warn("conversion from %s to %s is unsupported",
> > +    encs[vid->enc].name, encs[vid->enc_xv].name);
> > + return 0;
> > + }
> > + }
> > + if (vid->enc < 0) {
> > + warnx("could not find a usable encoding");
> > + return 0;
> > + }
> > + if ((vid->mode & M_OUT_XV) && !(encs[vid->enc_xv].flags & SW_XV)) {
> >   warnx("Xv adaptor %d can't display %s",
> >      vid->xdsp.adaps[vid->xdsp.cur_adap].adap_index,
> > -    encs[vid->enc].name);
> > +    encs[vid->enc_xv].name);
> >   return 0;
> >   }
> > - if (((vid->mode & M_IN_DEV) &&
> > -    (encs[vid->enc].flags & SW_MASK) == SW_DEV) ||
> > -    ((vid->mode & M_OUT_XV) &&
> > -    (encs[vid->enc].flags & SW_MASK) == SW_XV))
> > - vid->conv_type = CONV_SWAP;
> >  
> >   return 1;
> >  }
> > @@ -1491,12 +1493,8 @@ setup(struct video *vid)
> >   }
> >  
> >   if (vid->conv_type) {
> > - if (vid->conv_type == CONV_SWAP) {
> > - vid->conv_bufsz = vid->bpf;
> > - } else {
> > - warnx("invalid conversion type");
> > - return 0;
> > - }
> > + vid->conv_bufsz =
> > +    (vid->width * vid->height * encs[vid->enc_xv].bpp) / NBBY;
> >   if ((vid->conv_buffer = calloc(1, vid->conv_bufsz)) == NULL) {
> >   warn("conv_buffer");
> >   return 0;
> > @@ -1633,6 +1631,34 @@ got_shutdown(int s)
> >  }
> >  
> >  int
> > +yuy2_to_yv12(uint8_t *src, uint8_t *dst, int width, int height)
> > +{
> > + int row, col, c, o;
> > + int iy = 0;
> > + int iu = width * height;
> > + int iv = iu + iu / 4;
> > +
> > + for (row = 0; row < height * 2; row += 2) {
> > + c = row * width;
> > + for (col = 0; col < width * 2 ; col += 4) {
> > + o = c + col;
> > + dst[iy++] = src[o];
> > + dst[iy++] = src[o + 2];
> > + if ((row & 0x03)  == 0) {
> > + o++;
> > + dst[iv++] = (src[o] + src[o + width * 2]) / 2;
> > + o += 2;
> > + dst[iu++] = (src[o] + src[o + width * 2]) / 2;
> > + if (o + width * 2 >= iu*2)
> > + printf("hello\n");
> > + }
> > + }
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +int
> >  stream(struct video *vid)
> >  {
> >   struct xdsp *x = &vid->xdsp;
> > @@ -1721,7 +1747,9 @@ stream(struct video *vid)
> >   src = vid->frame_buffer;
> >   if (vid->conv_type == CONV_SWAP) {
> >   swab(src, vid->conv_buffer, vid->bpf);
> > - src = vid->conv_buffer;
> > + } else if (vid->conv_type == CONV_YV12) {
> > + yuy2_to_yv12(src, vid->conv_buffer,
> > +    vid->width, vid->height);
> >   }
> >  
> >   if ((vid->mode & M_OUT_FILE) && wout) {
> > @@ -1743,6 +1771,8 @@ stream(struct video *vid)
> >   }
> >   }
> >   if (vid->mode & M_OUT_XV) {
> > + src = (vid->conv_type) ?
> > +    vid->conv_buffer : vid->frame_buffer;
> >   x->xv_image->data = src;
> >   if (x->resized) {
> >   x->resized = 0;
> > @@ -1863,6 +1893,7 @@ main(int argc, char *argv[])
> >   vid.dev.fd = vid.iofile_fd = -1;
> >   vid.mode = M_IN_DEV | M_OUT_XV;
> >   vid.enc = -1;
> > + vid.enc_xv = -1;
> >   vid.mmap_on = 1; /* mmap method is default */
> >   wout = 1;
> >  
> >

Reply | Threaded
Open this post in threaded view
|

Re: video(1) and modesetting driver

Raphael Graf-2
In reply to this post by Artturi Alm
On Fri, Jan 25, 2019 at 12:52:38PM +0200, Artturi Alm wrote:

> On Fri, Jan 25, 2019 at 11:16:49AM +0100, Raphael Graf wrote:
> > Hi Martin,
> > Thanks for your feedback!
> >
> > On Wed, Jan 23, 2019 at 12:54:51PM -0200, Martin Pieuchot wrote:
> > > Hello Raphael,
> > >
> > > On 16/01/19(Wed) 12:41, Raphael Graf wrote:
> > > > Here is an attempt to make video(1) work with the modesetting driver.
> > > > See https://marc.info/?l=openbsd-bugs&m=152231686416039&w=2
> > > >
> > > > The general idea:
> > > > If there is no common encoding for input (device) and output (Xv), the
> > > > encoding is converted to something supported by the output.
> > > > No conversion is done if the output is sent to file (options -o/-O).
> > > >
> > > > The option -e is now used as the input format. The output format
> > > > is selected automatically (if output goes to Xv).
> > > >
> > > > The following encodings are supported: yuy2, yuyv, uyuy, yv12
> > >
> > > Did you test all of them?
> >
> > Yes, I have tested all encodings with these two devices:
> >
> > uvideo0 at uhub0 port 6 configuration 1 interface 0
> > "Chicony Electronics Co., Ltd. product 0x480c" rev 2.00/31.34 addr 2
> >
> > uvideo0 at uhub1 port 8 configuration 1 interface 0 "SC20A38485AA4629RX
> > Integrated Camera" rev 2.00/0.21 addr 4
> >
> > >
> > > > yuy2 and yuyv are treated as two different encodings, they share the same
> > > > pixelformat, but have different ids. (This behaviour is different from the
> > > > current description of '-e' in the manpage)
> > >
> > > What about fixing the manpage then? ;)
> >
> > The new diff below contains the changes to the manpage.
> >
> > >
> > > > I have tested on two lenovo laptops using both drivers (modesetting and intel).
> > > >
> > > > On my laptop (lenovo X1 Carbon), with modesetting diver:
> > > > yuyv is converted to yv12 before sending to Xv.
> > > >
> > > > On the same laptop With intel driver:
> > > > yuyv is converted to yuy2 (no-op)
> > >
> > > Nice work, some comments below.
> > >
> > > Make sure the code you're writing follows style(9) :o)
> > >
> > > > Index: video.c
> > > > ===================================================================
> > > > RCS file: /cvs/xenocara/app/video/video.c,v
> > > > retrieving revision 1.26
> > > > diff -u -p -u -p -r1.26 video.c
> > > > --- video.c 4 Jan 2019 17:45:00 -0000 1.26
> > > > +++ video.c 16 Jan 2019 11:30:21 -0000
> > > > @@ -38,6 +38,8 @@
> > > >  #include <X11/extensions/Xvlib.h>
> > > >  #include <X11/Xatom.h>
> > > >
> > > > +#define V_V4L2_PIX_FMT_YUY2    v4l2_fourcc('Y', 'U', 'Y', '2')
> > >
> > > Why do we need this define and not the others?  Aren't we missing an
> > > include?
> >
> > Actually, I don't know why this define is missing in videoio.h.
> > This YUY2 format (id:0x32595559) is reported by Xv.
> > The uvideo(4) driver reports YUYV which has a define in videoio.h.
> > (YUY2 and YUYV are equivalent)
> >
> > In the new diff, I have changed the variable names enc_in/enc_out to enc/enc_xv
> > as the second 'enc' is only used for Xv output.
> > I also fixed the style(9) issues and added a CONV_NONE define.
> >
> > Note that I'm not sure if the algorithm used in the yuy2_to_yv12 function is
> > correct in respect to color theory..
> > (It is a conversion from 16 to 12 bits/pixel)
> >
> > Sorry, it is still a big diff, feedback is welcome.
> >
> >
>
> Hi,
>
> got one question below.
>
> -Artturi
>
> [...snip...]
>
> > @@ -1633,6 +1631,34 @@ got_shutdown(int s)
> >  }
> >  
> >  int
> > +yuy2_to_yv12(uint8_t *src, uint8_t *dst, int width, int height)
> > +{
> > + int row, col, c, o;
> > + int iy = 0;
> > + int iu = width * height;
> > + int iv = iu + iu / 4;
> > +
> > + for (row = 0; row < height * 2; row += 2) {
> > + c = row * width;
> > + for (col = 0; col < width * 2 ; col += 4) {
> > + o = c + col;
> > + dst[iy++] = src[o];
> > + dst[iy++] = src[o + 2];
> > + if ((row & 0x03)  == 0) {
> > + o++;
> > + dst[iv++] = (src[o] + src[o + width * 2]) / 2;
> > + o += 2;
> > + dst[iu++] = (src[o] + src[o + width * 2]) / 2;
> > + if (o + width * 2 >= iu*2)
> > + printf("hello\n");
>
> is that a placeholder for something relevant, or what does hello mean here?

Oups, sorry, can you ignore that for now? I'll remove this in the next iteration.

>
> > + }
> > + }
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +int
> >  stream(struct video *vid)
> >  {
> >   struct xdsp *x = &vid->xdsp;
>
> [...snip...]

Reply | Threaded
Open this post in threaded view
|

Re: video(1) and modesetting driver

Raphael Graf-2
In reply to this post by Martin Pieuchot
On Fri, Jan 25, 2019 at 09:09:29AM -0200, Martin Pieuchot wrote:

> On 25/01/19(Fri) 11:16, Raphael Graf wrote:
> > On Wed, Jan 23, 2019 at 12:54:51PM -0200, Martin Pieuchot wrote:
> > > > Index: video.c
> > > > ===================================================================
> > > > RCS file: /cvs/xenocara/app/video/video.c,v
> > > > retrieving revision 1.26
> > > > diff -u -p -u -p -r1.26 video.c
> > > > --- video.c 4 Jan 2019 17:45:00 -0000 1.26
> > > > +++ video.c 16 Jan 2019 11:30:21 -0000
> > > > @@ -38,6 +38,8 @@
> > > >  #include <X11/extensions/Xvlib.h>
> > > >  #include <X11/Xatom.h>
> > > >
> > > > +#define V_V4L2_PIX_FMT_YUY2    v4l2_fourcc('Y', 'U', 'Y', '2')
> > >
> > > Why do we need this define and not the others?  Aren't we missing an
> > > include?
> >
> > Actually, I don't know why this define is missing in videoio.h.
> > This YUY2 format (id:0x32595559) is reported by Xv.
> > The uvideo(4) driver reports YUYV which has a define in videoio.h.
> > (YUY2 and YUYV are equivalent)
>
> I see.  I believe you're being too clever her.  XvListImageFormats(3)
> returns you a list with an Id in FourCC format and you're using V4L2
> defines to match these IDs.
>
> I'd suggest you define only one encoding "yuy2" or "yuyv" and make the
> matching function, find_enc_by_id() a bit more clever.  That would
> explicitly document that Xv and V4L2 use different names for the same
> format.  On top of that you'll get rid of your conversion code :)
>  

The new diff below solves this yuy2/yuyv problem by defining them both under
the same name 'yuy2'.
The only change to the manpage is now the addition of yv12 to the list of
valid encodings.

The trickiest part is the 'choose_enc' function where encodings are chosen
based on device capabilities.
The following conversions are now possible:
yuy2 -> uyvy
yuy2 -> yv12
uyvy -> yuy2
uyvy -> yv12

As my webcam only provides yuy2, I have used input-files for testing:
$ video -i test.yuy2
$ video -i test.uyvy -e uyvy
$ video -i test.yv12 -e yv12

These examples work for me with both drivers (modesetting and intel).
The conversion to yv12 has a small performance impact, of course. Do you think
the performance is acceptable?

> > In the new diff, I have changed the variable names enc_in/enc_out to enc/enc_xv
> > as the second 'enc' is only used for Xv output.
> > I also fixed the style(9) issues and added a CONV_NONE define.
>
> I find enc_in/enc_out more explicit, but let's keep it like that for
> now.  This also makes the diff smaller :)
>
> > Note that I'm not sure if the algorithm used in the yuy2_to_yv12 function is
> > correct in respect to color theory..
> > (It is a conversion from 16 to 12 bits/pixel)
>
> I'm sure you can find some good documentation and/or example about that
> on the Web or in the existing freesoftware ecosystem.
>

Yes, there are libraries like 'libyuv' doing it in the same way.
(they also have optimized versions (SSE etc.))

Any comments, questions or feedback?



Index: video.1
===================================================================
RCS file: /cvs/xenocara/app/video/video.1,v
retrieving revision 1.13
diff -u -p -u -p -r1.13 video.1
--- video.1 4 Jun 2016 07:44:32 -0000 1.13
+++ video.1 7 Feb 2019 12:08:37 -0000
@@ -84,9 +84,10 @@ The default is 0, the first adaptor repo
 .It Fl e Ar encoding
 Lowercase FOURCC name of video encoding to use.
 Valid arguments are
-.Ql uyvy
+.Ql uyvy ,
+.Ql yuy2
 and
-.Ql yuy2 .
+.Ql yv12 .
 The default is
 .Ql yuy2
 unless
Index: video.c
===================================================================
RCS file: /cvs/xenocara/app/video/video.c,v
retrieving revision 1.27
diff -u -p -u -p -r1.27 video.c
--- video.c 22 Jan 2019 20:02:40 -0000 1.27
+++ video.c 7 Feb 2019 12:08:37 -0000
@@ -38,6 +38,9 @@
 #include <X11/extensions/Xvlib.h>
 #include <X11/Xatom.h>
 
+/* Synonym for YUYV */
+#define V_V4L2_PIX_FMT_YUY2    v4l2_fourcc('Y', 'U', 'Y', '2')
+
 /* Xv(3) adaptor properties */
 struct xv_adap {
  char name[128];
@@ -133,20 +136,22 @@ struct dev {
 /* video encodingss */
 struct encodings {
  char *name;
+ int id;
  int bpp;
- int xv_id;
- int dev_id;
-#define SW_DEV 0x1
-#define SW_XV 0x2
-#define SW_MASK (SW_DEV | SW_XV)
+#define SUP_DEV 0x1
+#define SUP_XV 0x2
  int flags;
 } encs[] = {
+#define ENC_YUYV 1
+ { "yuy2", V4L2_PIX_FMT_YUYV, 16, 0 },
 #define ENC_YUY2 0
- { "yuy2", 16, -1, -1, 0 },
-#define ENC_UYVY 1
- { "uyvy", 16, -1, -1, 0 },
-#define ENC_LAST 2
- { NULL, 0, 0, 0, 0 }
+ { "yuy2", V_V4L2_PIX_FMT_YUY2, 16, 0 },
+#define ENC_UYVY 2
+ { "uyvy", V4L2_PIX_FMT_UYVY,  16, 0 },
+#define ENC_YV12 3
+ { "yv12", V4L2_PIX_FMT_YVU420, 12, 0 },
+#define ENC_LAST 4
+ { NULL, 0, 0, 0 }
 };
 
 struct video {
@@ -162,9 +167,12 @@ struct video {
  char iofile[FILENAME_MAX];
  int iofile_fd;
  char *sz_str;
-#define CONV_SWAP 0x1
+#define CONV_NONE 0x1
+#define CONV_SWAP 0x2
+#define CONV_YV12 0x4
  int conv_type;
  int enc;
+ int enc_xv;
  int full_screen;
  int net_wm;
  int width;
@@ -216,6 +224,7 @@ int stream(struct video *);
 void got_frame(int);
 void got_shutdown(int);
 int find_enc(char *);
+int find_enc_by_id(int);
 void usage(void);
 
 static volatile sig_atomic_t play, shutdown, hold, wout;
@@ -242,6 +251,17 @@ find_enc(char *name)
 }
 
 int
+find_enc_by_id(int id)
+{
+ int i;
+
+ for (i = 0; i < ENC_LAST; i++)
+ if (encs[i].id == id)
+ break;
+ return i;
+}
+
+int
 xv_get_info(struct video *vid)
 {
  struct xdsp *x = &vid->xdsp;
@@ -250,8 +270,7 @@ xv_get_info(struct video *vid)
  XvEncodingInfo *xv_encs;
  struct xv_adap *adap;
  unsigned int nenc, p;
- int num_xvformats, nadaps, i, j, ret;
- char fmtName[5];
+ int num_xvformats, nadaps, i, j, ret, enc;
 
  if ((x->dpy = XOpenDisplay(NULL)) == NULL) {
  warnx("cannot open display %s", XDisplayName(NULL));
@@ -312,16 +331,9 @@ xv_get_info(struct video *vid)
     &num_xvformats);
  adap->nfmts = 0;
  for (j = 0; j < num_xvformats; j++) {
- snprintf(fmtName, sizeof(fmtName), "%c%c%c%c",
-    xvformats[j].id & 0xff,
-    (xvformats[j].id >> 8) & 0xff,
-    (xvformats[j].id >> 16) & 0xff,
-    (xvformats[j].id >> 24) & 0xff);
- if (!strcmp(fmtName, "YUY2")) {
- encs[ENC_YUY2].xv_id = xvformats[j].id;
- adap->fmts[adap->nfmts++] = xvformats[j].id;
- } else if (!strcmp(fmtName, "UYVY")) {
- encs[ENC_UYVY].xv_id = xvformats[j].id;
+ enc = find_enc_by_id(xvformats[j].id);
+ if (enc < ENC_LAST) {
+ encs[enc].flags |= SUP_XV;
  adap->fmts[adap->nfmts++] = xvformats[j].id;
  }
  if (adap->nfmts >= MAX_FMTS)
@@ -369,20 +381,20 @@ xv_sel_adap(struct video *vid)
  x->cur_adap = i;
  adap = &x->adaps[i];
  for (i = 0; i < adap->nfmts; i++) {
- if (adap->fmts[i] == encs[vid->enc].xv_id)
+ if (adap->fmts[i] == encs[vid->enc_xv].id)
  break;
  }
  if (i >= adap->nfmts) {
  warnx("Xv adaptor '%d' doesn't support %s",
     x->adaps[x->cur_adap].adap_index,
-    encs[vid->enc].name);
+    encs[vid->enc_xv].name);
  return 0;
  }
  }
  for (i = 0; i < x->nadaps && x->cur_adap == -1; i++) {
  adap = &x->adaps[i];
  for (j = 0; j < adap->nfmts; j++) {
- if (adap->fmts[j] == encs[vid->enc].xv_id) {
+ if (adap->fmts[j] == encs[vid->enc_xv].id) {
  x->cur_adap = i;
  break;
  }
@@ -445,7 +457,7 @@ xv_dump_info(struct video *vid)
 
  fprintf(stderr, "  encodings: ");
  for (i = 0, j = 0; i < ENC_LAST; i++) {
- if (encs[i].xv_id != -1 && !(encs[i].flags & SW_XV)) {
+ if (encs[i].id != -1 && (encs[i].flags & SUP_XV)) {
  if (j)
  fprintf(stderr, ", ");
  fprintf(stderr, "%s", encs[i].name);
@@ -502,7 +514,7 @@ xv_init(struct video *vid)
 
  resize_window(vid, 0);
 
- x->xv_image = XvCreateImage(x->dpy, x->port, encs[vid->enc].xv_id,
+ x->xv_image = XvCreateImage(x->dpy, x->port, encs[vid->enc_xv].id,
     vid->frame_buffer, vid->width, vid->height);
 
  return 1;
@@ -780,36 +792,19 @@ dev_get_encs(struct video *vid)
  fmtdesc.index = 0;
  fmtdesc.type = d->buf_type;
  while (ioctl(d->fd, VIDIOC_ENUM_FMT, &fmtdesc) >= 0) {
- if (fmtdesc.pixelformat == V4L2_PIX_FMT_YUYV) {
- i = find_enc("yuy2");
- if (i < ENC_LAST)
- encs[i].dev_id = fmtdesc.pixelformat;
- }
- if (fmtdesc.pixelformat == V4L2_PIX_FMT_UYVY) {
- i = find_enc("uyvy");
- if (i < ENC_LAST)
- encs[i].dev_id = fmtdesc.pixelformat;
- }
+ i = find_enc_by_id(fmtdesc.pixelformat);
+ if (i < ENC_LAST)
+ encs[i].flags |= SUP_DEV;
  fmtdesc.index++;
  }
  for (i = 0; encs[i].name; i++) {
- if (encs[i].dev_id != -1)
+ if (encs[i].flags & SUP_DEV)
  break;
  }
  if (i >= ENC_LAST) {
  warnx("%s has no usable YUV encodings", d->path);
  return 0;
  }
- if (encs[ENC_YUY2].dev_id == -1 &&
-    encs[ENC_UYVY].dev_id != -1) {
- encs[ENC_YUY2].dev_id = encs[ENC_UYVY].dev_id;
- encs[ENC_YUY2].flags |= SW_DEV;
- }
- if (encs[ENC_UYVY].dev_id == -1 &&
-    encs[ENC_YUY2].dev_id != -1) {
- encs[ENC_UYVY].dev_id = encs[ENC_YUY2].dev_id;
- encs[ENC_UYVY].flags |= SW_DEV;
- }
 
  return 1;
 }
@@ -824,7 +819,7 @@ dev_get_sizes(struct video *vid)
 
  nsizes = 0;
  fsize.index = 0;
- fsize.pixel_format = encs[vid->enc].dev_id;
+ fsize.pixel_format = encs[vid->enc].id;
  while (ioctl(d->fd, VIDIOC_ENUM_FRAMESIZES, &fsize) == 0) {
  switch (fsize.type) {
  case V4L2_FRMSIZE_TYPE_DISCRETE:
@@ -915,7 +910,7 @@ dev_get_rates(struct video *vid)
 
  for (i = 0; i < d->nsizes; i++) {
  bzero(&ival, sizeof(ival));
- ival.pixel_format = encs[vid->enc].dev_id;
+ ival.pixel_format = encs[vid->enc].id;
  ival.width = d->sizes[i].w;
  ival.height = d->sizes[i].h;
  ival.index = 0;
@@ -1072,7 +1067,7 @@ dev_dump_info(struct video *vid)
 
  fprintf(stderr, "  encodings: ");
  for (i = 0, j = 0; i < ENC_LAST; i++) {
- if (encs[i].dev_id != -1 && !(encs[i].flags & SW_DEV)) {
+ if (encs[i].flags & SUP_DEV) {
  if (j)
  fprintf(stderr, ", ");
  fprintf(stderr, "%s", encs[i].name);
@@ -1136,7 +1131,7 @@ dev_init(struct video *vid)
  fmt.type = d->buf_type;
  fmt.fmt.pix.width = vid->width;
  fmt.fmt.pix.height = vid->height;
- fmt.fmt.pix.pixelformat = encs[vid->enc].dev_id;
+ fmt.fmt.pix.pixelformat = encs[vid->enc].id;
  fmt.fmt.pix.field = V4L2_FIELD_ANY;
  if (ioctl(d->fd, VIDIOC_S_FMT, &fmt) < 0) {
  warn("VIDIOC_S_FMT");
@@ -1289,58 +1284,60 @@ int
 choose_enc(struct video *vid)
 {
  int i;
+ int enc, enc_xv;
 
  if (vid->enc < 0) {
- for (i = 0; vid->enc < 0 && i < ENC_LAST; i++) {
- if ((vid->mode & M_IN_DEV) && (vid->mode & M_OUT_XV)) {
- if (encs[i].dev_id != -1 &&
-    encs[i].xv_id != -1 &&
-    (encs[i].flags & SW_MASK) == 0)
- vid->enc = i;
- } else if (vid->mode & M_IN_DEV) {
- if (encs[i].dev_id != -1 &&
-    (encs[i].flags & SW_MASK) == 0)
+ if (vid->mode & M_IN_DEV) {
+ for (i = 0; vid->enc < 0 && i < ENC_LAST; i++)
+ if (encs[i].flags & SUP_DEV)
  vid->enc = i;
- } else if (vid->mode & M_OUT_XV) {
- if (encs[i].xv_id != -1 &&
-    (encs[i].flags & SW_MASK) == 0)
- vid->enc = i;
- }
+ } else {
+ vid->enc = ENC_YUYV;
  }
- for (i = 0; vid->enc < 0 && i < ENC_LAST; i++) {
- if ((vid->mode & M_IN_DEV) && (vid->mode & M_OUT_XV)) {
- if (encs[i].dev_id != -1 &&
-    encs[i].xv_id != -1)
- vid->enc = i;
- } else if (vid->mode & M_IN_DEV) {
- if (encs[i].dev_id != -1)
- vid->enc = i;
- } else if (vid->mode & M_OUT_XV) {
- if (encs[i].xv_id != -1)
- vid->enc = i;
- }
+ if ((vid->mode & M_IN_DEV) && (vid->mode & M_OUT_XV)) {
+ for (i = 0; vid->enc < 0 && i < ENC_LAST; i++)
+ if (encs[i].flags == (SUP_DEV | SUP_XV))
+ vid->enc = vid->enc_xv = i;
  }
  }
  if (vid->enc < 0) {
  warnx("could not find a usable encoding");
  return 0;
  }
- if ((vid->mode & M_IN_DEV) && encs[vid->enc].dev_id == -1) {
+ if ((vid->mode & M_IN_DEV) && !(encs[vid->enc].flags & SUP_DEV)) {
  warnx("device %s can't supply %s", vid->dev.path,
     encs[vid->enc].name);
  return 0;
  }
- if ((vid->mode & M_OUT_XV) && encs[vid->enc].xv_id == -1) {
+ if (vid->enc_xv < 0 && (vid->mode & M_OUT_XV)) {
+ if (encs[vid->enc].flags & SUP_XV)
+ vid->enc_xv = vid->enc;
+ for (i = 0; vid->enc_xv < 0 && i < ENC_LAST; i++)
+ if (encs[i].flags & SUP_XV)
+ vid->enc_xv = i;
+ }
+
+ if (vid->mode & M_OUT_XV && vid->enc != vid->enc_xv) {
+ /* check if conversion is possible */
+ enc = (vid->enc == ENC_YUYV) ? ENC_YUY2 : vid->enc;
+ enc_xv = (vid->enc_xv == ENC_YUYV) ? ENC_YUY2 : vid->enc_xv;
+ if (enc == enc_xv)
+ vid->conv_type = CONV_NONE;
+ else {
+ if (enc == ENC_UYVY || enc_xv == ENC_UYVY)
+ vid->conv_type |= CONV_SWAP;
+ if (enc_xv == ENC_YV12)
+ vid->conv_type |= CONV_YV12;
+ }
+ if (!vid->conv_type)
+ vid->enc_xv = vid->enc;
+ }
+ if ((vid->mode & M_OUT_XV) && !(encs[vid->enc_xv].flags & SUP_XV)) {
  warnx("Xv adaptor %d can't display %s",
     vid->xdsp.adaps[vid->xdsp.cur_adap].adap_index,
-    encs[vid->enc].name);
+    encs[vid->enc_xv].name);
  return 0;
  }
- if (((vid->mode & M_IN_DEV) &&
-    (encs[vid->enc].flags & SW_MASK) == SW_DEV) ||
-    ((vid->mode & M_OUT_XV) &&
-    (encs[vid->enc].flags & SW_MASK) == SW_XV))
- vid->conv_type = CONV_SWAP;
 
  return 1;
 }
@@ -1490,13 +1487,9 @@ setup(struct video *vid)
  return 0;
  }
 
- if (vid->conv_type) {
- if (vid->conv_type == CONV_SWAP) {
- vid->conv_bufsz = vid->bpf;
- } else {
- warnx("invalid conversion type");
- return 0;
- }
+ if (vid->conv_type > CONV_NONE) {
+ vid->conv_bufsz =
+    (vid->width * vid->height * encs[vid->enc_xv].bpp) / NBBY;
  if ((vid->conv_buffer = calloc(1, vid->conv_bufsz)) == NULL) {
  warn("conv_buffer");
  return 0;
@@ -1633,6 +1626,37 @@ got_shutdown(int s)
 }
 
 int
+yuy2_to_yv12(uint8_t *src, uint8_t *dst, int width, int height, int swap)
+{
+ int row, col;
+ uint8_t *s = src, *p;
+ uint8_t *dy = dst;
+ uint8_t *du = dy + width * height;
+ uint8_t *dv = du + width * height / 4;
+
+ if ((width | height) & 1)
+ errx(1, "frame size %dx%d is not supported", width, height);
+
+ for (row = 0; row < height; row++) {
+ for (col = 0; col < width; col += 2) {
+ p = (swap) ? s + 1 : s;
+ *(dy++) = *p;
+ *(dy++) = *(p + 2);
+ if (!(row & 0x01)) {
+ p = (swap) ? s - 1 : s;
+ *(du++) =
+    (*(p + 3) + *(p + 3 + width * 2) + 1) / 2;
+ *(dv++) =
+    (*(p + 1) + *(p + 1 + width * 2) + 1) / 2;
+ }
+ s += 4;
+ }
+ }
+
+ return 0;
+}
+
+int
 stream(struct video *vid)
 {
  struct xdsp *x = &vid->xdsp;
@@ -1719,10 +1743,12 @@ stream(struct video *vid)
  play = 0;
 
  src = vid->frame_buffer;
- if (vid->conv_type == CONV_SWAP) {
+ if (vid->conv_type & CONV_YV12)
+ yuy2_to_yv12(src, vid->conv_buffer,
+                            vid->width, vid->height,
+    (vid->conv_type & CONV_SWAP));
+ else if (vid->conv_type & CONV_SWAP)
  swab(src, vid->conv_buffer, vid->bpf);
- src = vid->conv_buffer;
- }
 
  if ((vid->mode & M_OUT_FILE) && wout) {
  done = 0;
@@ -1743,6 +1769,8 @@ stream(struct video *vid)
  }
  }
  if (vid->mode & M_OUT_XV) {
+ src = (vid->conv_type > CONV_NONE) ?
+    vid->conv_buffer : vid->frame_buffer;
  x->xv_image->data = src;
  if (x->resized) {
  x->resized = 0;
@@ -1863,6 +1891,7 @@ main(int argc, char *argv[])
  vid.dev.fd = vid.iofile_fd = -1;
  vid.mode = M_IN_DEV | M_OUT_XV;
  vid.enc = -1;
+ vid.enc_xv = -1;
  vid.mmap_on = 1; /* mmap method is default */
  wout = 1;
 

Reply | Threaded
Open this post in threaded view
|

Re: video(1) and modesetting driver

Martin Pieuchot
On 07/02/19(Thu) 13:52, Raphael Graf wrote:
> [...]
> The new diff below solves this yuy2/yuyv problem by defining them both under
> the same name 'yuy2'.

That's great.  I would just change the comment to explain that it's due
to an incoherency between the names reported by XvListImageFormats(3) and
V4L2 :)

> The only change to the manpage is now the addition of yv12 to the list of
> valid encodings.

Fine, I just missed the point: why do we need to support yv12?

> The trickiest part is the 'choose_enc' function where encodings are chosen
> based on device capabilities.
> The following conversions are now possible:
> yuy2 -> uyvy
> yuy2 -> yv12
> uyvy -> yuy2
> uyvy -> yv12
>
> As my webcam only provides yuy2, I have used input-files for testing:
> $ video -i test.yuy2
> $ video -i test.uyvy -e uyvy
> $ video -i test.yv12 -e yv12
>
> These examples work for me with both drivers (modesetting and intel).
> The conversion to yv12 has a small performance impact, of course. Do you think
> the performance is acceptable?

Do you think it is?  When is the conversion needed?

Reply | Threaded
Open this post in threaded view
|

Re: video(1) and modesetting driver

Raphael Graf-2
On Wed, Feb 13, 2019 at 04:09:10PM -0200, Martin Pieuchot wrote:
> On 07/02/19(Thu) 13:52, Raphael Graf wrote:
> > [...]
> > The new diff below solves this yuy2/yuyv problem by defining them both under
> > the same name 'yuy2'.
>
> That's great.  I would just change the comment to explain that it's due
> to an incoherency between the names reported by XvListImageFormats(3) and
> V4L2 :)

I'll do this when the general idea of the diff gets accepted..

>
> > The only change to the manpage is now the addition of yv12 to the list of
> > valid encodings.
>
> Fine, I just missed the point: why do we need to support yv12?

The support for yv12 as an input encoding is actually a side effect of the
implementation. Webcams (video(4)) do not provide yv12, but is now possible to
read and display yv12 encoded files.

>
> > The trickiest part is the 'choose_enc' function where encodings are chosen
> > based on device capabilities.
> > The following conversions are now possible:
> > yuy2 -> uyvy
> > yuy2 -> yv12
> > uyvy -> yuy2
> > uyvy -> yv12
> >
> > As my webcam only provides yuy2, I have used input-files for testing:
> > $ video -i test.yuy2
> > $ video -i test.uyvy -e uyvy
> > $ video -i test.yv12 -e yv12
> >
> > These examples work for me with both drivers (modesetting and intel).
> > The conversion to yv12 has a small performance impact, of course. Do you think
> > the performance is acceptable?
>
> Do you think it is?  When is the conversion needed?
>

Conversion to yv12 is needed when Xv does neither support yuy2 nor uyvy.
This is the case when the modesetting driver is in use (see output of xvinfo).
I think the performance is acceptable, it is hardly noticable on my laptops.
 

Reply | Threaded
Open this post in threaded view
|

Re: video(1) and modesetting driver

Martin Pieuchot
On 14/02/19(Thu) 11:24, Raphael Graf wrote:

> On Wed, Feb 13, 2019 at 04:09:10PM -0200, Martin Pieuchot wrote:
> > On 07/02/19(Thu) 13:52, Raphael Graf wrote:
> > > [...]
> > > The new diff below solves this yuy2/yuyv problem by defining them both under
> > > the same name 'yuy2'.
> >
> > That's great.  I would just change the comment to explain that it's due
> > to an incoherency between the names reported by XvListImageFormats(3) and
> > V4L2 :)
>
> I'll do this when the general idea of the diff gets accepted..
>
> >
> > > The only change to the manpage is now the addition of yv12 to the list of
> > > valid encodings.
> >
> > Fine, I just missed the point: why do we need to support yv12?
>
> The support for yv12 as an input encoding is actually a side effect of the
> implementation. Webcams (video(4)) do not provide yv12, but is now possible to
> read and display yv12 encoded files.
>
> >
> > > The trickiest part is the 'choose_enc' function where encodings are chosen
> > > based on device capabilities.
> > > The following conversions are now possible:
> > > yuy2 -> uyvy
> > > yuy2 -> yv12
> > > uyvy -> yuy2
> > > uyvy -> yv12
> > >
> > > As my webcam only provides yuy2, I have used input-files for testing:
> > > $ video -i test.yuy2
> > > $ video -i test.uyvy -e uyvy
> > > $ video -i test.yv12 -e yv12
> > >
> > > These examples work for me with both drivers (modesetting and intel).
> > > The conversion to yv12 has a small performance impact, of course. Do you think
> > > the performance is acceptable?
> >
> > Do you think it is?  When is the conversion needed?
> >
>
> Conversion to yv12 is needed when Xv does neither support yuy2 nor uyvy.
> This is the case when the modesetting driver is in use (see output of xvinfo).
> I think the performance is acceptable, it is hardly noticable on my laptops.

Good, then please update the comment and I'll commit your diff :)

Thanks for solving this issue.