<!-- 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>
***: camus has quit IRC (Ping timeout: 480 seconds)
<br> jernej_ has quit IRC ()
<br> jernej has joined #linux-media
<br> milek7 has quit IRC (Remote host closed the connection)
<br> milek7 has joined #linux-media
<br> camus has joined #linux-media
<br> camus1 has quit IRC (Read error: Connection reset by peer)
<br> bingbu has quit IRC (Ping timeout: 480 seconds)
<br> bingbu has joined #linux-media
<br> eelstrebor has quit IRC (Quit: Ex-Chat)
<br> camus1 has joined #linux-media
<br> camus has quit IRC (Ping timeout: 480 seconds)
<br> bingbu has quit IRC (Ping timeout: 480 seconds)
<br> bingbu has joined #linux-media
<br> camus has joined #linux-media
<br> camus1 has quit IRC (Read error: Connection reset by peer)
<br> camus1 has joined #linux-media
<br> camus has quit IRC (Ping timeout: 480 seconds)
<br> camus1 has quit IRC (Remote host closed the connection)
<br> camus has joined #linux-media
<br> fleebs has quit IRC (Remote host closed the connection)
<br> bingbu has quit IRC (Ping timeout: 480 seconds)
<br> bingbu has joined #linux-media
<br> camus1 has joined #linux-media
<br> camus has quit IRC (Ping timeout: 480 seconds)
<br> GBenji has joined #linux-media
<br> jm_h has joined #linux-media
<br> djrscally has joined #linux-media
<br> ao2 has joined #linux-media
<br> svarbanov has joined #linux-media
<br> camus has joined #linux-media
<br> hiroh has joined #linux-media
<br> camus1 has quit IRC (Read error: Connection reset by peer)
<br> ao2 has quit IRC (Quit: Leaving)
<br> camus1 has joined #linux-media
<br> camus has quit IRC (Ping timeout: 480 seconds)
<br> johancb has joined #linux-media
<br> johancb has quit IRC ()
mchehab: <u>hverkuil</u>: ping
***: camus has joined #linux-media
<br> camus1 has quit IRC (Ping timeout: 480 seconds)
<br> mairacanal has joined #linux-media
<br> mairacanal has quit IRC ()
hverkuil: <u>mchehab</u>: pong
***: xroumegue has quit IRC (Quit: WeeChat 2.3)
<br> xroumegue has joined #linux-media
<br> camus1 has joined #linux-media
mchehab: <u>hverkuil</u>: I found a bug when testing atomisp which actually seems to be a V4L2 core issue
<br> basically, the subdev wrapper code calls call_enum_mbus_code() for enum mbus formats
***: camus has quit IRC (Ping timeout: 480 seconds)
mchehab: this function internally calls check_state_pads()
<br> which seems to be the wrong thing to do... as enumerating formats should not depend on .which being TRY_FORMAT or ACTIVE
<br> static int check_state_pads(u32 which, struct v4l2_subdev_state *state)
<br> {
<br> if (which == V4L2_SUBDEV_FORMAT_TRY && (!state || !state->pads))
<br> return -EINVAL;
<br> return 0;
<br> }
<br> (btw, just returning a generic error code there without printing anything sounds a bad idea... it took me a lot of time to test where this was failing)
hverkuil: https://hverkuil.home.xs4all.nl/spec/userspace-api/v4l/vidioc-subdev-enum-mbus-code.html: "Available media bus formats may depend on the current ‘try’ formats at other pads of the sub-device, as well as on the current active links."
mchehab: well, the way this is implemented is not "may" but, instead, "shall"
<br> I mean, it is just returning -EINVAL if !state
<br> (atomisp calls it with state = NULL)
<br> I have already a fix for atomisp, but it sounds that the change introduced by Changeset 374d62e7aa50 ("media: v4l2-subdev: Verify v4l2_subdev_call() pad config argument") may also cause other things to break
hverkuil: If I remember correctly, then !state just means that the driver doesn't support TRY at all. I.e., it never reserved space for the 'try' data structures. So I think it makes sense to reject this. The fix would be to properly support try formats in the driver.
mchehab: it is too early for that... see, atomisp calls it at VIDIOC_ENUM_FMT time
hverkuil: What is however not clear in the doc is that EINVAL can be returned if TRY isn't supported by the driver at all (as appears to be the case here).
<br> which subdev is failing here?
mchehab: ov2680
hverkuil: where is it called from in atomisp?
mchehab: atomisp_enum_fmt_cap()
<br> basically, this is broken: v4l2-ctl --list-formats
hverkuil: drivers/media/i2c/ov2680.c or drivers/staging/media/atomisp/i2c/atomisp-ov2680.c ? (the latter, I expect)
mchehab: it causes -EINVAL
<br> the staging one
<br> but the problem is not at the sensor driver
<br> what atomisp does is:
<br> rval = v4l2_subdev_call(camera, pad, enum_mbus_code, &state, &code);
<br> then it converts the mbus format into a fourcc
<br> after fixing it, it works fine:
<br> $ v4l2-ctl --list-formats -d /dev/video3
<br> <u>ioctl</u>: VIDIOC_ENUM_FMT
<br> <u>Type</u>: Video Capture
<br> [0]: 'YU12' (Planar YUV 4:2:0)
<br> [1]: 'YV12' (Planar YVU 4:2:0)
<br> [2]: '422P' (Planar YUV 4:2:2)
<br> [3]: 'Y444' (16-bit A/XYUV 4-4-4-4)
<br> [4]: 'NV12' (Y/CbCr 4:2:0)
<br> [5]: 'NV21' (Y/CrCb 4:2:0)
<br> [6]: 'NV16' (Y/CbCr 4:2:2)
<br> [7]: 'YUYV' (YUYV 4:2:2)
<br> [8]: 'UYVY' (UYVY 4:2:2)
<br> [9]: 'UYVY' (UYVY 4:2:2)
<br> [10]: 'BG10' (10-bit Bayer BGBG/GRGR)
<br> [11]: 'RGB4' (32-bit A/XRGB 8-8-8-8)
<br> [12]: 'RGBP' (16-bit RGB 5-6-5)
hverkuil: No, the problem is in atomisp_enum_fmt_cap: it should set code.which to V4L2_SUBDEV_FORMAT_ACTIVE.
mchehab: [13]: 'JPEG' (JFIF JPEG, compressed)
hverkuil: VIDIOC_ENUM_FMT only enumerates active formats, but 'code = { 0 }' sets code.which to 0 == TRY.
mchehab: yeah, that would solve, but still it sounds stupid, as it doesn't make any sense to try enum
<br> (btw i did a fix like that already)
<br> actually the same way as via driver does: setting a fake .pads
<br> - struct v4l2_subdev_mbus_code_enum code = { 0 };
<br> + struct v4l2_subdev_pad_config pad_cfg = { 0 };
<br> + struct v4l2_subdev_mbus_code_enum code = {
<br> + .which = V4L2_SUBDEV_FORMAT_TRY,
<br> + };
<br> + struct v4l2_subdev_state state = {
<br> + .pads = &pad_cfg
<br> + };
hverkuil: No, that's wrong. Just set .which to V4L2_SUBDEV_FORMAT_ACTIVE, that's all you need to do.
<br> ENUM_FMT operates on active formats, so TRY is just wrong.
mchehab: that's my point... does it make sense to even check .which for ENUM_FMT?
hverkuil: Yes, because the subdev driver that is called may be used in a MC-centric driver as well where TRY *does* make sense.
mchehab: ok. fix sent to ML
hverkuil: <u>mchehab</u>: updated media_build as well, should compile again.
mchehab: good
***: camus has joined #linux-media
<br> camus1 has quit IRC (Read error: Connection reset by peer)
<br> mugnierb2 is now known as mugnierb
<br> eelstrebor has joined #linux-media
<br> mairacanal has joined #linux-media
<br> mairacanal has quit IRC (Remote host closed the connection)
<br> sailorek1234 has joined #linux-media
<br> camus1 has joined #linux-media
<br> camus has quit IRC (Ping timeout: 480 seconds)
<br> sailorek1234 has quit IRC (Quit: sailorek1234)
<br> sailorek1234 has joined #linux-media
<br> sailorek1234 has quit IRC ()
<br> camus has joined #linux-media
<br> camus1 has quit IRC (Ping timeout: 480 seconds)
<br> mtretter has quit IRC ()
<br> mtretter has joined #linux-media
<br> GBenji has quit IRC (Quit: Leaving.)
<br> gouchi has joined #linux-media
<br> camus1 has joined #linux-media
<br> camus has quit IRC (Ping timeout: 480 seconds)
<br> jarthur has joined #linux-media
<br> bingbu has quit IRC (Ping timeout: 480 seconds)
<br> bingbu has joined #linux-media
<br> macc24_ has joined #linux-media
<br> macc24 has quit IRC (Ping timeout: 480 seconds)
<br> ao2 has joined #linux-media
<br> camus has joined #linux-media
<br> camus1 has quit IRC (Ping timeout: 480 seconds)
<br> macc24_ has quit IRC (Ping timeout: 480 seconds)
<br> camus1 has joined #linux-media
<br> jm_h has quit IRC (Quit: Leaving)
<br> camus has quit IRC (Ping timeout: 480 seconds)
<br> gouchi has quit IRC (Remote host closed the connection)
<br> ao2 has quit IRC (Quit: Leaving)
pinchartl: <u>mchehab</u>: enumerating formats can depend on the state
<br> for instance, when enumerating formats on source pads, the formats depends on the format of the sink pad(s)
<br> for sensors with a single source pad that shouldn't matter
<br> but I think we should enforce a valid state being passed to all operations regardless
<br> Tomi's ongoing work on subdev states will make all this easier
***: montjoie has joined #linux-media
<br> montjoie_ has quit IRC (Ping timeout: 480 seconds)
<br> djrscally has quit IRC (Quit: Konversation terminated!)
<br> djrscally has joined #linux-media
<br> camus has joined #linux-media
<br> camus1 has quit IRC (Ping timeout: 480 seconds)
<br> svarbanov has quit IRC (Ping timeout: 480 seconds)
<br> jarthur has quit IRC (Quit: Textual IRC Client: www.textualapp.com)
<br> djrscally has quit IRC (Ping timeout: 480 seconds)
<br> camus1 has joined #linux-media