| ↑back Search ←Prev date Next date→ Show only urls | (Click on time to select a line by its url) |
| Who | What | When |
|---|---|---|
| *** | BrianG61UK has quit IRC (Read error: Connection reset by peer) | [00:36] |
| ......... (idle for 42mn) | ||
| lexano has quit IRC (Ping timeout: 480 seconds) | [01:18] | |
| ....... (idle for 34mn) | ||
| cphealy has quit IRC (Ping timeout: 480 seconds) | [01:52] | |
| ten15723774324630506 has quit IRC (Remote host closed the connection)
ten15723774324630506 has joined #linux-media | [01:58] | |
| ...... (idle for 27mn) | ||
| cphealy has joined #linux-media | [02:26] | |
| ................. (idle for 1h20mn) | ||
| eelstrebor has quit IRC (Quit: Ex-Chat) | [03:46] | |
| ............................... (idle for 2h31mn) | ||
| djrscally has joined #linux-media | [06:17] | |
| ..... (idle for 21mn) | ||
| frieder has joined #linux-media | [06:38] | |
| ......... (idle for 40mn) | ||
| ao2 has joined #linux-media | [07:18] | |
| ...................... (idle for 1h49mn) | ||
| hverkuil | jmondi: I am indeed finding a bunch of bugs (i.e. missing tests) in how v4l2-compliance checks CREATE_BUFS. Stay tuned :-) | [09:07] |
| jmondi | hverkuil: I am ;) | [09:09] |
| ......................... (idle for 2h1mn) | ||
| hverkuil | jmondi: I pushed my v4l2-compliance fixes to the v4l-utils repo. It is a lot better now, and it should find the issues I reported with the v9 series.
In fact, I found a bunch of vivid bugs as well, so I also posted fixed for that driver. Please test and check that v9 indeed fails with the new compliance test. I was pleased that no bugs in vb2 were found, it was all driver bugs. | [11:10] |
| ..... (idle for 20mn) | ||
| *** | lexano has joined #linux-media | [11:32] |
| hverkuil | jmondi: also, FYI, if your driver creates a /dev/mediaX device, then you can just run 'v4l2-compliance -m0' to test /dev/media0 and all found v4l (subdev) devices.
No need to test each video device separately. | [11:39] |
| pinchartl | that's what I do and it's quite useful | [11:46] |
| jmondi | hverkuil: thanks, I'll test v9 and will make sure the errors are gone on v10
ah nice, I usually test subdevs manually :/ | [11:56] |
| hverkuil | bonus points: -m0 will also do a compliance test of the media device :-) | [12:01] |
| pinchartl | an opportunity for more errors :-D | [12:08] |
| *** | tmerciai has quit IRC (Ping timeout: 480 seconds) | [12:15] |
| ..... (idle for 20mn) | ||
| tmerciai has joined #linux-media | [12:35] | |
| ...... (idle for 27mn) | ||
| BrianG61UK has joined #linux-media | [13:02] | |
| ................ (idle for 1h19mn) | ||
| jmassot_ has joined #linux-media | [14:21] | |
| jmassot has quit IRC (Ping timeout: 480 seconds) | [14:27] | |
| jmondi | hverkuil: failure detected by
fail: v4l2-test-buffers.cpp(895): q.create_bufs(node, 1, &fmt) != EINVAL | [14:37] |
| hverkuil | jmondi: with v9? | [14:41] |
| jmondi | yes
however I'm not sure I understand the test that fails this fmt.s_sizeimage(65536, fmt.g_num_planes() - 1); doesn't seem to match the comment that says " should we allow CREATE_BUFS to create buffers with more planes than required" ? while it seems you're just setting 65536 as the size image of the last valid plane ? | [14:41] |
| hverkuil | First it tests adding an extra plane, but setting the size to 0, which should fail.
Then it sets that size to 65536, and it tries again. Now it should fail because more planes are specified than the format requires. | [14:43] |
| jmondi | ah the plane number is retained from the above test
fmt.s_num_planes(fmt.g_num_planes() + 1); | [14:44] |
| hverkuil | Perhaps I should reset the format between the two tests, I agree it is a bit unexpected. | [14:44] |
| jmondi | I suspect the reason it fails it's either because an invalid number of planes, or an invalid 65536 size
also, if I'm not mistaken fmt.s_num_planes(fmt.g_num_planes() + 1); fmt.s_sizeimage(0, fmt.g_num_planes() - 1); yes, you're adding one extra plane but setting size to 0 to the last valid one ? unless fmt.s_num_planes() modifies the number of planes returned by fmt.g_num_planes() :) which probably does | [14:45] |
| hverkuil | There are two things to check: the vb2 core will check if all planes have a non-0 size as passed by CREATE_BUFS. So if the last plane has a 0 size, then vb2 must catch that and return EINVAL. That's the first test. | [14:47] |
| jmondi | anyway, let me see if with v10 the error is gone | [14:47] |
| hverkuil | The second test requires the driver to check if the requested number of planes matches the current format. | [14:48] |
| *** | gib2615[m] has quit IRC (Read error: Connection reset by peer)
ThomasDevoogdt[m] has quit IRC (Read error: Connection reset by peer) joyoseller[m] has quit IRC (Read error: Connection reset by peer) yhzte[m] has quit IRC (Read error: Connection reset by peer) jjtech[m] has quit IRC (Read error: Connection reset by peer) Tooniis[m] has quit IRC (Write error: connection closed) robertmader[m] has quit IRC (Write error: connection closed) z3ntu has quit IRC (Write error: connection closed) jack_kekzoz[m] has quit IRC (Write error: connection closed) tomba has quit IRC (Write error: connection closed) KieranBingham[m] has quit IRC (Write error: connection closed) | [14:48] |
| jmondi | hverkuil: Grand Total for pispbe device /dev/media1: 392, Succeeded: 392, Failed: 0, Warnings: 0
v10 on its way | [15:02] |
| pinchartl | jmondi: nice! | [15:10] |
| hverkuil | pinchartl: jmondi: I actually recommend that you test other drivers that you maintain as well for bugs in the CREATE_BUFS implementation, using the new v4l2-compliance. The tests should catch a lot more corner cases.
I found an embarrassing number of bugs in vivid, so it would not surprise me if there are more problems in existing drivers. | [15:17] |
| kbingham | how do we get this automated ;-) | [15:27] |
| pinchartl | hverkuil: I'll give t a try when I can. at the latest when I send patches with significant changes
kbingham: fully agreed | [15:28] |
| hverkuil | Yes, if you are working on a driver, just give it a spin to check the code. | [15:30] |
| *** | BrianG61UK has quit IRC (Remote host closed the connection) | [15:32] |
| ..... (idle for 21mn) | ||
| jmassot has joined #linux-media | [15:53] | |
| jmassot_ has quit IRC (Ping timeout: 480 seconds) | [15:58] | |
| jmondi | hverkuil: sailus: v10of the pispbe out, hope we're still in time for v6.11 ! | [16:02] |
| hverkuil | sailus: if you can make a PR tomorrow morning, then I'll process it in the afternoon. Alternatively, if you don't have the time, then I can also make a PR.
jmondi: ah, you missed a bunch of comment I made in my v9 review that came after the queue_setup function. sorry, a v11 will be needed. Actually, that was another weird thing that v4l2-compliance apparently missed: s_fmt didn't check for vb2_is_busy(), but I thought v4l2-compliance had a test for that. I'll check v4l2-compliance, perhaps there is a bug there. | [16:05] |
| It's missing a test for that :-( I'd have sworn it was there, but it really isn't. | [16:18] | |
| *** | frieder has quit IRC (Remote host closed the connection) | [16:23] |
| jmondi | I indeed didn't read the full email, I thought it was only about queue_setup() :(
sorry, I'll work on a v11 | [16:26] |
| .... (idle for 16mn) | ||
| hverkuil | working on a v4l2-compliance update for the s_fmt/EBUSY issue. | [16:45] |
| jmondi | that I noticed was missing on another driver I was working recently
and already mainlined so it's indeed a welcome addition so sorry for missing the rest of the email, I really didn't understand there were multiple comments I have all fixes ready, but I want to test a bit more carefully a few corner cases, so probably something for tomorrow | [16:50] |
| hverkuil | I typically end my last comment with a 'Regards, Hans', so if you don't see that, then there are more comments :-) | [16:52] |
| jmondi | now I noticed ;) | [16:52] |
| hverkuil | I'm not pushing my v4l2-compliance change for EBUSY: it needs more work. Which is probably why I never added that test, it is not as easy as it looks at first sight.
You need to somehow create a new valid format that is different from the current format. And do that for all buffer types. | [16:59] |
| jmondi | why does it have to be different ? the driver should refuse s_fmt regardless, doesn't it ? | [17:05] |
| hverkuil | Not necessarily. If there is only a single fixed format for example, then it is perfectly fine to set the fmt: nothing changes.
Not uncommon, esp. for non-video device nodes (e.g. VBI). Also, it is allowed to change the format in certain cases as long as the total buffer size doesn't become larger. | [17:06] |
| jmondi | true that | [17:07] |
| hverkuil | I need to think about this a bit more. I might implement it just for video nodes initially. | [17:09] |
| *** | aedancullen has joined #linux-media | [17:17] |
| jmondi | hverkuil: still there ? | [17:26] |
| *** | rauji____ has quit IRC () | [17:30] |
| hverkuil | yes | [17:30] |
| jmondi | hverkuil: just to make sure, from your v9 comments, can I remove initializing cap->bus_info, capabilities and device_caps from the querycap handler ?
I chased it a bit in the core and I'm quite sure I can remove them but a lot of drivers initialize it, so... | [17:32] |
| hverkuil | Yes, they can be removed. | [17:33] |
| jmondi | thanks for confirming | [17:33] |
| hverkuil | Many older drivers predating this core code still set it, but newer drivers shouldn't. | [17:33] |
| ......... (idle for 40mn) | ||
| *** | gouchi has joined #linux-media | [18:13] |
| jmondi | hverkuil: sailus let's see if v11 is the right one ;) | [18:15] |
| .... (idle for 18mn) | ||
| sailus | X-)
I'll pick them tomorrow morning, there's a few other fairly trivial patches I'll take at the same time. | [18:33] |
| .................. (idle for 1h29mn) | ||
| *** | ao2 has quit IRC (Quit: Leaving) | [20:03] |
| .............. (idle for 1h5mn) | ||
| gouchi has quit IRC (Remote host closed the connection)
danitool has joined #linux-media | [21:08] | |
| ............. (idle for 1h1mn) | ||
| cyrinux30 has quit IRC ()
cyrinux30 has joined #linux-media | [22:12] | |
| djrscally has quit IRC (Ping timeout: 480 seconds) | [22:26] | |
| ......... (idle for 40mn) | ||
| BrianG61UK has joined #linux-media | [23:06] | |
| ....... (idle for 32mn) | ||
| lexano has quit IRC (Ping timeout: 480 seconds) | [23:38] | |
| ↑back Search ←Prev date Next date→ Show only urls | (Click on time to select a line by its url) |