<!-- Some styling for better description lists --><style type='text/css'>dt { font-weight: bold;float: left;display:inline;margin-right: 1em} dd { display:block; margin-left: 2em}</style>
***: bingbu has quit IRC (Ping timeout: 246 seconds)
<br> iive has quit IRC (Quit: They came for me...)
hverkuil: <u>ribalda</u>: I just pushed additional control checks for v4l2-compliance.
<br> Specifically it tests if enumerating only compound controls (V4L2_CTRL_FLAG_NEXT_COMPOUND) works.
<br> uvc will fail on that since it doesn't check for that flag.
ribalda: <u>hverkuil</u>: it is not fare to change v4l2-compliance when I am done
<br> :P
hverkuil: Since uvc doesn't support compound controls it is easy to fix: if only the V4L2_CTRL_FLAG_NEXT_COMPOUND flag is set, just return -EINVAL.
ribalda: <u>hverkuil</u>: thanks!, I will add it to my v3
hverkuil: for both QUERYCTRL and QUERY_EXT_CTRL
ribalda: as you saw, the -M is fixed now. I am only missing the -s
<br> I hope sergey can take a look to that
<br> otherwise I might give it a chance. I think you did the 90%
<br> I really like v4l2-compliance. Thanks for it :)
kbingham: ribalda, fixing some millions of devices in one fell swoop ;-)
hverkuil: <u>ribalda</u>: I've rebased my patch and it seems to work with v4l2-compliance -s. I'll post it.
ribalda: <u>hverkuil</u>: That sounds awesome
<br> <u>hverkuil</u>: I can put it in the top of my v3 if you want
hverkuil: <u>ribalda</u>: posted. Yes, please add it to v3. I added a few TODOs in my emails that should be fixed or tested.
<br> Ah, something is missing for the metadata device node: v4l2-compliance says that streaming is not supported, that can't be right :-)
<br> Probably forgot to hook up something.
ribalda: <u>hverkuil</u>: for extra bonus points: test if unbind of the driver works while streaming
<br> you mean rmmod while using it?
<br> ricardo@piling:~/work/linux (uvc-compliance-v3)$ sudo rmmod uvcvideo
<br> <u>rmmod</u>: ERROR: Module uvcvideo is in use
hverkuil: no, I meant unbind via /sys/module/uvcvideo/drivers/usb\:uvcvideo/unbind
<br> It should work since that's basically the same as an unplug.
ribalda: ok, will try
<br> btw. I just tried v4l2-compliance master. and it works
<br> with no change in my code :S
hverkuil: I'll post a v2 of my patch, I found the metadata issue.
ribalda: unbind works (no crash)
<br> <u>hverkuil</u>: great, I am ready to send v3 when you share the patch
hverkuil: posted.
<br> <u>Note</u>: metadata only works if the chosen video format supports metadata, for my webcam I need to chose h264 as format, YUYV doesn't produce any metadata, so no buffers arrive.
<br> MJPG doesn't produce metadata either.
ribalda: <u>hverkuil</u>: https://git.kernel.org/pub/scm/linux/kernel/git/ribalda/linux.git/log/?h=uvc-compliance-v3
<br> in case you want to take a quick peek before I send it to ML
hverkuil: <u>pinchartl</u>: ribalda: is there a way to know if a certain format will produce metadata as well in UVC? It would be nice if attempts to stream metadata just fail if it is known that no metadata will be produced.
ribalda: (I have to change to a different computer to send as @chromium)
hverkuil: using a suffix to differentiate between the video types is not a good idea, they should be a prefix. If dev->name is long, then the suffix might well be cut off.
<br> Other than that it looks good (but as mentioned in the TODO list in my RFC the last patch needs more testing and can be improved a bit)
pinchartl: <u>hverkuil</u>: whether metadata can be produced depends on the device, not the pixel format
<br> if a device has no device-specific metadata, the driver can always produce standard UVC metadata
hverkuil: so there is no way of knowing this without actually trying to stream?
pinchartl: V4L2_META_FMT_UVC is always supported
ribalda: <u>hverkuil</u>: I will try on a chromeboot the suspend/resume. there is good tooling for that
<br> the unbind is already tested
<br> <u>hverkuil</u>: regarding the prefix. it has 9 spaces left than the final size for that reason
hverkuil: ah, I missed that.
<br> <u>pinchartl</u>: I'm not getting any metadata if I stream with YUYV. So if that's supposed to work, then something seems wrong (this is without any patches applied)
<br> Only with h264 as format do I receive metadata.
<br> (HD Pro Webcam C920)
pinchartl: <u>hverkuil</u>: strange. I wouldn't have expected that
hverkuil: I always assumed it was format dependent :-)
pinchartl: it's not supposed to be
<br> but it may be allowed by the UVC spec
<br> I'd expect YUYV to still generate timestamp metadata
hverkuil: <u>pinchartl</u>: if you want me to test something, just let me know.
pinchartl: <u>hverkuil</u>: well, I know I won't have time to look at this in the near future :-)
hverkuil: <u>pinchartl</u>: I'll do some digging. uvc appears to be capture some meta data, but for some reason no meta buffers are marked done.
<br> UVC_QUEUE_DROP_CORRUPTED gets set.
<br> UVC_QUEUE_DROP_CORRUPTED and buf->error
<br> so the buffer is requeued.
<br> <u>robertfoss</u>: ping
robertfoss: <u>hverkuil</u>: pong
hverkuil: <u>robertfoss</u>: quick question as a Cisco engineer (not with my reviewer hat on :-) ):
<br> Can the camss driver support RGB coming into the CSI? (So not bayer, but RGB 24 bit)
<br> I see only bayer, yuyv and luma-only support at the moment.
robertfoss: I haven't tested it, but it should work. any raw CSI dump should work
hverkuil: so it is a driver limitation, not a hardware limitation, right?
robertfoss: yes
hverkuil: Second question: the tc358840 HDMI-to-CSI bridge can use two 4-lane CSI receivers where the left part of a 4k image is transmitted to one CSI port and the right part to another.
<br> An SoC like the tegra can combine those into a single buffer. Is something like that supported in the hardware?
<br> (the tegra uses DMA to combine the two images into the single buffer)
robertfoss: I don't know, there's some documentation I dont have access to however.
<br> for gen2 devices, the csiphy/csid/vfe are physically connected as a channel
<br> so two csid blocks never connect to one vfe
hverkuil: Is sdm845 gen2 or gen3?
robertfoss: sdm845 is gen2
hverkuil: OK.
robertfoss: however it is at least possible that some some of the hardware blocks that have not been implemented could combine the output of two VFEs.
<br> but I haven't seen that scenario mentioned in any of the documentation
hverkuil: It would work if the DMA engine can support strides. I believe that's basically how it works on the Tegra. Two independent DMA engines writing to the same buffer, one writing the left part, the other the right part.
<br> (just curious if something like that can be done)
robertfoss: there is some stride support related to dealing with different channels for YUV formats
<br> but I haven't explored it, nor written any support for it in the driver
hverkuil: OK, thank you for the info!
robertfoss: So if two csi channels were to be configured properly and told to write to the same buffer, I can see something like that working.
<br> np
***: arnd has quit IRC (Changing host)
hverkuil: <u>pinchartl</u>: figured out where things go wrong with uvc metadata:
<br> uvc_video_decode_meta() has this check at the start:
<br> if (meta_buf->length - meta_buf->bytesused <
<br> length + sizeof(meta->ns) + sizeof(meta->sof)) {
<br> meta_buf->error = 1;
<br> return;
<br> }
<br> For the H264 format bytesused goes up to about 570, for YUYV it will actually go up to a bit over 5000 bytes, and for MJPG up to about 2706 bytes.
<br> Since meta_buf->length is 1024, this will fall into the if and set error to 1.
<br> Changing UVC_METADATA_BUF_SIZE to 10240 will make this work.
<br> I've no idea if this makes sense, or if there is more going on in the metadata parsing.
<br> An alternative is to remove the 'meta_buf->error = 1;' line, but then data is lost.
<br> <u>ribalda</u>: no need to mail to both hverkuil@xs4all.nl and hverkuil-cisco@xs4all.nl: they are aliases, so now I have duplicate mails :-)
<br> hverkuil@xs4all.nl is preferred.
<br> <u>ribalda</u>: meta data streaming is flaky: it works if I start streaming video first, then stream metadata, and stop streaming metadata before stopping video. Any other combination causes weird USB reset conditions. Would be good to know if you have the same issue. I'm testing on a VM instance, so that may cause additional problems.
ribalda: <u>hverkuil</u>: sorry :S
hverkuil: I'll try to see if I have the same USB issues on my laptop.
ribalda: with the 10240 hack it works really good
<br> I only get frames when there is video streaming. I can start/stop the devices in any combination
<br> no error in dmesg
<br> full disclosure: I am testing on top of the noncontiguos tree
<br> but that should not be an issue
<br> I am changing 1024 to 10240 in my v4 branch
<br> https://git.kernel.org/pub/scm/linux/kernel/git/ribalda/linux.git/commit/?h=uvc-compliance-v4&id=ab4ef30d6bd184323f7d89a42b5a8435f0c61907
hverkuil: <u>ribalda</u>: the usb issues disappear when I use a laptop instead of a VM.
<br> might be the USB passthrough or my USB hub.
ribalda: all right :)
hverkuil: I get one fail:
<br> if I stream video (v4l2-ctl --stream-mmap), then run v4l2-compliance -d1 (i.e. the metadata device), I get this:
<br> <u>fail</u>: v4l2-test-formats.cpp(452): expected EINVAL, but got 16 when getting format for buftype 13
<br> test VIDIOC_S_FMT: FAIL
ribalda: ok, will take a look
hverkuil: This *only* happens if I am streaming video. If not, then it's fine.
ribalda: for the 10240, I can either make a patch with Credits-to:, or you make it.
hverkuil: <u>ribalda</u>: just add it yourself to your branch.
<br> Note that I have no clue if it is the right (tm) thing to do. And can other uvc devices use more than 10240 bytes? It's a kind of arbitrary value based on what I observed.
ribalda: will write that in the commit message. most likely pinchartl will know what to do
hverkuil: <u>ribalda</u>: found the bug, replied to the patch.
ribalda: <u>hverkuil</u>: great! I updated by v4 branch
<br> after I get some feedback from pinchartl I resend it
<br> :) :)
hverkuil: I still get these control warnings:
<br> <u>warn</u>: v4l2-test-controls.cpp(756): g_ext_ctrls(0) invalid error_idx 0
<br> <u>warn</u>: v4l2-test-controls.cpp(823): error_idx should be equal to count
<br> <u>warn</u>: v4l2-test-controls.cpp(844): error_idx should be equal to count
<br> (plus a few -EIO warnings, but that's probably due to UVC device issues)
neg: <u>sailus</u>: ping
hverkuil: <u>ribalda</u>: actually, the controls that result in -EIO are both marked 'inactive' in 'v4l2-ctl -l'. This looks like this might be a real issue.
ribalda: the firmware does not like to write in inactive controls
hverkuil: and you shouldn't.
<br> well... let me check how that is handled elsewhere.
<br> There are two control flags that are relevant here: V4L2_CTRL_FLAG_GRABBED and V4L2_CTRL_FLAG_INACTIVE. INACTIVE means that you can write it, but the value won't be used until something else changes. In my case the Exposure Absolute control is inactive because Exposure, Auto is set to true. Only if Exposure, Auto is set to false will Exposure Absolute become active.
<br> Setting Exposure Absolute while inactive is meant to work, but it might not be actually written to the hardware until control becomes active.
<br> V4L2_CTRL_FLAG_GRABBED is different: there you can't write to the control at all. The typical use-case of that is because e.g. streaming is in progress and you are not allowed to change the control while streaming.
<br> I think that uvc should return -EACCES when an attempt is made to set inactive controls.
<br> v4l2-compliance should allow that as an error code, but only for the uvc driver.
<br> The control framework is able to handle this case more elegantly: it will remember the last set inactive value, and when the control becomes active it will update the hardware. But that's really hard to model in uvc.
ribalda: <u>hverkuil</u>: ok. I will look into that for v4
<br> will also take a look to change the v4l2-compliance,
<br> will probably do it in Monday if it fits your schedule
<br> thanks a lot for a great working day :)
hverkuil: it really wasn't what I planned to be doing today :-( I got seriously side-tracked by this.
<br> Once this is all done it would also be great to add support for uvc to the contrib/test/test-media script. Full fledged testing including dmabuf and metadata.
ribalda: I am sorry. I really apreciate your help. These are the kind of changes that pay off in the future.
pinchartl: <u>hverkuil</u>: good findings. we could indeed increase the buffer size. UVC doesn't specify a maximum size for metadata :-S
<br> we may be able to compute it based on the frame size though
<br> or at least compute a maximum value
ribalda: <u>hverkuil</u>: pinchartl: I think this is it for me today. fredagsslik time :). Have a great weekend
sailus: <u>neg</u>: Pong.
<br> Just replied. :-)
<br> <u>hverkuil</u>: Do you prefer your @cisco.com e-mail for patches? I see that SoB and From: didn't match in the IPU3 doc patch.
neg: <u>sailus</u>: Thanks
sailus: <u>hverkuil</u>: It was a CCS driver patch actually. I used the -cisco address in From: just as the other patch (and SoB lines).
***: unidan1 is now known as unidan
<br> GBenji has left