| Message ID | 20200904095148.844292-2-kieran.bingham@ideasonboard.com |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series |
|
| Related | show |
On Fri, 4 Sep 2020 10:51:45 +0100 Kieran Bingham <kieran.bingham@ideasonboard.com> wrote: > The ControlValue structure is currently defined with a 16-bit hole > (causing unaligned access to the numElements_ field, though that's a > separate topic). > > This structure has a static assertion to ensure that its size does not > change without due care, as it forms part of our ABI and is used in > Serialisation between the pipeline handlers and IPA componenents. > > The m68k architecture is the only target which fails this assertion, > which is likely because it can pack the structure more efficiently, > producing a different binary size. > > This is likely an area we will tackle before stabilising our ABI, but > until then, disable m68k builds as libcamera is not expected to be > supported on this target. > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> We normally ask to have an autobuilder failure reference here. No need to resend just for that, we can add it when applying. Thanks! Thomas
Hi Thomas, On 04/09/2020 13:50, Thomas Petazzoni wrote: > On Fri, 4 Sep 2020 10:51:45 +0100 > Kieran Bingham <kieran.bingham@ideasonboard.com> wrote: > >> The ControlValue structure is currently defined with a 16-bit hole >> (causing unaligned access to the numElements_ field, though that's a >> separate topic). >> >> This structure has a static assertion to ensure that its size does not >> change without due care, as it forms part of our ABI and is used in >> Serialisation between the pipeline handlers and IPA componenents. >> >> The m68k architecture is the only target which fails this assertion, >> which is likely because it can pack the structure more efficiently, >> producing a different binary size. >> >> This is likely an area we will tackle before stabilising our ABI, but >> until then, disable m68k builds as libcamera is not expected to be >> supported on this target. >> >> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > We normally ask to have an autobuilder failure reference here. No need > to resend just for that, we can add it when applying. Sorry, I didn't realise. But don't worry -there are plenty ;-) Here's the latest: > Results for the '2020.08.x' branch > ---------------------------------- > > Build failures related to your packages: > > arch | reason | url > -------------+--------------------------------+--------------------------------------------------------------------------------- > m68k | libcamera-96fab38e02792a109... | http://autobuild.buildroot.net/results/9dce26e94299a2c61bba60cbc7803926e2f85e29
On Fri, 4 Sep 2020 10:51:45 +0100 Kieran Bingham <kieran.bingham@ideasonboard.com> wrote: > diff --git a/package/libcamera/Config.in b/package/libcamera/Config.in > index 960d78b82e95..1ea747d1c57e 100644 > --- a/package/libcamera/Config.in > +++ b/package/libcamera/Config.in > @@ -5,6 +5,8 @@ menuconfig BR2_PACKAGE_LIBCAMERA > depends on BR2_TOOLCHAIN_GCC_AT_LEAST_5 # C++14 > depends on !BR2_STATIC_LIBS # gnutls > depends on BR2_USE_WCHAR # gnutls > + # Invalid packing size of ControlValue struct on m68k > + depends on !BR2_m68k Another comment: this dependency should be replicated in the Config.in comment for libcamera, so that the comment that says "libcamera needs a toolchain w/ ..." doesn't appear on m68k... which would be meaningless as libcamera is not available at all for this architecture. In general, we like to have: config BR2_PACKAGE_LIBCAMERA_ARCH_SUPPORTS config default y depends on !BR2_m68k and then use that in BR2_PACKAGE_LIBCAMERA and the Config.in comment. This way, if other packages select libcamera, they can also re-use this "depends on BR2_PACKAGE_LIBCAMERA_ARCH_SUPPORTS". Thomas
Kieran, All, On 2020-09-04 10:51 +0100, Kieran Bingham spake thusly: > The ControlValue structure is currently defined with a 16-bit hole > (causing unaligned access to the numElements_ field, though that's a > separate topic). > > This structure has a static assertion to ensure that its size does not > change without due care, as it forms part of our ABI and is used in > Serialisation between the pipeline handlers and IPA componenents. > > The m68k architecture is the only target which fails this assertion, > which is likely because it can pack the structure more efficiently, > producing a different binary size. > > This is likely an area we will tackle before stabilising our ABI, but > until then, disable m68k builds as libcamera is not expected to be > supported on this target. > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> Applied to master, after doing the slight adjustments suggested by Thomas: - introduce BR2_PACKAGE_LIBCAMERA_ARCH_SUPPORTS - propagate that to the comment - add autobuilder reference Thanks! Regards, Yann E. MORIN. > --- > package/libcamera/Config.in | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/package/libcamera/Config.in b/package/libcamera/Config.in > index 960d78b82e95..1ea747d1c57e 100644 > --- a/package/libcamera/Config.in > +++ b/package/libcamera/Config.in > @@ -5,6 +5,8 @@ menuconfig BR2_PACKAGE_LIBCAMERA > depends on BR2_TOOLCHAIN_GCC_AT_LEAST_5 # C++14 > depends on !BR2_STATIC_LIBS # gnutls > depends on BR2_USE_WCHAR # gnutls > + # Invalid packing size of ControlValue struct on m68k > + depends on !BR2_m68k > select BR2_PACKAGE_GNUTLS > select BR2_PACKAGE_LIBCAMERA_PIPELINE_UVCVIDEO if !BR2_PACKAGE_LIBCAMERA_HAS_PIPELINE > help > -- > 2.25.1 > > _______________________________________________ > buildroot mailing list > buildroot@busybox.net > http://lists.busybox.net/mailman/listinfo/buildroot
Kieran, All, On 2020-09-04 22:33 +0200, Yann E. MORIN spake thusly: > On 2020-09-04 10:51 +0100, Kieran Bingham spake thusly: > > The ControlValue structure is currently defined with a 16-bit hole > > (causing unaligned access to the numElements_ field, though that's a > > separate topic). > > > > This structure has a static assertion to ensure that its size does not > > change without due care, as it forms part of our ABI and is used in > > Serialisation between the pipeline handlers and IPA componenents. > > > > The m68k architecture is the only target which fails this assertion, > > which is likely because it can pack the structure more efficiently, > > producing a different binary size. > > > > This is likely an area we will tackle before stabilising our ABI, but > > until then, disable m68k builds as libcamera is not expected to be > > supported on this target. > > > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > Applied to master, after doing the slight adjustments suggested by > Thomas: > > - introduce BR2_PACKAGE_LIBCAMERA_ARCH_SUPPORTS > - propagate that to the comment > - add autobuilder reference Ah, I forgot to note: - move the arch dependency to the top (indeed we like to have those dependencies first: https://buildroot.org/downloads/manual/manual.html#_config_files Cheers! Regards, Yann E. MORIN. > Thanks! > > Regards, > Yann E. MORIN. > > > --- > > package/libcamera/Config.in | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/package/libcamera/Config.in b/package/libcamera/Config.in > > index 960d78b82e95..1ea747d1c57e 100644 > > --- a/package/libcamera/Config.in > > +++ b/package/libcamera/Config.in > > @@ -5,6 +5,8 @@ menuconfig BR2_PACKAGE_LIBCAMERA > > depends on BR2_TOOLCHAIN_GCC_AT_LEAST_5 # C++14 > > depends on !BR2_STATIC_LIBS # gnutls > > depends on BR2_USE_WCHAR # gnutls > > + # Invalid packing size of ControlValue struct on m68k > > + depends on !BR2_m68k > > select BR2_PACKAGE_GNUTLS > > select BR2_PACKAGE_LIBCAMERA_PIPELINE_UVCVIDEO if !BR2_PACKAGE_LIBCAMERA_HAS_PIPELINE > > help > > -- > > 2.25.1 > > > > _______________________________________________ > > buildroot mailing list > > buildroot@busybox.net > > http://lists.busybox.net/mailman/listinfo/buildroot > > -- > .-----------------.--------------------.------------------.--------------------. > | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | > | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | > | +33 561 099 427 `------------.-------: X AGAINST | \e/ There is no | > | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | > '------------------------------^-------^------------------^--------------------' > _______________________________________________ > buildroot mailing list > buildroot@busybox.net > http://lists.busybox.net/mailman/listinfo/buildroot
>>>>> "Kieran" == Kieran Bingham <kieran.bingham@ideasonboard.com> writes: > The ControlValue structure is currently defined with a 16-bit hole > (causing unaligned access to the numElements_ field, though that's a > separate topic). > This structure has a static assertion to ensure that its size does not > change without due care, as it forms part of our ABI and is used in > Serialisation between the pipeline handlers and IPA componenents. > The m68k architecture is the only target which fails this assertion, > which is likely because it can pack the structure more efficiently, > producing a different binary size. > This is likely an area we will tackle before stabilising our ABI, but > until then, disable m68k builds as libcamera is not expected to be > supported on this target. > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> Committed to 2020.08.x, thanks.
diff --git a/package/libcamera/Config.in b/package/libcamera/Config.in index 960d78b82e95..1ea747d1c57e 100644 --- a/package/libcamera/Config.in +++ b/package/libcamera/Config.in @@ -5,6 +5,8 @@ menuconfig BR2_PACKAGE_LIBCAMERA depends on BR2_TOOLCHAIN_GCC_AT_LEAST_5 # C++14 depends on !BR2_STATIC_LIBS # gnutls depends on BR2_USE_WCHAR # gnutls + # Invalid packing size of ControlValue struct on m68k + depends on !BR2_m68k select BR2_PACKAGE_GNUTLS select BR2_PACKAGE_LIBCAMERA_PIPELINE_UVCVIDEO if !BR2_PACKAGE_LIBCAMERA_HAS_PIPELINE help
The ControlValue structure is currently defined with a 16-bit hole (causing unaligned access to the numElements_ field, though that's a separate topic). This structure has a static assertion to ensure that its size does not change without due care, as it forms part of our ABI and is used in Serialisation between the pipeline handlers and IPA componenents. The m68k architecture is the only target which fails this assertion, which is likely because it can pack the structure more efficiently, producing a different binary size. This is likely an area we will tackle before stabilising our ABI, but until then, disable m68k builds as libcamera is not expected to be supported on this target. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> --- package/libcamera/Config.in | 2 ++ 1 file changed, 2 insertions(+)