Hi Laurent and Hans, On Wed, Feb 21, 2018 at 10:16:25PM +0200, Laurent Pinchart wrote: > No, I'm sorry, for MC-based drivers this isn't correct. The media entity that > symbolizes the DMA engine indeed has a sink pad, but it's a video node, not a > subdev. It thus has no media bus format configured for its sink pad. The > closest pad in the pipeline that has a media bus format is the source pad of > the subdev connected to the video node. > > There's no communication within the kernel at G/S_FMT time between the video > node and its connected subdev. The only time we look at the pipeline as a > whole is when starting the stream to validate that the pipeline is correctly > configured. We thus have to implement G/S_FMT on the video node without any > knowledge about the connected subdev, and thus accept any colorspace. A few more notes related to this --- there's no propagation of sub-device format across the entities; there were several reasons for the design choice. The V4L2 pixel format in the video node must be compatible with the sub-device format when streaming is started. And... the streaming will start in the pipeline defined by the enabled links to/from the video node. In principle nothign prevents having multiple links there, and at the time S_FMT IOCTL is called on the video node, none of those links could be enabled. And that's perfectly valid use of the API. A lot of the DMA engine drivers are simply devices that receive data and write that to system memory, they really don't necessarily know anything else. For the hardware this data is just pixels (or even bits, especially in the case of CSI-2!). So I agree with Laurent here that requiring correct colour space for [GS]_FMT IOCTLs on video nodes in the general case is not feasible (especially on MC-centric devices), due to the way the Media controller pipeline and formats along that pipeline are configured and validated (i.e. at streamon time). But what to do here then? The colourspace field is not verified even in link validation so there's no guarantee it's correctly set more or less anywhere else than in the source of the stream. And if the stream has multiple sources with different colour spaces, then what do you do? That's perhaps a theoretical case but the current frameworks and APIs do in principle support that. Perhaps we should specify that the user should always set the same colourspace on the sink end of a link that was there in the source? The same should likely apply to the rest of the fields apart from width, height and code, too. Before the user configures formats this doesn't work though, and this does not address the matter with v4l2-compliance. Granted that the drivers will themselves handle the colour space information correctly, it'd still provide a way for the user to gain the knowledge of the colour space which I believe is what matters. -- Kind regards, Sakari Ailus e-mail: sakari.ailus@xxxxxx
