hverkuil: ping mchehab: pong two things: 1. I intend to apply that patch with converts strncpy->strscpy as-is I remember you raised some questions, but, frankly, can't remember anymore :-) feel free to submit a patch on the top of it if you find anything that should be done on a different way. On a quick look, what's there is exactly what we're initializing already with strscpy() e. g. things that belong to the uAPI, like driver's name trying to find my comments on it. (btw, already applied your patches - I'm now cleaning up the patches I submitted myself) https://www.mail-archive.com/linux-media@vger.kernel.org/msg134931.html Basically getting rid of hardcoded sizes, which defeat the purpose IMHO. ok, I'll apply it with such changes 2. yesterday I did some changes (not submitted yet) at vim2m I noticed some weird things, though that is making me believe that maybe there's an issue at qvidcap does it properly handle different endiannes? the RGB3 issue you mentioned? I checked code, but couldn't find anything wrong. How do you test with qvidcap? yes. RGB3/BGR3 are handled one way; RGB/BGR on a different way I'm using it on different machines the X display is on an x86_64 machine streams generated by v4l2-ctl were either on a Tizen/arm32 machine or on another x86_64 So all are little endian? not sure if Tizen was built with BE or LE and you use streaming over the network with v4l2-ctl? yes Can you add the --stream-lossless to v4l2-ctl? and see if it still happens? the fwht compression it uses otherwise has undergone changes, so the bug could be there. let me try same thing how does it handle RGB and RGB3? I mean: what's the internal difference between them? FTY, those are the RGB formats that vim2m supports: [0]: 'RGBP' (16-bit RGB 5-6-5) [1]: 'RGBR' (16-bit RGB 5-6-5 BE) [2]: 'RGB3' (24-bit RGB 8-8-8) [3]: 'BGR3' (24-bit BGR 8-8-8) What do you mean with RGB? RGB23? Oh, 16 bit RGB passing just "RGB" to it, or just "BGR" does it mean 16-bits or 24-bits? pixelformat=RGB pixelformat RGB doesn't exist. v4l2-ctl should report an error. it doesn't. It accepts both RGB and BGR and those display fine god knows which pixelformat it selects. what's more weird is that RGB3/BGR3 displays fine with Gstreamer and the code seems to be correct at vim2m mchehab: it looks that if it cannot find the given pixelformat, then it picks the very first format reported by v4l2-ctl --list-formats. better to produce an error (it might print a message, but that would make harder if this is used on automated tests) for sure there's something wrong at qvidcap/v4l2-ctl side when I call v4l2-ctl with BGR3, if qvidcap didn't receive any frame yet, it prints: Pixel Format: 'RGBP' 16-bit RGB 5-6-5 (at qvidcap side) if it already started, even when the format changed, it doesn't re-print the pixel format message let me rebuild it on both machines, to be sure I'm on the very latest version same thing... it doesn't call printf("New Pixel Format: '%s' %s\n", fcc2s(m_overridePixelFormat).c_str(), pixfmt2s(m_overridePixelFormat).c_str()); when I ask for format changes there is still some flaky behavior there, I'm aware of that. it always displays Pixel Format: 'RGBP' 16-bit RGB 5-6-5 hmm... my script is actually changing the out format, and not the cap one fixed v4l2-ctl: it now errors on invalid pixelformats. hverkuil: basically, --set-fmt-video is supposed to be setting the video at the capture stream, but it is setting it at the output stream instead huh? --set-fmt-video does set the capture video. I added some debug messages just after S_FMT to be sure what it is doing at: void vidcap_set(cv4l_fd &_fd) I added if (!ret) printf("Capture Format: '%s'\n", fcc2s(vfmt.fmt.pix.pixelformat).c_str()); and at vidout_set() I added: if (!ret) printf("Output Format: '%s'\n", fcc2s(pixfmt).c_str()); (those were just after doioctl() when I ask the vim2m_test_with_qvidcap.sh to use BGR3, it displays: Format BGR3 Capture Format: 'RGBP' Output Format: 'BGR3' and, at qvidcap, it only shows RGBP no matter what I use with --set-fmt-video so, it sounds that, for m2m device, it is doing something odd You set the pixelformat with -x: that's the shortcut for --set-fmt-video-out. You do not set the pixelformat at all with --set-fmt-video.        v4l2-ctl --stream-mmap --stream-out-mmap \                --stream-to-host ${HOST}:${PORT} \                --stream-out-hor-speed 1 -x pixelformat=${FMT} \                --set-fmt-video width=${CAPWIDTH},height=${CAPHEIGHT} \                -d /dev/${VDEV} --stream-count=${COUNT} true yeah, that's indeed a mistake (I got that line from the time we've discussed about vim2m, back in jan) git push ops wrong window script fixed v4l2-ctl-streaming.cpp: In function ‘__u32 read_u32(FILE*)’: v4l2-ctl-streaming.cpp:434:7: warning: ignoring return value of ‘size_t fread(void*, size_t, size_t, FILE*)’, declared with attribute warn_unused_result [-Wunused-result] fread(&v, 1, sizeof(v), f); ~~~~~^~~~~~~~~~~~~~~~~~~~~ mchehab: fixed ok, it seems that now all formats are ok Kernel patch sent hverkuil: sent also the strscpy() patches with the fixes you pointed replied to your vim2m patch reviewed strscpy patch. One small thing that needs to be corrected and then it's good to go. Very nice cleanup. hverkuil: please check if the patches I sent are ok... I ended by pushing them by mistake! was supposed to just sync the status changes from my patchsets if needed, I'll revert or do a rebase looks good. ok, thanks for a quick review! hverkuil: cleanup finished I delegated a few patches that might need to further attention to myself also applied the pending pull requests mchehab: time for vacation? :-) yay! I'll just do a quick look at pdf generation last time I checked, it was failing Hmm, there are three strlcpy's left in drivers/staging/media. maybe. didn't check there this time drivers/staging/media/imx/imx-media-dev-common.c: strlcpy(imxmd->md.model, "imx-media", sizeof(imxmd->md.model)); drivers/staging/media/imx/imx-media-dev-common.c: strlcpy(imxmd->v4l2_dev.name, "imx-media", drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c: strlcpy(vpu->mdev.model, DRIVER_NAME, sizeof(vpu->mdev.model)); yep, 3 cases I guess those came after my patch series all of them are simple replacements, it seems hverkuil: just sent a patch fixing it basically: for i in $(git grep -l strlcpy drivers/staging/media/); do sed s,strlcpy,strscpy,g -i $i; done acked it thanks! If you can commit it, then I plan on adding a check for strcpy, strncpy and strlcpy in the daily build I will commit it the Latex issue is hard to fix it is at the big table at Documentation/media/uapi/v4l/pixfmt-packed-yuv.rst sphinx-build 1.7.8 something related to .. tabularcolumns:: there hmm... table is wrong even on html, it is causing troubles several cells have more than 34 columns