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...]
