| Message ID | 20210906225636.14683-12-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
On 06/09/2021 23:56, Laurent Pinchart wrote: > When creating FrameBuffer instances, the V4L2VideoDevice computes plane > offsets using minimal stride for the format. This doesn't always produce > a valid result when the device requires padding at the end of lines. Fix > it by computing offsets using the stride reported by V4L2. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > Reviewed-by: Hirokazu Honda <hiroh@chromium.org> > --- > src/libcamera/v4l2_videodevice.cpp | 16 +++++++++++++--- > 1 file changed, 13 insertions(+), 3 deletions(-) > > diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp > index 88535f5a07c7..c6c9263c49e9 100644 > --- a/src/libcamera/v4l2_videodevice.cpp > +++ b/src/libcamera/v4l2_videodevice.cpp > @@ -1354,11 +1354,21 @@ std::unique_ptr<FrameBuffer> V4L2VideoDevice::createBuffer(unsigned int index) > size_t offset = 0; > > for (size_t i = 0; i < planes.size(); ++i) { > + /* > + * The stride is reported by V4L2 for the first plane > + * only. Compute the stride of the other planes by > + * taking the horizontal subsampling factor into > + * account, which is equal to the bytesPerGroup ratio of > + * the planes. > + */ Yikes, this feels like it might be fragile, but I can't see where it would break yet so ... it sounds ok I think. Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > + unsigned int stride = format_.planes[0].bpl > + * formatInfo_->planes[i].bytesPerGroup > + / formatInfo_->planes[0].bytesPerGroup; > + > planes[i].fd = fd; > planes[i].offset = offset; > - > - /* \todo Take the V4L2 stride into account */ > - planes[i].length = formatInfo_->planeSize(format_.size, i); > + planes[i].length = formatInfo_->planeSize(format_.size.height, > + i, stride); > offset += planes[i].length; > } > } >
Hi Kieran, On Tue, Sep 07, 2021 at 12:37:49AM +0100, Kieran Bingham wrote: > On 06/09/2021 23:56, Laurent Pinchart wrote: > > When creating FrameBuffer instances, the V4L2VideoDevice computes plane > > offsets using minimal stride for the format. This doesn't always produce > > a valid result when the device requires padding at the end of lines. Fix > > it by computing offsets using the stride reported by V4L2. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > > Reviewed-by: Hirokazu Honda <hiroh@chromium.org> > > --- > > src/libcamera/v4l2_videodevice.cpp | 16 +++++++++++++--- > > 1 file changed, 13 insertions(+), 3 deletions(-) > > > > diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp > > index 88535f5a07c7..c6c9263c49e9 100644 > > --- a/src/libcamera/v4l2_videodevice.cpp > > +++ b/src/libcamera/v4l2_videodevice.cpp > > @@ -1354,11 +1354,21 @@ std::unique_ptr<FrameBuffer> V4L2VideoDevice::createBuffer(unsigned int index) > > size_t offset = 0; > > > > for (size_t i = 0; i < planes.size(); ++i) { > > + /* > > + * The stride is reported by V4L2 for the first plane > > + * only. Compute the stride of the other planes by > > + * taking the horizontal subsampling factor into > > + * account, which is equal to the bytesPerGroup ratio of > > + * the planes. > > + */ > > Yikes, this feels like it might be fragile, but I can't see where it > would break yet so ... it sounds ok I think. That's the rule that V4L2 mandates, so I think it should be fine. > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > + unsigned int stride = format_.planes[0].bpl > > + * formatInfo_->planes[i].bytesPerGroup > > + / formatInfo_->planes[0].bytesPerGroup; > > + > > planes[i].fd = fd; > > planes[i].offset = offset; > > - > > - /* \todo Take the V4L2 stride into account */ > > - planes[i].length = formatInfo_->planeSize(format_.size, i); > > + planes[i].length = formatInfo_->planeSize(format_.size.height, > > + i, stride); > > offset += planes[i].length; > > } > > }
diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index 88535f5a07c7..c6c9263c49e9 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -1354,11 +1354,21 @@ std::unique_ptr<FrameBuffer> V4L2VideoDevice::createBuffer(unsigned int index) size_t offset = 0; for (size_t i = 0; i < planes.size(); ++i) { + /* + * The stride is reported by V4L2 for the first plane + * only. Compute the stride of the other planes by + * taking the horizontal subsampling factor into + * account, which is equal to the bytesPerGroup ratio of + * the planes. + */ + unsigned int stride = format_.planes[0].bpl + * formatInfo_->planes[i].bytesPerGroup + / formatInfo_->planes[0].bytesPerGroup; + planes[i].fd = fd; planes[i].offset = offset; - - /* \todo Take the V4L2 stride into account */ - planes[i].length = formatInfo_->planeSize(format_.size, i); + planes[i].length = formatInfo_->planeSize(format_.size.height, + i, stride); offset += planes[i].length; } }