| Message ID | 20230124092218.2266563-1-raj.khem@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Hi Khem, Quoting Khem Raj via libcamera-devel (2023-01-24 09:22:18) > Fixes following errors with gcc-13 > > ../git/src/cam/file_sink.cpp:92:45: error: possibly dangling reference to a temporary [-Werror=dangling-reference] > 92 | const FrameMetadata::Plane &meta = buffer->metadata().planes()[i]; > | ^~~~ > ../git/src/cam/file_sink.cpp:92:81: note: the temporary was destroyed at the end of the full expression '(& buffer->libcamera::FrameBuffer::metadata())->libcamera::FrameMetadata::planes().libcamera::Span<const libcamera::FrameMetadata::Plane>::operator[](i)' > 92 | const FrameMetadata::Plane &meta = buffer->metadata().planes()[i]; > | ^ > cc1plus: all warnings being treated as errors > > Signed-off-by: Khem Raj <raj.khem@gmail.com> Thanks for identifying and fixing this issue. Wim has also discovered this today. He reports that your patch does not apply on the latest libcamera, and looking at the paths below - we moved src/cam/ into src/apps/cam a few weeks back. Could you rebase this on top of master please? If you can Cc Wim, who is also affected by this, I expect he can also test it and we can get it merged quickly. Thanks -- Kieran > --- > src/cam/file_sink.cpp | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/src/cam/file_sink.cpp b/src/cam/file_sink.cpp > index 45213d4a..897c4b37 100644 > --- a/src/cam/file_sink.cpp > +++ b/src/cam/file_sink.cpp > @@ -89,13 +89,13 @@ void FileSink::writeBuffer(const Stream *stream, FrameBuffer *buffer) > Image *image = mappedBuffers_[buffer].get(); > > for (unsigned int i = 0; i < buffer->planes().size(); ++i) { > - const FrameMetadata::Plane &meta = buffer->metadata().planes()[i]; > + unsigned int bytesused = buffer->metadata().planes()[i].bytesused; > > Span<uint8_t> data = image->data(i); > - unsigned int length = std::min<unsigned int>(meta.bytesused, data.size()); > + unsigned int length = std::min<unsigned int>(bytesused, data.size()); > > - if (meta.bytesused > data.size()) > - std::cerr << "payload size " << meta.bytesused > + if (bytesused > data.size()) > + std::cerr << "payload size " << bytesused > << " larger than plane size " << data.size() > << std::endl; > > -- > 2.39.1 >
Hi On 2023. január 24., kedd 10:22, Khem Raj via libcamera-devel <libcamera-devel@lists.libcamera.org> wrote: > Fixes following errors with gcc-13 > > ../git/src/cam/file_sink.cpp:92:45: error: possibly dangling reference to a temporary [-Werror=dangling-reference] > 92 | const FrameMetadata::Plane &meta = buffer->metadata().planes()[i]; > | ^~~~ > ../git/src/cam/file_sink.cpp:92:81: note: the temporary was destroyed at the end of the full expression '(& buffer->libcamera::FrameBuffer::metadata())->libcamera::FrameMetadata::planes().libcamera::Span<const libcamera::FrameMetadata::Plane>::operator[](i)' > 92 | const FrameMetadata::Plane &meta = buffer->metadata().planes()[i]; > | ^ > cc1plus: all warnings being treated as errors I think this warning is a false-positive. See a similar example here: https://gcc.godbolt.org/z/rf8v4j6YG This should probably be reported to the GCC bug tracker if that has not already been done. > > Signed-off-by: Khem Raj <raj.khem@gmail.com> > --- > src/cam/file_sink.cpp | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/src/cam/file_sink.cpp b/src/cam/file_sink.cpp > index 45213d4a..897c4b37 100644 > --- a/src/cam/file_sink.cpp > +++ b/src/cam/file_sink.cpp > @@ -89,13 +89,13 @@ void FileSink::writeBuffer(const Stream *stream, FrameBuffer *buffer) > Image *image = mappedBuffers_[buffer].get(); > > for (unsigned int i = 0; i < buffer->planes().size(); ++i) { > - const FrameMetadata::Plane &meta = buffer->metadata().planes()[i]; > + unsigned int bytesused = buffer->metadata().planes()[i].bytesused; > > Span<uint8_t> data = image->data(i); > - unsigned int length = std::min<unsigned int>(meta.bytesused, data.size()); > + unsigned int length = std::min<unsigned int>(bytesused, data.size()); > > - if (meta.bytesused > data.size()) > - std::cerr << "payload size " << meta.bytesused > + if (bytesused > data.size()) > + std::cerr << "payload size " << bytesused > << " larger than plane size " << data.size() > << std::endl; > > -- > 2.39.1 > > Regards, Barnabás Pőcze
On Tue, 24 Jan 2023 at 09:22, Khem Raj via libcamera-devel <libcamera-devel@lists.libcamera.org> wrote: > > Fixes following errors with gcc-13 > > ../git/src/cam/file_sink.cpp:92:45: error: possibly dangling reference to a temporary [-Werror=dangling-reference] > 92 | const FrameMetadata::Plane &meta = buffer->metadata().planes()[i]; > | ^~~~ > ../git/src/cam/file_sink.cpp:92:81: note: the temporary was destroyed at the end of the full expression '(& buffer->libcamera::FrameBuffer::metadata())->libcamera::FrameMetadata::planes().libcamera::Span<const libcamera::FrameMetadata::Plane>::operator[](i)' > 92 | const FrameMetadata::Plane &meta = buffer->metadata().planes()[i]; > | ^ > cc1plus: all warnings being treated as errors > > Signed-off-by: Khem Raj <raj.khem@gmail.com> I understand this is a compiler bug, but could we merge this anyway? Having the same issue in Fedora 38 and this code is just as good as the code that reproduces the compiler bug. Reviewed-by: Eric Curtin <ecurtin@redhat.com> Is mise le meas/Regards, Eric Curtin > --- > src/cam/file_sink.cpp | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/src/cam/file_sink.cpp b/src/cam/file_sink.cpp > index 45213d4a..897c4b37 100644 > --- a/src/cam/file_sink.cpp > +++ b/src/cam/file_sink.cpp > @@ -89,13 +89,13 @@ void FileSink::writeBuffer(const Stream *stream, FrameBuffer *buffer) > Image *image = mappedBuffers_[buffer].get(); > > for (unsigned int i = 0; i < buffer->planes().size(); ++i) { > - const FrameMetadata::Plane &meta = buffer->metadata().planes()[i]; > + unsigned int bytesused = buffer->metadata().planes()[i].bytesused; > > Span<uint8_t> data = image->data(i); > - unsigned int length = std::min<unsigned int>(meta.bytesused, data.size()); > + unsigned int length = std::min<unsigned int>(bytesused, data.size()); > > - if (meta.bytesused > data.size()) > - std::cerr << "payload size " << meta.bytesused > + if (bytesused > data.size()) > + std::cerr << "payload size " << bytesused > << " larger than plane size " << data.size() > << std::endl; > > -- > 2.39.1 >
diff --git a/src/cam/file_sink.cpp b/src/cam/file_sink.cpp index 45213d4a..897c4b37 100644 --- a/src/cam/file_sink.cpp +++ b/src/cam/file_sink.cpp @@ -89,13 +89,13 @@ void FileSink::writeBuffer(const Stream *stream, FrameBuffer *buffer) Image *image = mappedBuffers_[buffer].get(); for (unsigned int i = 0; i < buffer->planes().size(); ++i) { - const FrameMetadata::Plane &meta = buffer->metadata().planes()[i]; + unsigned int bytesused = buffer->metadata().planes()[i].bytesused; Span<uint8_t> data = image->data(i); - unsigned int length = std::min<unsigned int>(meta.bytesused, data.size()); + unsigned int length = std::min<unsigned int>(bytesused, data.size()); - if (meta.bytesused > data.size()) - std::cerr << "payload size " << meta.bytesused + if (bytesused > data.size()) + std::cerr << "payload size " << bytesused << " larger than plane size " << data.size() << std::endl;
Fixes following errors with gcc-13 ../git/src/cam/file_sink.cpp:92:45: error: possibly dangling reference to a temporary [-Werror=dangling-reference] 92 | const FrameMetadata::Plane &meta = buffer->metadata().planes()[i]; | ^~~~ ../git/src/cam/file_sink.cpp:92:81: note: the temporary was destroyed at the end of the full expression '(& buffer->libcamera::FrameBuffer::metadata())->libcamera::FrameMetadata::planes().libcamera::Span<const libcamera::FrameMetadata::Plane>::operator[](i)' 92 | const FrameMetadata::Plane &meta = buffer->metadata().planes()[i]; | ^ cc1plus: all warnings being treated as errors Signed-off-by: Khem Raj <raj.khem@gmail.com> --- src/cam/file_sink.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)