| Message ID | 20200421203954.15585-2-andrey.konovalov@linaro.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Andrey, Thank you for the patch. On Tue, Apr 21, 2020 at 11:39:53PM +0300, Andrey Konovalov wrote: > Change SimpleCameraData::setupFormats() to return -EINVAL if the sink > pad of the link doesn't support the format set on the source pad of this > link. Note that there's at least one driver (omap3isp) that supports different formats on the two ends of a link (it's used at the input of the CCDC block to model bits being dropped on the bus, for instance when connecting D[9:2] of a 10-bit sensor to D[7:0] of the CCDC input). This is a bit of a hack and shouldn't be encouraged. The OMAP3 ISP would also need its own pipeline handler anyway. I think your patch is thus fine. > Signed-off-by: Andrey Konovalov <andrey.konovalov@linaro.org> > --- > src/libcamera/pipeline/simple/simple.cpp | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp > index b5f9177..8212bd9 100644 > --- a/src/libcamera/pipeline/simple/simple.cpp > +++ b/src/libcamera/pipeline/simple/simple.cpp > @@ -372,6 +372,7 @@ int SimpleCameraData::setupFormats(V4L2SubdeviceFormat *format, > MediaLink *link = e.link; > MediaPad *source = link->source(); > MediaPad *sink = link->sink(); > + V4L2SubdeviceFormat source_format; s/source_format/sourceFormat/ > > if (source->entity() != sensor_->entity()) { > V4L2Subdevice *subdev = pipe->subdev(source->entity()); > @@ -380,11 +381,22 @@ int SimpleCameraData::setupFormats(V4L2SubdeviceFormat *format, > return ret; > } > > + source_format = *format; You can move this and the variable declaration within the if () below. > if (sink->entity()->function() != MEDIA_ENT_F_IO_V4L) { > V4L2Subdevice *subdev = pipe->subdev(sink->entity()); > ret = subdev->setFormat(sink->index(), format, whence); > if (ret < 0) > return ret; > + > + if (format->mbus_code != source_format.mbus_code > + || format->size != source_format.size) { We usually put the || at the end of the line. > + LOG(SimplePipeline, Debug) > + << "Source pad format isn't supported " > + << "by the sink pad of the link: " > + << "Source: " << source_format.toString() > + << "Sink: " << format->toString(); It could be useful to tell which link this is related to. How about << "Source '" << source->entity()->name() << "':" << source->index() << " produces " << sourceFormat.toString() << ", sink '" << sink->entity()->name() << "':" << sink->index() << " requires " << format->toString(); With those changes, Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> I can fix when applying if you're fine with this. > + return -EINVAL; > + } > } > > LOG(SimplePipeline, Debug)
diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp index b5f9177..8212bd9 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -372,6 +372,7 @@ int SimpleCameraData::setupFormats(V4L2SubdeviceFormat *format, MediaLink *link = e.link; MediaPad *source = link->source(); MediaPad *sink = link->sink(); + V4L2SubdeviceFormat source_format; if (source->entity() != sensor_->entity()) { V4L2Subdevice *subdev = pipe->subdev(source->entity()); @@ -380,11 +381,22 @@ int SimpleCameraData::setupFormats(V4L2SubdeviceFormat *format, return ret; } + source_format = *format; if (sink->entity()->function() != MEDIA_ENT_F_IO_V4L) { V4L2Subdevice *subdev = pipe->subdev(sink->entity()); ret = subdev->setFormat(sink->index(), format, whence); if (ret < 0) return ret; + + if (format->mbus_code != source_format.mbus_code + || format->size != source_format.size) { + LOG(SimplePipeline, Debug) + << "Source pad format isn't supported " + << "by the sink pad of the link: " + << "Source: " << source_format.toString() + << "Sink: " << format->toString(); + return -EINVAL; + } } LOG(SimplePipeline, Debug)
Change SimpleCameraData::setupFormats() to return -EINVAL if the sink pad of the link doesn't support the format set on the source pad of this link. Signed-off-by: Andrey Konovalov <andrey.konovalov@linaro.org> --- src/libcamera/pipeline/simple/simple.cpp | 12 ++++++++++++ 1 file changed, 12 insertions(+)