| Message ID | 20250619082027.4058507-1-chenghaoyang@google.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Hi 2025. 06. 19. 10:20 keltezéssel, Harvey Yang írta: > From: Harvey Yang <chenghaoyang@chromium.org> > > Previously, const global `pixelInfo` in `encoder_libjpeg.cpp` is > initialized before const global `pixelFormatInfo` in `formats.cpp`, > even if `formats.cpp` is wrapped in the base libcamera shared library, > and the android adapter (which contains `encoder_libjpeg.cpp`) > depends on the base libcamera shared library. I generally agree with any removal of non-constinit or mutable global state. So were you getting `Unsupported pixel format ...` warnings previously, or a crash or something else? > > The reason should be that C++ initialization order doesn't follow the > linking process. I would have assumed that any reasonable linker will bring up the dependencies before the dependents, but I suppose it is not impossible that the ordering is different for some reason. Regards, Barnabás Pőcze > > This patch migrates const global pixelInfo to static to avoid the race > condition. > > Signed-off-by: Harvey Yang <chenghaoyang@chromium.org> > --- > src/android/jpeg/encoder_libjpeg.cpp | 28 ++++++++++++++-------------- > 1 file changed, 14 insertions(+), 14 deletions(-) > > diff --git a/src/android/jpeg/encoder_libjpeg.cpp b/src/android/jpeg/encoder_libjpeg.cpp > index cb242b5ec6a8..3ba5103d5ad5 100644 > --- a/src/android/jpeg/encoder_libjpeg.cpp > +++ b/src/android/jpeg/encoder_libjpeg.cpp > @@ -38,26 +38,26 @@ struct JPEGPixelFormatInfo { > bool nvSwap; > }; > > -const std::map<PixelFormat, JPEGPixelFormatInfo> pixelInfo{ > - { formats::R8, { JCS_GRAYSCALE, PixelFormatInfo::info(formats::R8), false } }, > - > - { formats::RGB888, { JCS_EXT_BGR, PixelFormatInfo::info(formats::RGB888), false } }, > - { formats::BGR888, { JCS_EXT_RGB, PixelFormatInfo::info(formats::BGR888), false } }, > - > - { formats::NV12, { JCS_YCbCr, PixelFormatInfo::info(formats::NV12), false } }, > - { formats::NV21, { JCS_YCbCr, PixelFormatInfo::info(formats::NV21), true } }, > - { formats::NV16, { JCS_YCbCr, PixelFormatInfo::info(formats::NV16), false } }, > - { formats::NV61, { JCS_YCbCr, PixelFormatInfo::info(formats::NV61), true } }, > - { formats::NV24, { JCS_YCbCr, PixelFormatInfo::info(formats::NV24), false } }, > - { formats::NV42, { JCS_YCbCr, PixelFormatInfo::info(formats::NV42), true } }, > -}; > - > const struct JPEGPixelFormatInfo &findPixelInfo(const PixelFormat &format) > { > static const struct JPEGPixelFormatInfo invalidPixelFormat { > JCS_UNKNOWN, PixelFormatInfo(), false > }; > > + static const std::map<PixelFormat, JPEGPixelFormatInfo> pixelInfo{ > + { formats::R8, { JCS_GRAYSCALE, PixelFormatInfo::info(formats::R8), false } }, > + > + { formats::RGB888, { JCS_EXT_BGR, PixelFormatInfo::info(formats::RGB888), false } }, > + { formats::BGR888, { JCS_EXT_RGB, PixelFormatInfo::info(formats::BGR888), false } }, > + > + { formats::NV12, { JCS_YCbCr, PixelFormatInfo::info(formats::NV12), false } }, > + { formats::NV21, { JCS_YCbCr, PixelFormatInfo::info(formats::NV21), true } }, > + { formats::NV16, { JCS_YCbCr, PixelFormatInfo::info(formats::NV16), false } }, > + { formats::NV61, { JCS_YCbCr, PixelFormatInfo::info(formats::NV61), true } }, > + { formats::NV24, { JCS_YCbCr, PixelFormatInfo::info(formats::NV24), false } }, > + { formats::NV42, { JCS_YCbCr, PixelFormatInfo::info(formats::NV42), true } }, > + }; > + > const auto iter = pixelInfo.find(format); > if (iter == pixelInfo.end()) { > LOG(JPEG, Error) << "Unsupported pixel format for JPEG encoder: "
Hi Barnabás, On Thu, Jun 19, 2025 at 5:57 PM Barnabás Pőcze <barnabas.pocze@ideasonboard.com> wrote: > > Hi > > 2025. 06. 19. 10:20 keltezéssel, Harvey Yang írta: > > From: Harvey Yang <chenghaoyang@chromium.org> > > > > Previously, const global `pixelInfo` in `encoder_libjpeg.cpp` is > > initialized before const global `pixelFormatInfo` in `formats.cpp`, > > even if `formats.cpp` is wrapped in the base libcamera shared library, > > and the android adapter (which contains `encoder_libjpeg.cpp`) > > depends on the base libcamera shared library. > > I generally agree with any removal of non-constinit or mutable global state. > > So were you getting `Unsupported pixel format ...` warnings previously, > or a crash or something else? Yes, I got `Unsupported pixel format ...` warnings, and after adding logs, I found `pixelFormatInfo` has size 0 when being queried. > > > > > > The reason should be that C++ initialization order doesn't follow the > > linking process. > > I would have assumed that any reasonable linker will bring up the dependencies > before the dependents, but I suppose it is not impossible that the ordering > is different for some reason. The testing environment is Android OS, FYI. BR, Harvey > > > Regards, > Barnabás Pőcze > > > > > This patch migrates const global pixelInfo to static to avoid the race > > condition. > > > > Signed-off-by: Harvey Yang <chenghaoyang@chromium.org> > > --- > > src/android/jpeg/encoder_libjpeg.cpp | 28 ++++++++++++++-------------- > > 1 file changed, 14 insertions(+), 14 deletions(-) > > > > diff --git a/src/android/jpeg/encoder_libjpeg.cpp b/src/android/jpeg/encoder_libjpeg.cpp > > index cb242b5ec6a8..3ba5103d5ad5 100644 > > --- a/src/android/jpeg/encoder_libjpeg.cpp > > +++ b/src/android/jpeg/encoder_libjpeg.cpp > > @@ -38,26 +38,26 @@ struct JPEGPixelFormatInfo { > > bool nvSwap; > > }; > > > > -const std::map<PixelFormat, JPEGPixelFormatInfo> pixelInfo{ > > - { formats::R8, { JCS_GRAYSCALE, PixelFormatInfo::info(formats::R8), false } }, > > - > > - { formats::RGB888, { JCS_EXT_BGR, PixelFormatInfo::info(formats::RGB888), false } }, > > - { formats::BGR888, { JCS_EXT_RGB, PixelFormatInfo::info(formats::BGR888), false } }, > > - > > - { formats::NV12, { JCS_YCbCr, PixelFormatInfo::info(formats::NV12), false } }, > > - { formats::NV21, { JCS_YCbCr, PixelFormatInfo::info(formats::NV21), true } }, > > - { formats::NV16, { JCS_YCbCr, PixelFormatInfo::info(formats::NV16), false } }, > > - { formats::NV61, { JCS_YCbCr, PixelFormatInfo::info(formats::NV61), true } }, > > - { formats::NV24, { JCS_YCbCr, PixelFormatInfo::info(formats::NV24), false } }, > > - { formats::NV42, { JCS_YCbCr, PixelFormatInfo::info(formats::NV42), true } }, > > -}; > > - > > const struct JPEGPixelFormatInfo &findPixelInfo(const PixelFormat &format) > > { > > static const struct JPEGPixelFormatInfo invalidPixelFormat { > > JCS_UNKNOWN, PixelFormatInfo(), false > > }; > > > > + static const std::map<PixelFormat, JPEGPixelFormatInfo> pixelInfo{ > > + { formats::R8, { JCS_GRAYSCALE, PixelFormatInfo::info(formats::R8), false } }, > > + > > + { formats::RGB888, { JCS_EXT_BGR, PixelFormatInfo::info(formats::RGB888), false } }, > > + { formats::BGR888, { JCS_EXT_RGB, PixelFormatInfo::info(formats::BGR888), false } }, > > + > > + { formats::NV12, { JCS_YCbCr, PixelFormatInfo::info(formats::NV12), false } }, > > + { formats::NV21, { JCS_YCbCr, PixelFormatInfo::info(formats::NV21), true } }, > > + { formats::NV16, { JCS_YCbCr, PixelFormatInfo::info(formats::NV16), false } }, > > + { formats::NV61, { JCS_YCbCr, PixelFormatInfo::info(formats::NV61), true } }, > > + { formats::NV24, { JCS_YCbCr, PixelFormatInfo::info(formats::NV24), false } }, > > + { formats::NV42, { JCS_YCbCr, PixelFormatInfo::info(formats::NV42), true } }, > > + }; > > + > > const auto iter = pixelInfo.find(format); > > if (iter == pixelInfo.end()) { > > LOG(JPEG, Error) << "Unsupported pixel format for JPEG encoder: " >
On Thu, Jun 19, 2025 at 06:02:33PM +0800, Cheng-Hao Yang wrote: > On Thu, Jun 19, 2025 at 5:57 PM Barnabás Pőcze wrote: > > 2025. 06. 19. 10:20 keltezéssel, Harvey Yang írta: > > > From: Harvey Yang <chenghaoyang@chromium.org> > > > > > > Previously, const global `pixelInfo` in `encoder_libjpeg.cpp` is > > > initialized before const global `pixelFormatInfo` in `formats.cpp`, > > > even if `formats.cpp` is wrapped in the base libcamera shared library, > > > and the android adapter (which contains `encoder_libjpeg.cpp`) > > > depends on the base libcamera shared library. > > > > I generally agree with any removal of non-constinit or mutable global state. > > > > So were you getting `Unsupported pixel format ...` warnings previously, > > or a crash or something else? > > Yes, I got `Unsupported pixel format ...` warnings, and after adding logs, > I found `pixelFormatInfo` has size 0 when being queried. There's something I don't quite get. src/android/jpeg/encoder_libjpeg.cpp is compiled in libcamera-hal.so, which links against libcamera.so. The latter provides PixelFormatInfo::info(). Does this mean that the global variables of libcamera-hal.so get initialized before those of libcamera.so ? Is that allowed ? More investigation seems to be needed. > > > The reason should be that C++ initialization order doesn't follow the > > > linking process. > > > > I would have assumed that any reasonable linker will bring up the dependencies > > before the dependents, but I suppose it is not impossible that the ordering > > is different for some reason. > > The testing environment is Android OS, FYI. How to you compile libcamera on Android ? > > > This patch migrates const global pixelInfo to static to avoid the race > > > condition. > > > > > > Signed-off-by: Harvey Yang <chenghaoyang@chromium.org> > > > --- > > > src/android/jpeg/encoder_libjpeg.cpp | 28 ++++++++++++++-------------- > > > 1 file changed, 14 insertions(+), 14 deletions(-) > > > > > > diff --git a/src/android/jpeg/encoder_libjpeg.cpp b/src/android/jpeg/encoder_libjpeg.cpp > > > index cb242b5ec6a8..3ba5103d5ad5 100644 > > > --- a/src/android/jpeg/encoder_libjpeg.cpp > > > +++ b/src/android/jpeg/encoder_libjpeg.cpp > > > @@ -38,26 +38,26 @@ struct JPEGPixelFormatInfo { > > > bool nvSwap; > > > }; > > > > > > -const std::map<PixelFormat, JPEGPixelFormatInfo> pixelInfo{ > > > - { formats::R8, { JCS_GRAYSCALE, PixelFormatInfo::info(formats::R8), false } }, > > > - > > > - { formats::RGB888, { JCS_EXT_BGR, PixelFormatInfo::info(formats::RGB888), false } }, > > > - { formats::BGR888, { JCS_EXT_RGB, PixelFormatInfo::info(formats::BGR888), false } }, > > > - > > > - { formats::NV12, { JCS_YCbCr, PixelFormatInfo::info(formats::NV12), false } }, > > > - { formats::NV21, { JCS_YCbCr, PixelFormatInfo::info(formats::NV21), true } }, > > > - { formats::NV16, { JCS_YCbCr, PixelFormatInfo::info(formats::NV16), false } }, > > > - { formats::NV61, { JCS_YCbCr, PixelFormatInfo::info(formats::NV61), true } }, > > > - { formats::NV24, { JCS_YCbCr, PixelFormatInfo::info(formats::NV24), false } }, > > > - { formats::NV42, { JCS_YCbCr, PixelFormatInfo::info(formats::NV42), true } }, > > > -}; > > > - > > > const struct JPEGPixelFormatInfo &findPixelInfo(const PixelFormat &format) > > > { > > > static const struct JPEGPixelFormatInfo invalidPixelFormat { > > > JCS_UNKNOWN, PixelFormatInfo(), false > > > }; > > > > > > + static const std::map<PixelFormat, JPEGPixelFormatInfo> pixelInfo{ > > > + { formats::R8, { JCS_GRAYSCALE, PixelFormatInfo::info(formats::R8), false } }, > > > + > > > + { formats::RGB888, { JCS_EXT_BGR, PixelFormatInfo::info(formats::RGB888), false } }, > > > + { formats::BGR888, { JCS_EXT_RGB, PixelFormatInfo::info(formats::BGR888), false } }, > > > + > > > + { formats::NV12, { JCS_YCbCr, PixelFormatInfo::info(formats::NV12), false } }, > > > + { formats::NV21, { JCS_YCbCr, PixelFormatInfo::info(formats::NV21), true } }, > > > + { formats::NV16, { JCS_YCbCr, PixelFormatInfo::info(formats::NV16), false } }, > > > + { formats::NV61, { JCS_YCbCr, PixelFormatInfo::info(formats::NV61), true } }, > > > + { formats::NV24, { JCS_YCbCr, PixelFormatInfo::info(formats::NV24), false } }, > > > + { formats::NV42, { JCS_YCbCr, PixelFormatInfo::info(formats::NV42), true } }, > > > + }; > > > + > > > const auto iter = pixelInfo.find(format); > > > if (iter == pixelInfo.end()) { > > > LOG(JPEG, Error) << "Unsupported pixel format for JPEG encoder: "
Hi Laurent, On Thu, Jun 19, 2025 at 6:30 PM Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > On Thu, Jun 19, 2025 at 06:02:33PM +0800, Cheng-Hao Yang wrote: > > On Thu, Jun 19, 2025 at 5:57 PM Barnabás Pőcze wrote: > > > 2025. 06. 19. 10:20 keltezéssel, Harvey Yang írta: > > > > From: Harvey Yang <chenghaoyang@chromium.org> > > > > > > > > Previously, const global `pixelInfo` in `encoder_libjpeg.cpp` is > > > > initialized before const global `pixelFormatInfo` in `formats.cpp`, > > > > even if `formats.cpp` is wrapped in the base libcamera shared library, > > > > and the android adapter (which contains `encoder_libjpeg.cpp`) > > > > depends on the base libcamera shared library. > > > > > > I generally agree with any removal of non-constinit or mutable global state. > > > > > > So were you getting `Unsupported pixel format ...` warnings previously, > > > or a crash or something else? > > > > Yes, I got `Unsupported pixel format ...` warnings, and after adding logs, > > I found `pixelFormatInfo` has size 0 when being queried. > > There's something I don't quite get. > src/android/jpeg/encoder_libjpeg.cpp is compiled in libcamera-hal.so, > which links against libcamera.so. The latter provides > PixelFormatInfo::info(). Does this mean that the global variables of > libcamera-hal.so get initialized before those of libcamera.so ? Is that > allowed ? More investigation seems to be needed. I had the same question, while the experiments showed the other way around. There are also similar discussions like: https://stackoverflow.com/questions/3746238/c-global-initialization-order-ignores-dependencies > > > > > The reason should be that C++ initialization order doesn't follow the > > > > linking process. > > > > > > I would have assumed that any reasonable linker will bring up the dependencies > > > before the dependents, but I suppose it is not impossible that the ordering > > > is different for some reason. > > > > The testing environment is Android OS, FYI. > > How to you compile libcamera on Android ? Currently we have to rewrite with Android.bp, instead of using the available `meson.build`s, while as I mentioned before, I built `libcamera_src_defaults`, which contains `src/libcamera/formats.cpp`. And built `libcamera_android_src_defaults` (depending on `libcamera_src_defaults`) , which contains `src/android/jpeg/encoder_libjpeg.cpp`. BR, Harvey > > > > > This patch migrates const global pixelInfo to static to avoid the race > > > > condition. > > > > > > > > Signed-off-by: Harvey Yang <chenghaoyang@chromium.org> > > > > --- > > > > src/android/jpeg/encoder_libjpeg.cpp | 28 ++++++++++++++-------------- > > > > 1 file changed, 14 insertions(+), 14 deletions(-) > > > > > > > > diff --git a/src/android/jpeg/encoder_libjpeg.cpp b/src/android/jpeg/encoder_libjpeg.cpp > > > > index cb242b5ec6a8..3ba5103d5ad5 100644 > > > > --- a/src/android/jpeg/encoder_libjpeg.cpp > > > > +++ b/src/android/jpeg/encoder_libjpeg.cpp > > > > @@ -38,26 +38,26 @@ struct JPEGPixelFormatInfo { > > > > bool nvSwap; > > > > }; > > > > > > > > -const std::map<PixelFormat, JPEGPixelFormatInfo> pixelInfo{ > > > > - { formats::R8, { JCS_GRAYSCALE, PixelFormatInfo::info(formats::R8), false } }, > > > > - > > > > - { formats::RGB888, { JCS_EXT_BGR, PixelFormatInfo::info(formats::RGB888), false } }, > > > > - { formats::BGR888, { JCS_EXT_RGB, PixelFormatInfo::info(formats::BGR888), false } }, > > > > - > > > > - { formats::NV12, { JCS_YCbCr, PixelFormatInfo::info(formats::NV12), false } }, > > > > - { formats::NV21, { JCS_YCbCr, PixelFormatInfo::info(formats::NV21), true } }, > > > > - { formats::NV16, { JCS_YCbCr, PixelFormatInfo::info(formats::NV16), false } }, > > > > - { formats::NV61, { JCS_YCbCr, PixelFormatInfo::info(formats::NV61), true } }, > > > > - { formats::NV24, { JCS_YCbCr, PixelFormatInfo::info(formats::NV24), false } }, > > > > - { formats::NV42, { JCS_YCbCr, PixelFormatInfo::info(formats::NV42), true } }, > > > > -}; > > > > - > > > > const struct JPEGPixelFormatInfo &findPixelInfo(const PixelFormat &format) > > > > { > > > > static const struct JPEGPixelFormatInfo invalidPixelFormat { > > > > JCS_UNKNOWN, PixelFormatInfo(), false > > > > }; > > > > > > > > + static const std::map<PixelFormat, JPEGPixelFormatInfo> pixelInfo{ > > > > + { formats::R8, { JCS_GRAYSCALE, PixelFormatInfo::info(formats::R8), false } }, > > > > + > > > > + { formats::RGB888, { JCS_EXT_BGR, PixelFormatInfo::info(formats::RGB888), false } }, > > > > + { formats::BGR888, { JCS_EXT_RGB, PixelFormatInfo::info(formats::BGR888), false } }, > > > > + > > > > + { formats::NV12, { JCS_YCbCr, PixelFormatInfo::info(formats::NV12), false } }, > > > > + { formats::NV21, { JCS_YCbCr, PixelFormatInfo::info(formats::NV21), true } }, > > > > + { formats::NV16, { JCS_YCbCr, PixelFormatInfo::info(formats::NV16), false } }, > > > > + { formats::NV61, { JCS_YCbCr, PixelFormatInfo::info(formats::NV61), true } }, > > > > + { formats::NV24, { JCS_YCbCr, PixelFormatInfo::info(formats::NV24), false } }, > > > > + { formats::NV42, { JCS_YCbCr, PixelFormatInfo::info(formats::NV42), true } }, > > > > + }; > > > > + > > > > const auto iter = pixelInfo.find(format); > > > > if (iter == pixelInfo.end()) { > > > > LOG(JPEG, Error) << "Unsupported pixel format for JPEG encoder: " > > -- > Regards, > > Laurent Pinchart
Hi 2025. 06. 20. 6:33 keltezéssel, Cheng-Hao Yang írta: > Hi Laurent, > > On Thu, Jun 19, 2025 at 6:30 PM Laurent Pinchart > <laurent.pinchart@ideasonboard.com> wrote: >> >> On Thu, Jun 19, 2025 at 06:02:33PM +0800, Cheng-Hao Yang wrote: >>> On Thu, Jun 19, 2025 at 5:57 PM Barnabás Pőcze wrote: >>>> 2025. 06. 19. 10:20 keltezéssel, Harvey Yang írta: >>>>> From: Harvey Yang <chenghaoyang@chromium.org> >>>>> >>>>> Previously, const global `pixelInfo` in `encoder_libjpeg.cpp` is >>>>> initialized before const global `pixelFormatInfo` in `formats.cpp`, >>>>> even if `formats.cpp` is wrapped in the base libcamera shared library, >>>>> and the android adapter (which contains `encoder_libjpeg.cpp`) >>>>> depends on the base libcamera shared library. >>>> >>>> I generally agree with any removal of non-constinit or mutable global state. >>>> >>>> So were you getting `Unsupported pixel format ...` warnings previously, >>>> or a crash or something else? >>> >>> Yes, I got `Unsupported pixel format ...` warnings, and after adding logs, >>> I found `pixelFormatInfo` has size 0 when being queried. >> >> There's something I don't quite get. >> src/android/jpeg/encoder_libjpeg.cpp is compiled in libcamera-hal.so, >> which links against libcamera.so. The latter provides >> PixelFormatInfo::info(). Does this mean that the global variables of >> libcamera-hal.so get initialized before those of libcamera.so ? Is that >> allowed ? More investigation seems to be needed. > > I had the same question, while the experiments showed the other way > around. There are also similar discussions like: > https://stackoverflow.com/questions/3746238/c-global-initialization-order-ignores-dependencies The case presented there I think is clear. But here we have two shared libraries where one depends on the other. It seems a bit odd that the dynamic linker would start the initialization of the dependent before the initialization of the dependency is done. > > >> >>>>> The reason should be that C++ initialization order doesn't follow the >>>>> linking process. >>>> >>>> I would have assumed that any reasonable linker will bring up the dependencies >>>> before the dependents, but I suppose it is not impossible that the ordering >>>> is different for some reason. >>> >>> The testing environment is Android OS, FYI. >> >> How to you compile libcamera on Android ? > > Currently we have to rewrite with Android.bp, instead of using the available > `meson.build`s, while as I mentioned before, I built `libcamera_src_defaults`, > which contains `src/libcamera/formats.cpp`. And built > `libcamera_android_src_defaults` (depending on `libcamera_src_defaults`) > , which contains `src/android/jpeg/encoder_libjpeg.cpp`. Can you tell us where that `Android.bp` file is? I can't find it. It does not make any of the two a static library, correct? Regards, Barnabás Pőcze > > BR, > Harvey > > [...]
Hi Barnabás, On Fri, Jun 20, 2025 at 3:30 PM Barnabás Pőcze <barnabas.pocze@ideasonboard.com> wrote: > > Hi > > 2025. 06. 20. 6:33 keltezéssel, Cheng-Hao Yang írta: > > Hi Laurent, > > > > On Thu, Jun 19, 2025 at 6:30 PM Laurent Pinchart > > <laurent.pinchart@ideasonboard.com> wrote: > >> > >> On Thu, Jun 19, 2025 at 06:02:33PM +0800, Cheng-Hao Yang wrote: > >>> On Thu, Jun 19, 2025 at 5:57 PM Barnabás Pőcze wrote: > >>>> 2025. 06. 19. 10:20 keltezéssel, Harvey Yang írta: > >>>>> From: Harvey Yang <chenghaoyang@chromium.org> > >>>>> > >>>>> Previously, const global `pixelInfo` in `encoder_libjpeg.cpp` is > >>>>> initialized before const global `pixelFormatInfo` in `formats.cpp`, > >>>>> even if `formats.cpp` is wrapped in the base libcamera shared library, > >>>>> and the android adapter (which contains `encoder_libjpeg.cpp`) > >>>>> depends on the base libcamera shared library. > >>>> > >>>> I generally agree with any removal of non-constinit or mutable global state. > >>>> > >>>> So were you getting `Unsupported pixel format ...` warnings previously, > >>>> or a crash or something else? > >>> > >>> Yes, I got `Unsupported pixel format ...` warnings, and after adding logs, > >>> I found `pixelFormatInfo` has size 0 when being queried. > >> > >> There's something I don't quite get. > >> src/android/jpeg/encoder_libjpeg.cpp is compiled in libcamera-hal.so, > >> which links against libcamera.so. The latter provides > >> PixelFormatInfo::info(). Does this mean that the global variables of > >> libcamera-hal.so get initialized before those of libcamera.so ? Is that > >> allowed ? More investigation seems to be needed. > > > > I had the same question, while the experiments showed the other way > > around. There are also similar discussions like: > > https://stackoverflow.com/questions/3746238/c-global-initialization-order-ignores-dependencies > > The case presented there I think is clear. But here we have two shared libraries > where one depends on the other. It seems a bit odd that the dynamic linker would > start the initialization of the dependent before the initialization of the > dependency is done. > > > > > > > >> > >>>>> The reason should be that C++ initialization order doesn't follow the > >>>>> linking process. > >>>> > >>>> I would have assumed that any reasonable linker will bring up the dependencies > >>>> before the dependents, but I suppose it is not impossible that the ordering > >>>> is different for some reason. > >>> > >>> The testing environment is Android OS, FYI. > >> > >> How to you compile libcamera on Android ? > > > > Currently we have to rewrite with Android.bp, instead of using the available > > `meson.build`s, while as I mentioned before, I built `libcamera_src_defaults`, > > which contains `src/libcamera/formats.cpp`. And built > > `libcamera_android_src_defaults` (depending on `libcamera_src_defaults`) > > , which contains `src/android/jpeg/encoder_libjpeg.cpp`. > > Can you tell us where that `Android.bp` file is? I can't find it. It does not > make any of the two a static library, correct? As it's in an Android vertical (for ChromeOS) repository, it's not public now, sorry. After some more investigation, I think I found what I missed: I used `cc_defaults` in Android.bp to build the core sources and the Android Adapter sources, while it doesn't produce intermediate results/shared libraries. When I added the final shared library with Virtual pipeline handler, it combines all the defaults dependencies into a single shared library, instead of doing it with linking, I believe. Seems that it's only a special case for Android. I'll check if we should refine our Android.bp to avoid cc_defaults or just adopt this patch in our repository. Doesn't need to merge this patch into the upstream though. BR, Harvey > > > Regards, > Barnabás Pőcze > > > > > > BR, > > Harvey > > > > [...] >
diff --git a/src/android/jpeg/encoder_libjpeg.cpp b/src/android/jpeg/encoder_libjpeg.cpp index cb242b5ec6a8..3ba5103d5ad5 100644 --- a/src/android/jpeg/encoder_libjpeg.cpp +++ b/src/android/jpeg/encoder_libjpeg.cpp @@ -38,26 +38,26 @@ struct JPEGPixelFormatInfo { bool nvSwap; }; -const std::map<PixelFormat, JPEGPixelFormatInfo> pixelInfo{ - { formats::R8, { JCS_GRAYSCALE, PixelFormatInfo::info(formats::R8), false } }, - - { formats::RGB888, { JCS_EXT_BGR, PixelFormatInfo::info(formats::RGB888), false } }, - { formats::BGR888, { JCS_EXT_RGB, PixelFormatInfo::info(formats::BGR888), false } }, - - { formats::NV12, { JCS_YCbCr, PixelFormatInfo::info(formats::NV12), false } }, - { formats::NV21, { JCS_YCbCr, PixelFormatInfo::info(formats::NV21), true } }, - { formats::NV16, { JCS_YCbCr, PixelFormatInfo::info(formats::NV16), false } }, - { formats::NV61, { JCS_YCbCr, PixelFormatInfo::info(formats::NV61), true } }, - { formats::NV24, { JCS_YCbCr, PixelFormatInfo::info(formats::NV24), false } }, - { formats::NV42, { JCS_YCbCr, PixelFormatInfo::info(formats::NV42), true } }, -}; - const struct JPEGPixelFormatInfo &findPixelInfo(const PixelFormat &format) { static const struct JPEGPixelFormatInfo invalidPixelFormat { JCS_UNKNOWN, PixelFormatInfo(), false }; + static const std::map<PixelFormat, JPEGPixelFormatInfo> pixelInfo{ + { formats::R8, { JCS_GRAYSCALE, PixelFormatInfo::info(formats::R8), false } }, + + { formats::RGB888, { JCS_EXT_BGR, PixelFormatInfo::info(formats::RGB888), false } }, + { formats::BGR888, { JCS_EXT_RGB, PixelFormatInfo::info(formats::BGR888), false } }, + + { formats::NV12, { JCS_YCbCr, PixelFormatInfo::info(formats::NV12), false } }, + { formats::NV21, { JCS_YCbCr, PixelFormatInfo::info(formats::NV21), true } }, + { formats::NV16, { JCS_YCbCr, PixelFormatInfo::info(formats::NV16), false } }, + { formats::NV61, { JCS_YCbCr, PixelFormatInfo::info(formats::NV61), true } }, + { formats::NV24, { JCS_YCbCr, PixelFormatInfo::info(formats::NV24), false } }, + { formats::NV42, { JCS_YCbCr, PixelFormatInfo::info(formats::NV42), true } }, + }; + const auto iter = pixelInfo.find(format); if (iter == pixelInfo.end()) { LOG(JPEG, Error) << "Unsupported pixel format for JPEG encoder: "