<!-- 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>
sailus: <u>hverkuil</u>: Hello!
hverkuil: <u>sailus</u>: I have a short (I hope) meeting, will ping you when done.
sailus: Ack.
hverkuil: <u>sailus</u>: pong
sailus: <u>hverkuil</u>: Pung.
<br> How do you do?
hverkuil: Reading your email reply...
sailus: It's pretty short. :-)
hverkuil: I'm fine, thanks.
sailus: What I thought was that it'd be good to address other known problems or at least consider them at the same time.
<br> Rather than to solve this precise with a solution which is specific to just this problem.
hverkuil: Erm, what other problems?
sailus: A lot of sensor drivers rely on register lists that assume certain sensor configuration, including cropping, scaling and a lot of other stuff.
<br> Cropping (most sensors have many points where you can crop) and scaling are typically used to achieve the desired output size.
<br> The register lists are as such quite annoying to work with, and they also have an effect of binding together these seemingly unrelated parameters.
<br> I've thought about this --- and presenting such configurations to the user space, but it'd require the request API as well which we do not have yet (and perhaps in that extent, probably not for some time).
<br> So I was wondering whether it'd be possible to get around this issue until we have something which is a lot better.
<br> Ideally we should also define the way in which the operations take place in hardware, or enumerate the order, for right now this varies too much from driver to driver --- often for reasons related to the hardware.
<br> Choosing the sensor output size also ignores the field of view.
<br> I wonder what pinchartl thinks.
hverkuil: Before you can even start on solving this we need to solve the specific issue I raised. If the sensor has multiple framesizes, then against which frame size does the crop rectangle apply?
<br> It's totally undefined. Either you need to be able to select a size from the list (as per my RFC), or present a fixed native size against which the crop is applied.
sailus: The enum_framesizes IOCTL isn't really meaningful for sensors anyway.
hverkuil: I always wondered about that.
sailus: Suppose a driver is happy with enum_framesizes, then I guess you could add an API to choose which one to use.
<br> But this is not what I'd hope sensors would generally do --- perhaps drivers that rely on register lists, yes.
hverkuil: I see quite a few sensors that implement enum_framesizes.
<br> AND crop
sailus: The fewer we have register lists, the better. :-)
hverkuil: But I do not trust that code at all.
sailus: There's a high likelihood there's sometihng wrong about it. :-I
hverkuil: It seems to me that a sensor either exposes a single NATIVE_SIZE against which the crop rectangle is applied, or it has a list of framesizes and you use the NATIVE_SIZE selection to select one.
<br> In both cases crop is against the native size rectangle. Which makes sense and is consistent.
sailus: When we do have a solution that can expose all the details to the user space (scaling, cropping etc.) related to a given register list configuration used by a driver, there are going to be a lot of drivers that need to be converted from whatever short term solution is used.
<br> I think that would be a somewhat unsatisfactory short term solution albeit a number of people would probably be ok with it. But as an API, it's not what sensors should be ideally using.
<br> What prevents using the format size for the purpose?
pinchartl: <u>hverkuil</u>: sensors should not have frame sizes to start with
sailus: I'm not saying it'd be a good solution but I don't think it's more wrong than a selection rectangle.
pinchartl: a sensor had a physical pixel array size, and on top of that can apply various combinations of cropping, binning, skipping and scaling to achieve a desired output size
sailus: ^ The smiapp driver exposes that through the V4L2 subdev API.
pinchartl: the reason why some sensor drivers implement a list of frame sizes is because the developer took the few sizes for which the vendor provided register values, and didn't bother to do anything else on top of that
hverkuil: You either have only one frame size (aka one native size) and a way to tell userspace what that is, or you have multiple frame sizes, but then you need a way to select which one you want if you want to crop/scale/compose.
sailus: <u>pinchartl</u>: Often the datasheets do not contain enough information to do better than that.
pinchartl: <u>sailus</u>: often there's even no datasheet available at all, it's not an excuse :-)
sailus: <u>pinchartl</u>: In practice there tend to be a lot of drivers relying on register lists.
pinchartl: <u>hverkuil</u>: if a sensor driver supports cropping and/or scaling it should not have multiple frame sizes
<br> the two should be mutually exclusive
<br> <u>sailus</u>: because we've never pushed back
sailus: I'm not saying we should endorse that, quite the contrary, but that's just a fact.
hverkuil: You can enumerate frame sizes AND expose the native size. As long as all selection rectangles are against that native size (if crop is supported), then all is fine as well.
<br> The frame sizes are then basically a hint of which scalings are supported.
pinchartl: <u>hverkuil</u>: it's a bad hint :-(
sailus: <u>hverkuil</u>: What I've lost is why NATIVE_SIZE?
<br> It already has an established meaning, and it is not the one proposed.
hverkuil: "The native size of the device, e.g. a sensor’s pixel array."
<br> Isn't that what we need?
sailus: <u>pinchartl</u>: Right now, pushing back on drivers with register lists would mean we'd have very few sensor drivers.
<br> <u>hverkuil</u>: I understood your proposal included you could set this.
pinchartl: <u>sailus</u>: I disagree
sailus: <u>pinchartl</u>: I'd be happy to discuss that, but I think that's a somewhat separate discussion.
<br> Could we discuss it separately?
pinchartl: sure
<br> <u>hverkuil</u>: back to your proposal, I don't think S_SELECTION(V4L2_SEL_TGT_NATIVE_SIZE) is a good idea
hverkuil: <u>sailus</u>: two options: my proposal was that you use it to pick one of the enumerated frame sizes and crop against that size. But I just realized that it works just as well if the sensor exposes the native size and the crop is always against that.
pinchartl: there's a single pixel array size, not multiple
<br> in the general case you need cropping before scaling and cropping after scaling
<br> in that case we currently need two subdevs to implement that
<br> but if you consider cropping before scaling only
hverkuil: No need to set, the crop coordinates can now be interpreted cleanly (against the native size).
<br> So:
sailus: <u>hverkuil</u>: In that case, the intermediate size is likely achieved using scaling and possibly also cropping.
pinchartl: then cropping should always be performed on the full pixel array, and scaling should come after that
<br> and this is what several of the aptina sensor drivers implement
hverkuil: So:
pinchartl: mt9p031 for instance
<br> .enum_frame_size() reports the scaling (through skipping and binning) capabilities
hverkuil: the pixel array is exposed by V4L2_SEL_TGT_NATIVE_SIZE (not settable, you can only get it)
<br> the crop is against that native size.
pinchartl: base on the pixel array size
<br> s/base/based/
hverkuil: enum_frame_size does as Laurent just said (scaling caps)
pinchartl: note that it's a hack, we need a better way to report scaling capabilities
hverkuil: set_fmt selects the output size after scaling.
<br> what we don't have is cropping after scaling.
pinchartl: we have cropping after scaling if we use two subdevs
<br> <u>sailus</u>: that's what smiapp does, right ?
sailus: smiapp exposesthree.
hverkuil: Yeah, that works.
sailus: There's the pixel array (crop), binner (binning) and scaler (cropping, scaling and cropping).
<br> I.e. you can crop in three locations, each with a little bit special set of features.
<br> The first crop is analogue, i.e. the cropped part of the pixel array isn't digitised at all.
<br> The next crop is digital, and you can crop more or less anything as long a all the dimensions are divisible by two.
hverkuil: So my proposal:
sailus: The last crop is for clipping pixels at the end of the line / rows at the end of the frame for making the size suitable for the receiver after scaling, for the scaling granularity is somewhat coarse.
hverkuil: if a subdev has a NATIVE_SIZE selection target, then you crop against that size. If there isn't one, AND there are multiple framesizes enumerated, AND the subdev support crop, then it is undefined what happens (i.e. driver specific).
pinchartl: why should it be driver-specific ? what could drivers do other than cropping on the pixel array ?
sailus: Cropping after binning or scaling?
pinchartl: is there any reason to allow that ?
sailus: What i have wanted to do actually is allow more freedoms in using selection rectangles, in order to tell user space in which order things happen in hardware.
hverkuil: Right. I actually suspect that they crop against the last set_fmt. The code all looks very weird. Also remember: userspace needs to know the size of the pixel array. Without the NATIVE_SIZE selection target it has no way of knowing that.
sailus: I'm not sure if that'd fully solve this problem but it could play part in solving it in a generic way.
pinchartl: <u>hverkuil</u>: set the cropping rectangle to 1000000x10000000 and you'll get the size of the pixel array
<br> the native size target is better, but there's a way even without it
hverkuil: I looked in a few sensor drivers that have this and I saw weird things.
sailus: <u>hverkuil</u>: The NATIVE_SIZE tells the size of the pixel array, I believe everyone agrees with that.
pinchartl: do you want to allow new sensor drivers to implement a driver-specific behaviour, or is your proposal for existing drivers only ?
sailus: <u>hverkuil</u>: What did you find?
<br> I'm sure there's all kinds of wacky stuff, considering the history of various drivers.
hverkuil: <u>pinchartl</u>: the starting point for this RFC was that Hugues wanted to add crop support to his stm32-dcmi driver.
<br> mt9p031_enum_frame_size is very strange: seems to create an artificial list of framesizes.
pinchartl: <u>hverkuil</u>: the list of frame sizes exposes scaling capabilities
sailus: <u>hverkuil</u>: Crop as in S_CROP?
pinchartl: the mt9p031 can from 1/1 to 1/8
hverkuil: S_SELECTION(TGT_CROP) to be precise.
<br> Hmm, the crop is against the native size, but that doesn't appear in the frame_size enumeration.
sailus: <u>hverkuil</u>: We have another problem here: the V4L2 API exposes scaling and cropping as single parameters, but the pipeline is more complex than that.
<br> And there's no single driver with knowledge of the entire pipeline.
pinchartl: crop should certainly not appear in frame size enumeration
sailus: I'd say the only way you could do this in a generic way would be in user space.
pinchartl: frame size enumeration is the only way we currently have to report discrete scaling options. it's not good, but if we use it, it should be used for that purpose only
sailus: <u>pinchartl</u>: Why should scaling then?
pinchartl: <u>sailus</u>: I've just said that we should not use frame size enumeration at all
sailus: Is there a need to enumerate scaling capabilities?
<br> The scaled sizes likely depend on cropping performed before the scaling as well.
pinchartl: maybe, maybe not
sailus: The smiapp driver does not and no-one has ever complained. :-)
pinchartl: I don't know whether userspace neeeds that information in practice
hverkuil: Hmm. Looking at the selection targets I think what would even be better is if the driver would just expose V4L2_SEL_TGT_CROP_BOUNDS.
sailus: You can still easily find the options if you know what'd you'd prefer: try and use the GT and LT flags.
pinchartl: no one complains because no one uses the upstream kernel in products...
sailus: <u>pinchartl</u>: How do you know? :-)
hverkuil: "Bounds of the crop rectangle. All valid crop rectangles fit inside the crop bounds rectangle." Supported by both v4l2 and v4l2 subdev.
pinchartl: why have we introduced the native size target then ?
hverkuil: To expose the native size. That may be different from the crop bounds.
sailus: I guess the crop bounds aren't necessarily the same as the native_size.
<br> Or a device has crop_bounds but not native_size.
hverkuil: Why didn't I think of this? All you need is to tell userspace what the crop bounds are. Problem solve.
<br> solved.
pinchartl: how does it solve the problem ?
hverkuil: Because now userspace knows what the crop coordinates are.
<br> are against.
pinchartl: no it doesn't
<br> it still doesn't tell where crop is applied (before or after scaling)
<br> does Hughes want to crop before or after scaling ?
hverkuil: Before.
pinchartl: good
<br> then let's specify that cropping is always before scaling
<br> sensor drivers can implement the crop bounds and/or native size targets
<br> (and should actually implement both)
<br> cropping is performed on that
hverkuil: crop after can only be supported with two subdevs. And yes, currently crop comes before scaling.
pinchartl: and s_fmt then applies scaling on top of the crop rectangle
<br> the only thing we need to device is what to do with enum frame size
hverkuil: Yes. All consistent with current usage.
pinchartl: s/device/decide/
sailus: https://hverkuil.home.xs4all.nl/spec/uapi/v4l/dev-subdev.html#selections-cropping-scaling-and-composition
<br> Please read "4.15.3.4. Order of configuration and format propagation".
<br> A lot of drivers don't implement it as documented though.
<br> There are also entirely reasonable (from HW point of view) cases from deviating that, and for that reason I think we should be able to tell the user space what does the hardware really do.
pinchartl: <u>sailus</u>: that section describes a subdev with sink and source pads. we haven't defined what happens for subdevs with source pads only
sailus: Yes, it is defined.
hverkuil: Right. In that case the 'sink media bus format' is the native size?
sailus: You simply don't have the rectangles on the sink pad if you have no sink pads.
pinchartl: the sink format is the native size in that case
<br> but then, if you don't have cropping on the sink pad, you can only crop after scaling
<br> while we have defined that sensor drivers should crop before scalling
<br> so it's inconsistent
<br> (not that it would be the first inconsistency we have...)
hverkuil: Is it inconsistent? Cropping on a source pad means cropping in the 'sink' format which is the native size.
sailus: s/We have defined that/A number of/; s/should //;
hverkuil: Then it is scaled to the source pad format.
<br> I.e. cropping before scaling.
<br> That said, I think it would be useful to have a sensor-specific example in that chapter.
sailus: Strictly speaking the compose (i.e. scaling) rectangle is only defined for sink pads.
<br> So the order of the operations, as documented is crop (sink), scaling (sink) and crop (source).
<br> If you don't have either of the pads, then you don't have the rectangles on those pads either.
<br> Simple but cruel.
pinchartl: <u>hverkuil</u>: section 4.15.3.4 defines how to scale using the sink compose selection
<br> and cropping on the source side crops after scaling
<br> while sensors behave completely differently
sailus: <u>pinchartl</u>: That's not quite true.
<br> Sensors typically have multiple scaling and cropping points but not all of them are always expoesd by the drivers.
<br> I don't think we should extend the spec by making special cases for individual pieces or hardware.
pinchartl: <u>sailus</u>: what we've just discussed, cropping on the pixel array and scaling through set format, which is what most sensor drivers implement, is not consistent with 4.15.3.4. that's my only point
sailus: <u>pinchartl</u>: Cropping is fine, but scaling using the format isn't in sync with the spec.
pinchartl: if you only crop it's fine yes
sailus: The API was added some six years ago and this was thought to be a reasonable compromise.
<br> I still think it is, but it is a tad too restrictive IMO.
hverkuil: Hmm, I expected to see a source compose rectangle, but apparently it "refers to the sink compose bounds rectangle".
sailus: Multiple sub-devices are often needed for just that.
<br> There are other reasons, but that's one of them.
<br> <u>hverkuil</u>: I suppose the driver Hugues was working on doesn't cope with sub-device drivers that expose more than one sub-device anyway?
pinchartl: (on a side note, this is another example - if one was needed - that shows we'll need to redesign large parts of the API in the near future. every discussion turns into issues nowadays)
hverkuil: <u>sailus</u>: probably not
sailus: <u>pinchartl</u>: I'd like to add that there are no solutions which are easy to implement, generic and complete and compatible with the history of APIs we already have.
pinchartl: <u>sailus</u>: it's the last point that we'll need to stop caring about in the redesign. it won't be compatible
<br> I'm fine continuing with the sensor-specific behaviour for crop and scaling for now, as we're stuck with it anyway
<br> when a sensor is exposed through a single subdev, crop is on the pixel array, and scaling is configured through s_fmt
sailus: In long term, we need to get rid of it.
<br> <u>pinchartl</u>: Works for me, but we'll need something better going forward pretty soon.
<br> The next related problem that needs to be solved will not be compatible with what you just said.
<br> Unless, that is, we add another special case.
pinchartl: <u>sailus</u>: my target is early 2018 for the large redesign. I don't expect to get buy-in from everybody though
sailus: Do you expect someone to disagree then? :-)
<br> Me?
<br> X-)
pinchartl: :-)
sailus: What do you want to redesign?
<br> Just how selections are handled, or a larger set of APIs?
pinchartl: everything. I want to go through the whole API and change what needs to be changed
sailus: <u>pinchartl</u>: We must have a list of things that aren't working right now.
<br> Could we maintain such in e.g. wiki or Etherpad?
hverkuil: (phone call)
pinchartl: <u>sailus</u>: yes we should
bparrot: pinchartl, V4L3 :)
sailus: Who will write the meeting notes? :-)
hverkuil: We do agree that if the sensor exports CROP_BOUNDS, then at least that clarifies against which coordinates the TGT_CROP applies?
<br> And that sensor drivers that support crop should also support CROP_BOUNDS?
sailus: Yes, absolutely!
hverkuil: (and probably CROP_DEFAULT as well for that matter)
sailus: And S_FMT configures scaling.
<br> I don't think we quite got to how would enum_framesizes interact with these.
pinchartl: <u>bparrot</u>: maybe. I won't insist on a name change :-)
hverkuil: I interpret enum_framesizes as a limited attempt at reporting scaler limitations.
sailus: https://hverkuil.home.xs4all.nl/spec/uapi/v4l/vidioc-subdev-enum-frame-size.html
<br> The documentation makes no mention of that.
<br> Originally this has come from the video ops I believe.
<br> What size should be assumed before scaling? Is cropping included, for example?
hverkuil: it comes from webcams that just list a bunch of frame sizes. No cropping, no scaling otherwise.
<br> since the sensor is completely hidden you have no idea of the native size. And the frame sizes may have different aspect ratios.
<br> <u>sailus</u>: pinchartl: just curious: why is it required that the source media bus format size is equal to the source crop selection size? Why can't it be larger/smaller if there is a scaler?
<br> theoretically you can also have a source compose rectangle, although such hardware is probably very rare.
<br> I also don't understand this line: "The exception to this rule is the source compose rectangle, which refers to the sink compose bounds rectangle — if it is supported by the hardware."
<br> Looking at the 5 steps you have: 1: sink pad format, 2: sink pad crop; 3: sink pad compose; 4: source pad crop; 5: source pad format. But I'm missing 4.5: source pad compose.
<br> <u>Hmm</u>: "The coordinates to a step always refer to the actual size of the previous step."
<br> But that wouldn't prevent a source pad compose. That would refer to the crop pad compose and depend entirely on the scaler (if any) capabilities and the crop size.
sailus: <u>hverkuil</u>: The scaler is expected to be configured against the sink compose selection target.
<br> s/against/using/
<br> The documentation does not recognise a compose rectangle on the source pads.
<br> That sentence is actually wrong: it should say sink instead of source.
<br> Then it'd make sense.
<br> The diagrams have sink, too.
<br> The reasoning behind that exception is related to the compose rectangle on V4L2 devices --- there can be a buffer on which the composition is performed on and its dimensions may differe from the previous step (i.e. the compose_bounds rectangle).
<br> In practice for smiapp the compose_bounds rectangle is the same as the crop rectangle.
hverkuil: So it should be: "The exception to this rule is the sink compose rectangle, which refers to the sink compose bounds rectangle — if it is supported by the hardware."
sailus: Yes. I wonder what's the history of the text. I remember I wrote it but I don't remember whether this was included in the original.
hverkuil: can you make a patch fixing that? It was extremely confusing when I read it.
sailus: Certainly.
hverkuil: Anyway, I still don't see what it wrong with a source compose rectangle. This is what a sensor with a scaler does: it has an internal image which you crop with a source crop rectangle and scale to a source compose rectangle. Non-sensor subdevs may not have it, but that's OK.
<br> It feels very unbalanced right now.
***: ribalda_ has quit IRC (Remote host closed the connection)
sailus: <u>hverkuil</u>: Done.
<br> <u>hverkuil</u>: The discussion back in ~ 2011 resulted in that: the major factor in that was the use case Samsung devices had at the time, the same reason why it's called compose (vs. scale).
<br> I have no objections to making this more flexible, but we need a way to enumerate the targets.
<br> And a reasonable API to access them.
hverkuil: But don't we have that already? An API I mean?
sailus: Somewhat, yes.
<br> But to make this more flexible will probably require amendments to that API.
<br> <u>hverkuil</u>: I'll pick the patch to my tree unless there are objections.
<br> It's in my for-4.14 branch.
***: benjiG has left
<br> bit_lySLH2uSZHed has left
CodeMaster_: Hello
<br> Good evening !
<br> So, I'm trying to create my own driver for a customized CSI parallel Camera. I'm looking for documentation regarding driver development.
<br> I've looked here: https://linuxtv.org/downloads/v4l-dvb-apis/uapi/v4l/driver.html#
<br> But it is under construction
<br> any help?
<br> lol anyone alive? I'm starting to worry about you guys
sailus: <u>CodeMaster_</u>: It's night. Most people are asleep.
<br> CSI or CSI-2?
<br> There are quite a few CSI-2 receiver drivers in the mainline kernel. E.g. omap3isp is a relatively good example.
-: headless wonders why sailus isn't asleep :-)
CodeMaster_: Oh hi sailus, so where are you based?
<br> I am not sure, CSI with the IMX6 IPU
<br> So im guessing its CSI
<br> Its for embedded linux, I'm looking for guidance on where to look, more than an answer
sailus: Finland.
<br> CSI-1 is very old.
<br> It's most probably CSI-2.
<br> omap3isp is a good example. For a sensor driver, either smiapp or one of the drivers merged recently --- they're more simple; see e.g. what git log tells of drivers/media/i2c.
<br> I think Steve had also a sensor driver for an Omnivision sensor. The i.MX 6 had some limitation if I understand correctly related to CSI-2 channels that usually are just... channel identifiers and not more than that.
CodeMaster_: Thanks, i'm going to look for the Omap3 one. THe omnivision sensor has 3 basic things: 1.- i2c for configuration 2.- internal sequence initialization and 3.- v4l interfaces
<br> what I can't find is like a minimum necesary v2l interfaces list and explanation
<br> I need something like this: https://www.linuxtv.org/downloads/legacy/video4linux/v4l2dwgNew.html
<br> but updated lol this is from 2k5
sailus: The omap3isp driver doesn't have a lot of extra in terms of interfaces if you leave out the private IOCTLs. It does support memory to memory operation and there's an image signal processor, too. These are still pretty well separated from the rest.
CodeMaster_: Back when linux 2.6 was THE thing
<br> great
sailus: Check this one too:
<br> https://hverkuil.home.xs4all.nl/spec/kapi/csi2.html
<br> 2k5 => 2500?
CodeMaster_: 2005, 12 years ago
sailus: Ah. Yes.
<br> That was a long time ago.
CodeMaster_: Thanks sailus