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;
> >
> >
