| Message ID | 20220720130348.1337-3-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Commit | f6d6181d3c91efa526b5027319331db9ac75f65b |
| Headers | show |
| Series |
|
| Related | show |
Hi Laurent On Wed, Jul 20, 2022 at 04:03:48PM +0300, Laurent Pinchart via libcamera-devel wrote: > The SDLTexture::update() function isn't meant to modify the data it > receives. Make the Span type const to ensure this at compile time. While > at it, pass the Span by value instead of reference, as a Span is only a > pointer and size, which will fit in registers and will avoid pointer > dereferences in the callee. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Looks good Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> Thanks j > --- > src/cam/sdl_texture.h | 2 +- > src/cam/sdl_texture_mjpg.cpp | 4 ++-- > src/cam/sdl_texture_mjpg.h | 4 ++-- > src/cam/sdl_texture_yuyv.cpp | 2 +- > src/cam/sdl_texture_yuyv.h | 2 +- > 5 files changed, 7 insertions(+), 7 deletions(-) > > diff --git a/src/cam/sdl_texture.h b/src/cam/sdl_texture.h > index 1d8b09288d50..f523fa5ebf51 100644 > --- a/src/cam/sdl_texture.h > +++ b/src/cam/sdl_texture.h > @@ -17,7 +17,7 @@ public: > SDLTexture(const SDL_Rect &rect, uint32_t pixelFormat, const int pitch); > virtual ~SDLTexture(); > int create(SDL_Renderer *renderer); > - virtual void update(const libcamera::Span<uint8_t> &data) = 0; > + virtual void update(libcamera::Span<const uint8_t> data) = 0; > SDL_Texture *get() const { return ptr_; } > > protected: > diff --git a/src/cam/sdl_texture_mjpg.cpp b/src/cam/sdl_texture_mjpg.cpp > index 7eddc00cd210..7542efd75d96 100644 > --- a/src/cam/sdl_texture_mjpg.cpp > +++ b/src/cam/sdl_texture_mjpg.cpp > @@ -43,7 +43,7 @@ SDLTextureMJPG::SDLTextureMJPG(const SDL_Rect &rect) > { > } > > -int SDLTextureMJPG::decompress(const Span<uint8_t> &data) > +int SDLTextureMJPG::decompress(Span<const uint8_t> data) > { > struct jpeg_decompress_struct cinfo; > > @@ -76,7 +76,7 @@ int SDLTextureMJPG::decompress(const Span<uint8_t> &data) > return 0; > } > > -void SDLTextureMJPG::update(const Span<uint8_t> &data) > +void SDLTextureMJPG::update(Span<const uint8_t> data) > { > decompress(data); > SDL_UpdateTexture(ptr_, nullptr, rgb_.get(), pitch_); > diff --git a/src/cam/sdl_texture_mjpg.h b/src/cam/sdl_texture_mjpg.h > index 328c45a913c5..5141ed73bf70 100644 > --- a/src/cam/sdl_texture_mjpg.h > +++ b/src/cam/sdl_texture_mjpg.h > @@ -14,10 +14,10 @@ class SDLTextureMJPG : public SDLTexture > public: > SDLTextureMJPG(const SDL_Rect &rect); > > - void update(const libcamera::Span<uint8_t> &data) override; > + void update(libcamera::Span<const uint8_t> data) override; > > private: > - int decompress(const libcamera::Span<uint8_t> &data); > + int decompress(libcamera::Span<const uint8_t> data); > > std::unique_ptr<unsigned char[]> rgb_; > }; > diff --git a/src/cam/sdl_texture_yuyv.cpp b/src/cam/sdl_texture_yuyv.cpp > index cc161b2cfa79..637c0900edff 100644 > --- a/src/cam/sdl_texture_yuyv.cpp > +++ b/src/cam/sdl_texture_yuyv.cpp > @@ -14,7 +14,7 @@ SDLTextureYUYV::SDLTextureYUYV(const SDL_Rect &rect) > { > } > > -void SDLTextureYUYV::update(const Span<uint8_t> &data) > +void SDLTextureYUYV::update(Span<const uint8_t> data) > { > SDL_UpdateTexture(ptr_, &rect_, data.data(), pitch_); > } > diff --git a/src/cam/sdl_texture_yuyv.h b/src/cam/sdl_texture_yuyv.h > index 9f7c72f09db2..529a72d6a40e 100644 > --- a/src/cam/sdl_texture_yuyv.h > +++ b/src/cam/sdl_texture_yuyv.h > @@ -13,5 +13,5 @@ class SDLTextureYUYV : public SDLTexture > { > public: > SDLTextureYUYV(const SDL_Rect &rect); > - void update(const libcamera::Span<uint8_t> &data) override; > + void update(libcamera::Span<const uint8_t> data) override; > }; > -- > Regards, > > Laurent Pinchart >
diff --git a/src/cam/sdl_texture.h b/src/cam/sdl_texture.h index 1d8b09288d50..f523fa5ebf51 100644 --- a/src/cam/sdl_texture.h +++ b/src/cam/sdl_texture.h @@ -17,7 +17,7 @@ public: SDLTexture(const SDL_Rect &rect, uint32_t pixelFormat, const int pitch); virtual ~SDLTexture(); int create(SDL_Renderer *renderer); - virtual void update(const libcamera::Span<uint8_t> &data) = 0; + virtual void update(libcamera::Span<const uint8_t> data) = 0; SDL_Texture *get() const { return ptr_; } protected: diff --git a/src/cam/sdl_texture_mjpg.cpp b/src/cam/sdl_texture_mjpg.cpp index 7eddc00cd210..7542efd75d96 100644 --- a/src/cam/sdl_texture_mjpg.cpp +++ b/src/cam/sdl_texture_mjpg.cpp @@ -43,7 +43,7 @@ SDLTextureMJPG::SDLTextureMJPG(const SDL_Rect &rect) { } -int SDLTextureMJPG::decompress(const Span<uint8_t> &data) +int SDLTextureMJPG::decompress(Span<const uint8_t> data) { struct jpeg_decompress_struct cinfo; @@ -76,7 +76,7 @@ int SDLTextureMJPG::decompress(const Span<uint8_t> &data) return 0; } -void SDLTextureMJPG::update(const Span<uint8_t> &data) +void SDLTextureMJPG::update(Span<const uint8_t> data) { decompress(data); SDL_UpdateTexture(ptr_, nullptr, rgb_.get(), pitch_); diff --git a/src/cam/sdl_texture_mjpg.h b/src/cam/sdl_texture_mjpg.h index 328c45a913c5..5141ed73bf70 100644 --- a/src/cam/sdl_texture_mjpg.h +++ b/src/cam/sdl_texture_mjpg.h @@ -14,10 +14,10 @@ class SDLTextureMJPG : public SDLTexture public: SDLTextureMJPG(const SDL_Rect &rect); - void update(const libcamera::Span<uint8_t> &data) override; + void update(libcamera::Span<const uint8_t> data) override; private: - int decompress(const libcamera::Span<uint8_t> &data); + int decompress(libcamera::Span<const uint8_t> data); std::unique_ptr<unsigned char[]> rgb_; }; diff --git a/src/cam/sdl_texture_yuyv.cpp b/src/cam/sdl_texture_yuyv.cpp index cc161b2cfa79..637c0900edff 100644 --- a/src/cam/sdl_texture_yuyv.cpp +++ b/src/cam/sdl_texture_yuyv.cpp @@ -14,7 +14,7 @@ SDLTextureYUYV::SDLTextureYUYV(const SDL_Rect &rect) { } -void SDLTextureYUYV::update(const Span<uint8_t> &data) +void SDLTextureYUYV::update(Span<const uint8_t> data) { SDL_UpdateTexture(ptr_, &rect_, data.data(), pitch_); } diff --git a/src/cam/sdl_texture_yuyv.h b/src/cam/sdl_texture_yuyv.h index 9f7c72f09db2..529a72d6a40e 100644 --- a/src/cam/sdl_texture_yuyv.h +++ b/src/cam/sdl_texture_yuyv.h @@ -13,5 +13,5 @@ class SDLTextureYUYV : public SDLTexture { public: SDLTextureYUYV(const SDL_Rect &rect); - void update(const libcamera::Span<uint8_t> &data) override; + void update(libcamera::Span<const uint8_t> data) override; };
The SDLTexture::update() function isn't meant to modify the data it receives. Make the Span type const to ensure this at compile time. While at it, pass the Span by value instead of reference, as a Span is only a pointer and size, which will fit in registers and will avoid pointer dereferences in the callee. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- src/cam/sdl_texture.h | 2 +- src/cam/sdl_texture_mjpg.cpp | 4 ++-- src/cam/sdl_texture_mjpg.h | 4 ++-- src/cam/sdl_texture_yuyv.cpp | 2 +- src/cam/sdl_texture_yuyv.h | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-)