| Message ID | 20201001154600.2722718-3-niklas.soderlund@ragnatech.se |
|---|---|
| State | Accepted |
| Commit | 1455bd0dc25d6c5a2cc8ef99dc030b9dca3df77d |
| Headers | show |
| Series |
|
| Related | show |
Hi Niklas, On Thu, Oct 01, 2020 at 05:45:56PM +0200, Niklas Söderlund wrote: > The upstream driver has changed how the link formats are validated when > starting to stream [1]. This revealed that libcamera did not adjust the > media bus format from the link between {main,self} resizer source pad > and the capture video device as expected by the driver. > > The media bus code YUYV8_2X8 was hardcoded to MEDIA_BUS_FMT_YUYV8_2X8 > for all pixel formats while it must be adjusted to YUYV8_1_5X8 for NV12 > and NV21, fix this. 5X8 ? I've seen comments on this but didn't get if that should be fixed upstream ? > > 1. 6803a9e0e1e43e9e ("media: staging: rkisp1: cap: simplify link validation by comparing media bus code") > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> > --- > * Changes since v1 > - Spelling commit message. > --- > src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp > index 410e9f5d94607f44..63c643f22affc74a 100644 > --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp > +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp > @@ -7,6 +7,8 @@ > > #include "rkisp1_path.h" > > +#include <linux/media-bus-format.h> > + > #include <libcamera/formats.h> > #include <libcamera/stream.h> > > @@ -125,6 +127,16 @@ int RkISP1Path::configure(const StreamConfiguration &config, > << "Configuring " << name_ << " resizer output pad with " > << ispFormat.toString(); > > + switch (config.pixelFormat) { > + case formats::NV12: > + case formats::NV21: > + ispFormat.mbus_code = MEDIA_BUS_FMT_YUYV8_1_5X8; > + break; > + default: > + ispFormat.mbus_code = MEDIA_BUS_FMT_YUYV8_2X8; > + break; > + } > + > ret = resizer_->setFormat(1, &ispFormat); > if (ret < 0) > return ret; > -- > 2.28.0 > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Jacopo, On 2020-10-02 16:09:30 +0200, Jacopo Mondi wrote: > Hi Niklas, > > On Thu, Oct 01, 2020 at 05:45:56PM +0200, Niklas Söderlund wrote: > > The upstream driver has changed how the link formats are validated when > > starting to stream [1]. This revealed that libcamera did not adjust the > > media bus format from the link between {main,self} resizer source pad > > and the capture video device as expected by the driver. > > > > The media bus code YUYV8_2X8 was hardcoded to MEDIA_BUS_FMT_YUYV8_2X8 > > for all pixel formats while it must be adjusted to YUYV8_1_5X8 for NV12 > > and NV21, fix this. > > 5X8 ? > > I've seen comments on this but didn't get if that should be fixed > upstream ? It looks like this should be fixed upstream, but what is in this patch matches current upstream and without it NV12 is broken. As it's fixed upstream an accompanying patch should be submitted to libcamera. > > > > > 1. 6803a9e0e1e43e9e ("media: staging: rkisp1: cap: simplify link validation by comparing media bus code") > > > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > > Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> > > --- > > * Changes since v1 > > - Spelling commit message. > > --- > > src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 12 ++++++++++++ > > 1 file changed, 12 insertions(+) > > > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp > > index 410e9f5d94607f44..63c643f22affc74a 100644 > > --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp > > @@ -7,6 +7,8 @@ > > > > #include "rkisp1_path.h" > > > > +#include <linux/media-bus-format.h> > > + > > #include <libcamera/formats.h> > > #include <libcamera/stream.h> > > > > @@ -125,6 +127,16 @@ int RkISP1Path::configure(const StreamConfiguration &config, > > << "Configuring " << name_ << " resizer output pad with " > > << ispFormat.toString(); > > > > + switch (config.pixelFormat) { > > + case formats::NV12: > > + case formats::NV21: > > + ispFormat.mbus_code = MEDIA_BUS_FMT_YUYV8_1_5X8; > > + break; > > + default: > > + ispFormat.mbus_code = MEDIA_BUS_FMT_YUYV8_2X8; > > + break; > > + } > > + > > ret = resizer_->setFormat(1, &ispFormat); > > if (ret < 0) > > return ret; > > -- > > 2.28.0 > > > > _______________________________________________ > > libcamera-devel mailing list > > libcamera-devel@lists.libcamera.org > > https://lists.libcamera.org/listinfo/libcamera-devel
diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp index 410e9f5d94607f44..63c643f22affc74a 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp @@ -7,6 +7,8 @@ #include "rkisp1_path.h" +#include <linux/media-bus-format.h> + #include <libcamera/formats.h> #include <libcamera/stream.h> @@ -125,6 +127,16 @@ int RkISP1Path::configure(const StreamConfiguration &config, << "Configuring " << name_ << " resizer output pad with " << ispFormat.toString(); + switch (config.pixelFormat) { + case formats::NV12: + case formats::NV21: + ispFormat.mbus_code = MEDIA_BUS_FMT_YUYV8_1_5X8; + break; + default: + ispFormat.mbus_code = MEDIA_BUS_FMT_YUYV8_2X8; + break; + } + ret = resizer_->setFormat(1, &ispFormat); if (ret < 0) return ret;
The upstream driver has changed how the link formats are validated when starting to stream [1]. This revealed that libcamera did not adjust the media bus format from the link between {main,self} resizer source pad and the capture video device as expected by the driver. The media bus code YUYV8_2X8 was hardcoded to MEDIA_BUS_FMT_YUYV8_2X8 for all pixel formats while it must be adjusted to YUYV8_1_5X8 for NV12 and NV21, fix this. 1. 6803a9e0e1e43e9e ("media: staging: rkisp1: cap: simplify link validation by comparing media bus code") Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> --- * Changes since v1 - Spelling commit message. --- src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 12 ++++++++++++ 1 file changed, 12 insertions(+)