mchehab: Ping? mchehab: sailus: pinchartl: question about the media controller API and properties: A property object has an owner id, i.e. to what object does it belong to. This can be any graph object. Whether the owner is an entity, pad, other property, etc. is defined by enum media_gobj_type and is encoded in the top 8 bits of the object ID. However, this information is currently in include/media/media-entity.h and not in uapi/linux/media.h Would it make sense to move that to the public API? So userspace can give it an ID and get back the media type? Or alternatively (and I did that in v4 of the properties series) expose this information in the property object as e.g. an owner_type field and its own set of type defines. Applications will typically make maps for entities, pads, links, etc. so knowing which map to use to lookup the property's owner is very useful. I have no clear preference myself. I'm slightly leaning towards adding an owner type since that allows us to change the way object IDs are created (perhaps in a different way then using the top 8 bits). do you foresee usage of properties for links ? Not at the moment (I don't think it's implemented for links in the first version), but properties are designed to be generic, so can be added to any object. In the initial version I only implemented them for entities, pads and properties (for nested properties) I think we need to be careful about what we're designing here nested properties ?? you're worrying me talk to sailus: he wanted that. sailus: ping what's the use case ? I think properties need to be organized in a tree but that doesn't mean having properties of properties unless we want to create a bloated API that nobody will be able to implement properly or use in a decent manner of course :-) Ah, no it is not quite that bad. and properties should not be graph objects there are related to graph objects but they shouldn't be graph objects themselves Why on earth not? Properties can even form a tree, which is a graph. the next thing you will tell me is that you use links to connect properties together... no, I don't do that. don't make them graph objects either and we'll be good :-) I'll try to allocate a day to review the design early next week what is the last version ? s/last/latest/ I'm going to post a v5 today. You can shoot at it, but I'm not going to replace the use of graph objects, since it that made it very easy to implement. If you want somethnig else, you can pick it up. I'll review it before deciding whether I want something different :-) but at a glance making properties graph objects seems a very bad idea to me I'll check the other design decisions will v5 include example use cases ? No 'real' use-cases, just code added to vimc to test the API. it's hard to review an API without use cases... would you have an hour to discuss that this afternoon (with Sakari too) ? I'd like to come up with a base list of use cases to review the API (and implementation) against and I'd say that we need real use cases implemented in drivers and userspace to validate the API not today (meetings this afternoon). Easier to discuss it tomorrow when I've posted v5. tomorrow is fine with me too we've made more than enough mistakes in the past when we designed APIs without a real implemented use case, to only realize later that it didn't match the needs let's not make the same mistake again :-) The only use-case that I *know* we need is to use properties to assign multiple functions to entities. And sailus want to use it to provide additional sensor info (backward, forward facing, etc) that's a good start :-) And once we have connector entities, then it can be used for the name/color/symbol on the backplane. But we still don't support connectors, so... do you implement assignment of multiple functions to entities in your patch series ? No, this series is just the core support. I've been trying to get people to review it for ages :-( would it be difficult to do so (in vimc ?) as a test ? I know I wish I could spend more time reviewing V4L2 core changes pinchartl: Pong. Not until January, my vacation starts this weekend. but a large part of that needs to be done in my spare time, and I have little spare time :-S then we can discuss use cases tomorrow, and resume discussions when you come back from holidays I'll review patches on the mailing list in the meantime Just wait for v5. sure the meantime will not be this week anyway :-) Sorry for being testy, but it is really annoying when I get comments about a core design issue that's been there since the first version. I'm only human. hverkuil: it's my fault for not having reviewed the first version (or at least for not having discussed it) I don't think I'd be happy about that either and I certainly don't blame you for having implemented it in a way that doesn't perfectly match the ideas that I have never communicated :-) hverkuil: My apologies for not providing review comments on the recent properties API patchsets. :-I sailus: pong hverkuil: I don't see any implementation issues on organizing properties like graphs I mean, if you associate an object ID with a property ID, and let a property to be associated to another property ID, you'll have a graph you may even have a N:1 association between graph objects and a property (that's from the API standpoint... I dunno if this is a good idea or not - need to see usecases) mchehab: I just sent a new pull request with changes according to the comments to the v8 set (I posted v9 set as well). There are more entries in the TODO file left to be addressed. Btw. checkpatch.pl warnings remain in the code. I can fix those soon. mchehab: I resent the pull request. The v2 had an old tag, missing some Reviewed-by tags. So the right one is v3. mchehab: please don't pull the IPU3 series yet, it has Reviewed-by tags from me which I never gave ok mchehab: Fixed in v4. mchehab: pinchartl: posted v5 of the properties series. hverkuil: thank you paulk-leonov: mripard: If you can take a look at the "[PATCHv5 0/8] vb2/cedrus: use timestamps to identify buffers" patch series and esp. test it, that would be great. It looks good to Mauro, so it seems we have a winner. Also take a look at Jonas Karlman's reply to "[PATCHv5 6/8] vb2: add vb2_find_timestamp()" regarding also testing against the current in-progress buffer for interlaced formats. An interesting corner case that requires an additional cedrus patch. hverkuil, that's replacing the original cookie idea ? ndufresne: I'm searching a way to calculate gow many bytes is athe width of a line *how but I've not found an answer to that question ndufresne: how cpuld I find that information? *could cristian_c, wait what ? "how many bytes is athe width of a line" ? do you mean calculating the strides ? something like https://gitlab.freedesktop.org/gstreamer/gst-plugins-base/blob/master/gst-libs/gst/video/video-info.c#L728 ndufresne: I'm looking at it static gboolean fill_planes (GstVideoInfo * info) v4l device buffer it's widthxheight ndufresne: if I don't calculate the exact bytes for the width of a line, frames cannot be captured from the video source (in my case, CVBS, pal) width of a line = stride GstVideoInfo struct has stride parameter/field well, gstreamer has it's own mechanism, but v4l2 bytesperline represent the stride of the first plane, we have code to extrapolate the strides of the other planes, and then carry that information along with the buffer (GstVideoMeta) and what I pointed you, is the default strides for the case none is specified, or the case we allocate a software buffer well a buffer for software operation ok ndufresne: anywsy, GstVideoMeta is not mentioned in video-info.c (but I suppose in gstreamer documentation) ndufresne: if I've understood properly, I should create a snippet/piece of code, in order to get info about stride/bytes (using gstvideometa) but I'll take a look at the capture device driver too about video format cristian_c, kernel side, ezequielg started to experiment with some shared code for drivers to reuse, as they all see to do the same thing ndufresne: yes. This seems to work for everyone. ah, ok I don't know if he has a related github repository (abput the experiment) I still have to look at ezequielg's code for that. Didn't get around to it. *about (I'm very interested, btw) hverkuil, should work indeed, even for the multi-slices case, since all slices of the same frame shares the same timestamp, and you can always abstract the timestamp with some counter hverkuil, I don't know how far he got, as he was trying to be smart and "generalize" the computation instead of just having a big switch like we do in gst we didn't make the code public in gst either, just to show how confident we are ;-D there is always new formats that get created to break smart code uhm, ok btw, v4l uses UYVY video format in that case (variant of YUYV, in little indian) is anyone aware of a tool like checkpatch.pl that wouldn't be specific to the kernel ? there are plenty of code reformatting tools, but there doesn't seem to be commit/patch style checkers hverkuil: cristian_c ndufresne: well basically you can checkout the rockchip vpu driver. it has helpers, which i plan to generalize. ah, that's were it lives now ;-P ok