[03:07] *** mripard has quit IRC (Ping timeout: 246 seconds)
[03:07] *** unidan has quit IRC (Ping timeout: 244 seconds)
[03:07] *** bbrezillon has quit IRC (Ping timeout: 245 seconds)
[07:21] <hverkuil> mchehab: I see that patches were merged in the master repo, but again I didn't see any mails sent to linuxtv-commits. Did something break again?
[07:37] <hverkuil> mchehab: hmm, it may have been false alarm. I'll let you know if there is a real problem.
[07:45] <mripard> hverkuil: on a related note, I haven't seen the cedrus H264 PR merged anywhere?
[07:47] <hverkuil> mripard: I'm sorry, it slipped to 5.3. A Pull request for it is pending so it should be merged soon after 5.2-rc1 is released.
[07:49] <mripard> ok, thanks :)
[07:51] <mripard> having some kind of notice in those situations would be great though
[07:54] <hverkuil> My fault, I should have let you know when it became clear that Mauro didn't have the time to review/merge it for 5.2. I'll try to do better.
[07:55] <hverkuil> mchehab: do you still plan to take this PR with rockchip fixes? https://patchwork.linuxtv.org/patch/55869/
[07:55] <hverkuil> for 5.2, that is.
[09:51] <hverkuil> svarbanov: ping
[09:52] <svarbanov> hverkuil, pong
[09:56] <hverkuil> Regarding https://patchwork.linuxtv.org/patch/56079/ and https://patchwork.linuxtv.org/patch/55656/:
[09:56] <hverkuil> Can these be added to a patch series that also makes use of them?
[09:57] <hverkuil> This to avoid adding a feature that isn't actually used by a driver.
[09:58] <svarbanov> hverkuil, sure, I can add them to my stateful codec API Venus patches
[09:59] <hverkuil> svarbanov: BTW, I added more compliance tests for stateful encoding/decoding here: https://git.linuxtv.org/hverkuil/v4l-utils.git/log/?h=vicodec
[10:00] <hverkuil> It can now also handle streams with resolution changes.
[10:01] <hverkuil> I do think that we will need better support in v4l2-mem2mem.c for these devices. It's getting quite complex in vicodec to deal with the various corner cases.
[10:01] <hverkuil> But we need to finalize the spec first.
[10:01] <svarbanov> hverkuil, great I can try them on Venus
[10:02] <hverkuil> let me know the results, or if you have any questions.
[10:03] <svarbanov> hverkuil, sure
[10:53] <gtucker> hverkuil: Hello, we're finally getting the test case order fixed in the kernelci reports.  Does this look good to you?
[10:53] <gtucker> https://github.com/kernelci/kernelci-backend/pull/133#issuecomment-490853779
[10:56] <hverkuil> gtucker: that looks much better!
[10:57] <hverkuil> One quick question: SKIP relates to the number of "OK (Not Supported)" tests?
[10:58] <gtucker> hverkuil: yes
[10:59] <hverkuil> I am not sure you should see that as a separate category.
[11:00] <hverkuil> It's not quite right that it skips the test either: in some cases when it detects that the driver doesn't support that feature it does additional tests as well.
[11:00] <hverkuil> E.g. if A isn't supported, then B shouldn't be supported either, so that's then checked as well.
[11:01] <gtucker> So, should these "Not Supported" results just be discarded?
[11:01] <gtucker> For the use-case you're mentioning, I guess there should be a test to check that unsupported features are consistent
[11:02] <hverkuil> I'd just consider them a PASS.
[11:03] <gtucker> Say, a test to do sanity check the capabilities of the driver
[11:05] <hverkuil> But the compliance test does that already. Where applicable it checks if unsupported features are consistent.
[11:05] <gtucker> Right, well OK it's easy to change that to treat "not supported" as "pass" if it makes more sense.
[11:07] <gtucker> I guess the definition of "skip" is somewhat arbitrary, it seemed to me that it meant that the test case was not run because it was not applicable for any reason (say, missing capability).
[11:07] <gtucker> The main difference is that regressions won't be tracked if a test case used to be skipped and then starts failing.
[11:08] <gtucker> Whereas if it was passing and starts failing, that's tracked as a regression.
[11:08] <hverkuil> No, that's not the case. It will actually try the ioctl, then (if it is not supported) it will verify that it is correct that it is not supported. Or vice versa: it is supported, but based on other capabilities it shouldn't.
[11:09] <hverkuil> So it is a real test, and nothing is skipped.
[11:09] <hverkuil> I am missing a total for the warnings.
[11:10] <gtucker> Right, I see.  Thanks for clarifying.
[11:10] <hverkuil> I think you want to be informed when the number of warnings goes up.
[11:10] <gtucker> Ah yes, that would require more parsing, and tbh I think it's over the threshold of what can reasonably be done using the human-readable output.
[11:10] <gtucker> Having the warnings in a machine-readable format would be good though.
[11:12] <gtucker> Adding this to the backlog :)
[11:13] <hverkuil> Parsing the cec/v4l2_compliance output isn't hard. test-media has perl code to extract this: https://git.linuxtv.org/v4l-utils.git/tree/contrib/test/test-media
[11:13] <hverkuil> Line 422.
[11:29] <svarbanov> hverkuil, how you test vicodec src change event, some hints
[11:31] <svarbanov> hverkuil, --stream-from-hdr option?
[11:35] <hverkuil> svarbanov: there are two options to read from a file:
[11:35] <hverkuil> --stream-from reads a regular bitstream (e.g. h264)
[11:37] <hverkuil> --stream-from-hdr reads a stream that was created by v4l2-ctl --stream-to-hdr: it whenever v4l2-ctl received a capture buffer it writes a 32-bit header + 4 bytes for the size of the payload in the buffer ('bytesused') followed by the actual payload.
[11:38] <hverkuil> So --stream-from-hdr fills the output buffer of the decoder with exactly the same amount of data that the encoder produced.
[11:38] <hverkuil> Whereas --stream-from just fills the output buffer until it is full and then queues it to the decoder.
[11:40] <svarbanov> hverkuil, OK, we have to write some utility which could be used to reformat elementary h264 bitstream to this format, I guess
[11:41] <hverkuil> You should be able to just use --stream-from with the raw h264 stream.
[11:41] <svarbanov> hverkuil, who will split NAL units in this case
[11:41] <svarbanov> hverkuil, you have to have some basic parser I guess
[11:42] <hverkuil> Well, it's a stateful decoder, so that's the hardware (possibly assisted by the driver), isn't it?
[11:44] <svarbanov> hverkuil, I think at least for the Venus this is not possible, someone (parser) in userspace should fill input decoder buffers with one on more NAL units. Let me try
[11:45] <hverkuil> If that's the case, then we probably need to introduce a new pixelformat to indicate that userspace still needs to parse.
[11:46] <hverkuil> Or a flag for use with ENUMFMT.
[11:52] <svarbanov> hverkuil, it shouldn't be pixelformat simply because we don't have pixels, flag is better
[11:53] <svarbanov> hverkuil, weird, but I cannot see STREAMON ioctl called by the v4l2-compliance
[11:54] <svarbanov> hverkuil, I will need more time to find out what is going on
[11:55] <hverkuil> grep for streamon (it's calling a helper function in cv4l-helpers.h)
[12:11] <svarbanov> hverkuil, hmm, test MMAP (select): OK (Not Supported)
[12:12] <hverkuil> weird.
[12:24] <svarbanov> hverkuil, do you have something hardcoded related to FILE_HDR_ID
[12:25] <hverkuil> svarbanov: what do you mean?
[12:26] <svarbanov> hverkuil, nevermind :) I was thinking that you expect to see this magic in the input file, but this is not the case with --stream-from=foo.264
[12:41] <svarbanov> hverkuil, node->valid_buftypes , who is supposed to fill valid_buftypes
[12:44] <hverkuil> Can you provide the full compliance output somewhere?
[12:45] <svarbanov> hverkuil, https://paste.ubuntu.com/p/CC3kfgHwFm/
[12:46] <svarbanov> hverkuil, can be the problem kernel version? I'm on 4.14.53 when running the test
[12:47] <hverkuil> It's a knock-on effect from this fail: v4l2-test-formats.cpp(452): !pix_mp.width || !pix_mp.height
[12:48] <hverkuil> svarbanov: I can improve this a bit.
[12:53] <hverkuil> svarbanov: add this patch: https://git.linuxtv.org/v4l-utils.git/commit/?id=0d61ddede7d340ffa1c75a2882e30c455ef3d8b8
[12:53] <hverkuil> it should prevent that knock-on effect. Can you test it?
[12:54] <hverkuil> pushed the same patch to my v4l-utils repo as well.
[12:58] <svarbanov> hverkuil, it is a bit different https://paste.ubuntu.com/p/C7FPWN52B9/
[12:58] <svarbanov> hverkuil, now I see that vb2::queue_setup returns EINVAL
[13:00] <hverkuil> that's a lot better. It's now trying to stream.
[13:00] <svarbanov> hverkuil, not yet, reqbuf failed
[13:02] <svarbanov> hverkuil, infact I cannot see REQBUF(OUTPUT)
[13:02] <svarbanov> hverkuil, meeting, will continue later
[13:04] <hverkuil> q.reqbufs calls VIDIOC_REQBUFS. See cv4l-helpers.h + v4l-helpers.h.
[13:35] <svarbanov> hverkuil, the sequence in testMmap is wrong, we have to reqbuf(output) first, but the code is trying reqbuf(capture) because it is first in an enum v4l2_buf_type
[13:37] <svarbanov> hverkuil, probably we need different testMmap for decoders/encoders
[13:53] <hverkuil> svarbanov: actually, it is legal to call reqbuf(capture) first. If the resolution of the capture buffers is different from what it actually in the stream, then a SOURCE_CHANGE event should be issued, just as if there was a midstream resolution change.
[13:57] <svarbanov> hverkuil, hmm, from my reading of the spec reqbuf(capture) should return an error
[13:58] <svarbanov> hverkuil, I think so because by this way/sequence you missed Initialization procedure
[13:59] <hverkuil> That was in an older version, but such exceptions actually make life harder.
[14:00] <hverkuil> It's still a draft version, and there are more changes planned. I hope tfiga has time soon to post a new version.
[14:01] <hverkuil> It's the same with returning a format with width/height set to 0/0.  It would be inconsistent with the general V4L2 rule to never return an invalid format.
[14:02] <hverkuil> svarbanov: the latest versions of the codec specs are here: https://hverkuil.home.xs4all.nl/codec-api/uapi/v4l/dev-mem2mem.html
[14:03] <hverkuil> As mentioned, this isn't the final version yet.
[14:03] <svarbanov> hverkuil, ok, then the state machine should be changed too, because I strictly follow it
[14:04] <svarbanov> hverkuil, it this case I don't know what we testing at all :)
[14:06] <hverkuil> brb
[14:09] <hverkuil> The compliance test is based on the latest stateful codec spec + changes discussed in mailthreads on the mailinglist.
[14:09] <hverkuil> I hoped tfiga was able to post a new version before he left on vacation, but that didn't happen.
[14:10] <hverkuil> It's also the reason why these compliance tests aren't pushed in the main v4l-utils repo: they aren't against the final spec.
[15:36] *** benjiG has left 
[17:03] *** BenG83 has left "Leaving"
[21:02] *** ldts has quit IRC (Ping timeout: 258 seconds)
[22:35] *** arnd has quit IRC (Ping timeout: 258 seconds)