Hi Hans, On Thursday 24 December 2015 12:17:48 hverkuil wrote: > On 2015-12-24 11:46, Laurent Pinchart wrote: > > On Wednesday 23 December 2015 10:47:57 Guennadi Liakhovetski wrote: > >> >>> 2. Separate buffer queues. Pros: (a) no need to extend multiplanar > >> >>> buffer implementation. Cons: (a) more difficult synchronisation with > >> >>> image frames, (b) still need to work out a way to specify the > >> >>> metadata version. > >> > > >> > Do you think you have different versions of metadata from a sensor, for > >> > instance? Based on what I've seen these tend to be sensor specific, or > >> > SMIA which defines a metadata type for each bit depth for compliant > >> > sensors. > >> > > >> > Each metadata format should have a 4cc code, SMIA bit depth specific or > >> > sensor specific where metadata is sensor specific. > >> > > >> > Other kind of metadata than what you get from sensors is not covered by > >> > the thoughts above. > >> > > >> > <URL:http://www.retiisi.org.uk/v4l2/foil/v4l2-multi-format.pdf> > >> > > >> > I think I'd still favour separate buffer queues. > >> > >> And separate video nodes then. > >> > >> >>> Any further options? Of the above my choice would go with (1) but > >> >>> with a dedicated metadata plane in struct vb2_buffer. > >> >> > >> >> 3. Use the request framework and return the metadata as control(s). > >> >> Since controls can be associated with events when they change you can > >> >> subscribe to such events. Note: currently I haven't implemented such > >> >> events for request controls since I am not certainly how it would be > >> >> used, but this would be a good test case. > >> >> > >> >> Pros: (a) no need to extend multiplanar buffer implementation, (b) > >> >> syncing up with the image frames should be easy (both use the same > >> >> request ID), (c) a lot of freedom on how to export the metadata. Cons: > >> >> (a) request framework is still work in progress (currently worked on > >> >> by Laurent), (b) probably too slow for really large amounts of > >> >> metadata, you'll need proper DMA handling for that in which case I > >> >> would go for 2. > >> > > >> > Agreed. You could consider it as a drawback that the number of new > >> > controls required for this could be large as well, but then already for > >> > other reasons the best implementation would rather be the second option > >> > mentioned. > >> > >> But wouldn't a single extended control with all metadata-transferred > >> controls solve the performance issue? > > > > You would still have to update the value of the controls in the kernel > > based on meta-data parsing, we've never measured the cost of such an > > operation when the number of controls is large. > > Before discounting this option we need to measure the cost first. I suspect > that if there are just a handful (let's say <= 10) of controls, then the > cost is limited. I certainly don't want to disregard the option before having tried it, my only point is that we need to measure performances before making a decision. I plan to do so but without an ETA at this point. > > An interesting side issue is that the control framework currently doesn't > > support updating controls from interrupt context as all controls are > > protected by a mutex. The cost of locking, once it will be reworked, also > > needs to be taken into account. > > I think the cost of locking can be limited (and I am not even sure if > locking is needed if we restrict the type of control where this can be > done). I have some ideas about that. Well, we'll need to do something, and a lockless algorithm would certainly be good :-) > >>>>> In either of the above options we also need a way to tell the user > >>>>> what is in the metadata buffer, its format. We could create new > >>>>> FOURCC codes for them, perhaps as V4L2_META_FMT_... or the user space > >>>>> could identify the metadata format based on the camera model and an > >>>>> opaque type (metadata version code) value. Since metadata formats > >>>>> seem to be extremely camera-specific, I'd go with the latter option. > >>> > >>> I think I'd use separate 4cc codes for the metadata formats when they > >>> really are different. There are plenty of possible 4cc codes we can > >>> use. > >>> > >>> :-) > >>> > >>> Documenting the formats might be painful though. > >> > >> The advantage of this approach together with a separate video node / > >> buffer queue is, that no changes to the core would be required. > >> > >> At the moment I think, that using (extended) controls would be the > >> most "correct" way to implement that metadata, but you can associate such > >> control values with frames only when the request API is there. Yet > >> another caveat is, that we define V4L2_CTRL_ID2CLASS() as ((id) & > >> 0x0fff0000UL) and V4L2_CID_PRIVATE_BASE as 0x08000000, so that drivers > >> cannot define private controls to belong to existing classes. Was this > >> intensional? > > PRIVATE_BASE is deprecated and cannot be used with the control framework > (it was a very bad design). The control framework provides backwards compat > code to handle old apps that still use PRIVATE_BASE. > > Control classes are not deprecated, only the use of the control_class > field in struct v4l2_ext_controls to limit the controls in the list to a > single control class is deprecated. That old limitation was from pre- > control-framework times to simplify driver design. With the creation of the > control framework that limitation is no longer needed. Doesn't that effectively deprecated control classes ? We can certainly group controls in categories for documentation purposes, but the control class as an API concept is quite useless nowadays. > Per-control class private controls are perfectly possible: such controls > start at control class base ID + 0x1000. > > > Control classes are a deprecated concept, so I don't think this matters > > much. > > As described above, this statement is not correct. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
