[00:44] *** djrscally has quit IRC (Ping timeout: 480 seconds) [00:44] *** camus1 has joined #linux-media [00:49] *** camus has quit IRC (Ping timeout: 480 seconds) [01:07] *** svarbanov has quit IRC (Ping timeout: 480 seconds) [01:16] *** svarbanov has joined #linux-media [02:01] *** svarbanov has quit IRC (Ping timeout: 480 seconds) [02:28] *** BrianG61UK_ has quit IRC (Read error: Connection reset by peer) [02:53] *** camus has joined #linux-media [02:58] *** camus1 has quit IRC (Ping timeout: 480 seconds) [03:10] *** jarthur has quit IRC (Quit: Textual IRC Client: www.textualapp.com) [03:10] *** eelstrebor has quit IRC (Quit: Ex-Chat) [03:53] <gnurou> ndufresne: at this point there is more dust than carpet anyway ;) [03:53] *** camus has quit IRC (Read error: Connection reset by peer) [03:53] *** camus has joined #linux-media [03:54] <gnurou> ndufresne: but I guess the suggestion of adding a flag to inform that CAPTURE buffer can be safely written into would work, wouldn't it? [03:54] <gnurou> in this case, user-space that want to be on the safe side or doesn't need to write into these buffers can just ignore the flag and follow the documented behavior [03:55] <gnurou> which is the only safe behavior at the moment anyway [03:56] <gnurou> so I don't see any problem with adding these two paragraphs to the spec? [03:59] *** fleebs has joined #linux-media [04:07] *** fleebs has quit IRC (Quit: fleebs) [04:34] *** camus1 has joined #linux-media [04:39] *** camus has quit IRC (Ping timeout: 480 seconds) [05:01] *** jarthur has joined #linux-media [05:12] *** camus has joined #linux-media [05:17] *** camus1 has quit IRC (Ping timeout: 480 seconds) [06:21] *** camus1 has joined #linux-media [06:21] *** camus has quit IRC (Ping timeout: 480 seconds) [07:07] *** ao2 has joined #linux-media [07:25] *** sailorek1234 has joined #linux-media [07:27] *** gouchi has joined #linux-media [07:27] *** gouchi has quit IRC (Remote host closed the connection) [07:31] *** camus has joined #linux-media [07:32] *** camus1 has quit IRC (Ping timeout: 480 seconds) [07:35] *** cphealy_ has joined #linux-media [07:40] *** cphealy has quit IRC (Ping timeout: 480 seconds) [07:55] *** djrscally has joined #linux-media [08:14] *** paulk2 has quit IRC (Quit: WeeChat 3.3) [08:15] *** mripard has joined #linux-media [08:21] *** GBenji has joined #linux-media [08:22] <jmondi> Eugen_H: format verification is perfromed by the media-controller framework in media_pipeline_start() which walks the graph and calls link_validate() [08:23] <jmondi> link_validate() it's a pad op and you can override it if you want in your driver [08:26] <jmondi> and it is again responsibility of userspace to set the right formats on each subdevice along the pipeline. In your example, 640x480 is the output format which is produced by down-scaling the sensor produced frame ? [08:31] <Eugen_H> jmondi: no, it's the format that the userspace configures with --set-fmt-video to the top /dev/video0 [08:36] <jmondi> sure, but how is that produced in your river ? What does your top driver represents ? [08:37] <jmondi> s/river/platform [08:38] <Eugen_H> my top driver captures the frame from the sensor and then just converts it to data and a dma engine dumps it to memory [08:38] <jmondi> I mean, is there be a downscale/crop happening somewhere between the sensor produced frame and the frame stored in memory ? Is your top driver that does so, the CSI-2 receiver or a scaler ? [08:38] <Eugen_H> so the frame size is given by the sensor frame [08:39] <Eugen_H> it's just a basic receiver but has a size limit on the frame , because of internal buffers. so it will crop it if it's exceeding the maximum size, but that's all about the cropping [08:39] <jmondi> no crop/downscale ? You platform will save to memory frames in the format produced by the sensor and that's it ? [08:40] <jmondi> format and size I meant [08:40] <Eugen_H> yes, exactly [08:40] <Eugen_H> it can perform some format conversions, but even if it does, it will not change the frame size [08:40] <Eugen_H> e.g. it will interpolate BAYER raw to RGB [08:40] <jmondi> so how can you produce 640x480 if the sensor is configured to produce 752x480 ? [08:40] <Eugen_H> it cannot. so it has to produce 752x480 [08:41] <jmondi> or the sensor has to produce 640x480 [08:41] <Eugen_H> but the top video driver does not know what frame size the sensor produces. so it informs userspace that it can be configured 16x16 up to 2900x1440 [08:41] <Eugen_H> so the userspace can configure anything, 640x480, or 2900x1400, regardless [08:42] <Eugen_H> the problem is that when the sensor starts, the sensor will throw 752x480 , but the userspace configured the top driver at an arbitrary resolution [08:42] <jmondi> userspace that knows that your top-driver cannot downscale should configure the sensor with the same frame size requested to the top driver [08:43] <Eugen_H> that's right. but what happens if the userspace does not ? [08:43] <Eugen_H> i just use v4l2-ctl --set-video-fmt to 640x480, my top video driver says it's fine, because it does not know what is the sensor frame size. [08:44] <Eugen_H> when streaming starts, the frame is bigger, and the userspace expects 640x480 but the frame is 752x480 [08:44] <Eugen_H> thus the buffer is overflown [08:45] <jmondi> ok, so if I got you right, the issue is that you have a sensor subdev directly connected to the video device that represents the dma-engine [08:45] <jmondi> an media_pipeline_start() cannot verify that the formats match as you only have a subdev [08:46] <jmondi> s/a subdev/one subdev/ [08:47] <Eugen_H> the issue is that when setting the format on the top video driver (--set-fmt-video); the sensor may or may not be configured at the frame size that it will stream. meaning the userspace can change the frame size *after* the top video driver is configured [08:47] <Eugen_H> reconfiguring the sensor at a different frame size and then starting streaming will make the userspace unaware of the real frame size that is coming in [08:47] <Eugen_H> the only solution I found so far is to return an error at streamon() if the sensor is emitting a frame that is a different size than what I would expect to receive [08:48] <Eugen_H> https://elixir.bootlin.com/linux/latest/source/drivers/media/platform/vsp1/vsp1_video.c#L81 [08:48] <Eugen_H> something similar with what is done here [08:49] *** svarbanov has joined #linux-media [08:50] <jmondi> I think in this case it's acceptable to call the remote subdev g_fmt() to verify the format [08:50] <Eugen_H> okay, I am doing that. My idea was to inform the userspace of frame size change. what is usually done in s_fmt, when userspace is requesting one format, but I can change the frame size and return new values [08:50] <jmondi> however your userspace should be aware that it apply to the sensor the same frame size as requested to the video node [08:51] <jmondi> and your pipeline handler (if you plan to work on that) is in control of that [08:51] <Eugen_H> yes, userspace should be aware, but I have to be careful about all possible cases, like what happens when the video node has another framesize than the sensor [08:53] <jmondi> is this a CSI-2 platform ? [08:53] <Eugen_H> in this specific case it's a parallel bus interface [08:57] <jmondi> ok, I was surprised as I thought CSI-2 receivers can usually perform at least some cropping [08:58] <Eugen_H> The hardware allows me to do more cropping. just that it's not implemented completely, and I would like to just pass the exact frame from the sensor at the moment, not crop it [08:59] <Eugen_H> I am also thinking about the usual dumb user, who might complain that he is using a high resolution sensor but our hardware outputs just 640x480 :)) [08:59] <Eugen_H> (because he can't configure a higher frame size and our driver just crops everything down to the default VGA res ) [09:49] *** wwilly has joined #linux-media [11:05] *** b-rad has quit IRC (Ping timeout: 480 seconds) [12:16] <ndufresne> gnurou: strictly speaking no, there is no problem, but it leaves application/software that would like to know when it can modify the buffer without any option, v4l2 ecosystem also does not provide any fast cloning of pictures, so there is really a gap there [12:17] <gnurou> right, what I mean is that we can probably easily add such a flag to the v4l2_buffer buffer. Do you think that would solve the problem, at least partially? [12:21] <ndufresne> at least from my point of view, it would do [12:22] <ndufresne> What it does is that it will inform my software, so I can export DMABuf FDs in RO, and also flag it in my abstraction [12:22] <gnurou> ok, let me try to spin a v2 that adds this flag, then the driver can set it if relevant for them [12:23] <ndufresne> gnurou: perhaps its worth mentionning what it does not do, I think what it does not do, is that it does not inform userspace how to configure the decoder to get writable buffers [12:23] <ndufresne> but I don't really have solution or use case to that ... [12:24] <gnurou> mmm are there decoders that support making optional copies of reference frames so userspace can write the buffers? That sounds very very specific to me [12:27] <ndufresne> As HW evolve, it looks like most decoders are aiming toward native tiled or compressed format, and any other formats are copy [12:27] <ndufresne> but Hantro and rkvdec does have that feature, you can produce read-only or read-write (secondary) NV12 [12:28] <ndufresne> as I said, I don't think v4l2 API can distringuish between two NV12 formats here [12:28] <ndufresne> this looks like a bigger work, and perhaps very niche [12:30] <ndufresne> on VA API, as an example, there is a clone function, though as drivers are GPU driven, the linear formats as references frame storage are long time gone [12:31] <ndufresne> gnurou: the clone function is mostly used for RW buffers when decoding VP9, since in VP9 you can get a partial header that tells you to output one of the reference, and that can be outputed multiple time [12:33] <ndufresne> (though, most encoders shares logic with hevc, and will simulate b-frames with that mechanism) [12:37] <gnurou> I see. It looks like there are two different things that we can signal here: [12:37] <gnurou> 1) The global ability of a decoder to produce writable buffers for the client, for a particular format/modifier combo, and [12:37] <gnurou> 2) Whether a particular frame of a decoded stream can be written into because it is not used as reference (I don't know if this particular ability could be required, but that's what I thought of first) [12:37] <gnurou> I was initially thinking that we want to support 2), but actually 1) is probably more useful, right? In this case the ability should probably be signaled when querying/setting a format, not when dequeuing frames [12:41] <ndufresne> ah sorry, for 2), in the case of stateless decoder, the driver is not aware, we just pick from the reference, but we have no mechanism to copy it [12:41] <gnurou> guess this is something we could consider to add along with modifiers support (not sure what happened to them) [12:42] <ndufresne> but for 2) in case of stateful decode, I really don't know, we can't really return the same buffer in the way vb2 works, so I guess there is a copy happening internally somewhere ... [12:43] <gnurou> for stateless decoders the client manages the reference frames itself, so IIUC the problem does not apply at all - this is a purely stateful feature. [12:43] <ndufresne> gnurou: modifier support was part of v4l2_buffer_ext proposal, currently on hold, Helen's left the profession, so we need a new volunteer [12:44] <ndufresne> well, for stateless, the driver may shadow the buffer, if the formats require a PP [12:44] <ndufresne> post processor [12:44] <ndufresne> but it is not implicated in 2) [12:45] <ndufresne> I think 1) is more useful, the question I was asking was basically were that flag would be [12:45] <ndufresne> svarbanov seemed to prefer v4l2_buffer.flags, and I tend to agree [12:46] <ndufresne> so the flow would be, the usuall, S_FMT, REQBUFS(), and the information would be known through QUERYBUF before doing mmap/expbuf [12:46] <ndufresne> specific to stateful decoder, it would be SRC_CH event, G_FMT, REQBUF, QUERYBUF [12:47] <ndufresne> and as usual, we can try another format too, which may lead to different buffer property [12:47] *** camus1 has joined #linux-media [12:48] <gnurou> if the capability is global to the format, it seems a bit wasteful to signal it in v4l2_buffer though... [12:48] <gnurou> unless we plan of doing 2), i.e. allow decoders that don't do any copy to signal non-reference frames as writable. Is that useful to anyone, who knows :) [12:50] <ndufresne> a small subtility, we aren't going to change the flags at run-time, so what you get at QUERYBUF, right after you have allocated the buffer will stay [12:51] <ndufresne> but indeed, to signal 2), but a bit theoritical, we'd need to set the flag per DQBUF [12:52] *** camus has quit IRC (Ping timeout: 480 seconds) [12:52] <ndufresne> but I guess you start to see why for 1, I believe that you can flag it in S_FMT structure, REQBUFS structure, or QUERYBUF structure (v4l2_buffer) ? [12:52] <ndufresne> they are pretty equal to me, but again, svarbanov and some other dev had a preference to v4l2_buffer, perhaps they have a use case for 2) ? [12:57] <gnurou> let's ask - in any case, I think we can agree that the default behavior should be the safest, i.e. assume that buffers are not writable unless being told the contrary? [13:16] <ndufresne> yeah, it is the common valid behaviour [13:21] *** eelstrebor has joined #linux-media [14:11] *** tomeu has joined #linux-media [14:39] *** sailorek1234 has quit IRC () [15:16] *** camus1 has quit IRC () [15:18] *** mugnierb2 has quit IRC (Quit: WeeChat 2.8) [15:19] *** mugnierb has joined #linux-media [15:36] *** ao2 has quit IRC (Remote host closed the connection) [15:37] *** BrianG61UK has quit IRC (Quit: Leaving) [15:42] *** camus has joined #linux-media [16:06] *** GBenji has quit IRC (Quit: Leaving.) [17:18] *** svarbanov has quit IRC (Ping timeout: 480 seconds) [17:32] *** camus has quit IRC () [17:37] *** gouchi has joined #linux-media [17:40] *** andrzej_p has quit IRC (Quit: Ping timeout (120 seconds)) [17:40] *** andrzej_p has joined #linux-media [19:40] *** svarbanov has joined #linux-media [20:40] *** ao2 has joined #linux-media [20:48] *** kitzman has quit IRC () [20:50] *** dekenevs has joined #linux-media [21:12] *** BrianG61UK has joined #linux-media [21:18] *** BrianG61UK_ has joined #linux-media [21:49] *** ao2 has quit IRC (Quit: Leaving) [22:29] *** gouchi has quit IRC (Remote host closed the connection) [23:47] *** djrscally has quit IRC (Quit: Konversation terminated!)