fork of libcamera

[christianrauch]

Commit 82c5ea2 is the last common commit (i.e. the merge-base) between the upstream master branch and your main branch, after which API incompatible controls (rpi::ScalerCrops) were added in 554b2a5.

Having this on the main branch suggests that this is now a fork of the upstream project and not temporary work that is supposed to be merged back upstream. Having this API-incompatible change released also supports this view.

Could you please elaborate on the strategy behind this? Are you trying to mere these changes into the upstream project (in which case you should probably not release those API-incompatible changes but keep them on a feature branch) or is this now an incompatible fork of libcamera (in which case you should probably rename the project)?

[naushir]

The libcamera project requires all kernel drivers to be available in the upstream kernel tree. We have been working with them over the last year to upstream the ISP and CSI-2 drivers for the Raspberry Pi 5 platform. The ISP (backend) driver has recently been merged, and the CSI-2 (cfe) driver is not there yet, but close. Because of this, we don't yet have Pi5 support in upstream libcamera, but have this fork where it is available for Raspberry Pi users. Eventually, once the CSI-2 driver is available upstream, we will merge Pi 5 support into mainline libcamera. I look forward to that day, as maintaining two trees while developing new features takes quite a lot of effort and resources.

Regarding the rpi::ScalerCrops change, we have pushed the patches to be merged upstream (https://patchwork.libcamera.org/project/libcamera/list/?series=4501). There have been some minor comments that need addressing, but we will merge it soon as well. Our intention has been and continues to be to push all our development into the mainline libcamera tree. However, rpi::ScalerCrops is a key feature that some of our customers need immediately, so we merged it in our downstream tree to start with. Note that this is not an API incompatible change, since the rpi::ScalerCrops relies on the use of vendor controls, a feature we introduced into libcamera to avoid API breakages in such cases.

If I can clarify anything further, please let me know.

[christianrauch]

Regarding the rpi::ScalerCrops change, we have pushed the patches to be merged upstream (https://patchwork.libcamera.org/project/libcamera/list/?series=4501). There have been some minor comments that need addressing, but we will merge it soon as well. Our intention has been and continues to be to push all our development into the mainline libcamera tree. However, rpi::ScalerCrops is a key feature that some of our customers need immediately, so we merged it in our downstream tree to start with.

I am very happy to hear that you intend to merge these changes back upstream again and I can now understand why you have to "deploy" these features before they are merged upstream.

Note that this is not an API incompatible change, since the rpi::ScalerCrops relies on the use of vendor controls, a feature we introduced into libcamera to avoid API breakages in such cases.

Do you mean using the macro LIBCAMERA_HAS_RPI_VENDOR_CONTROLS to check for vendor controls? This is already used in the upstream libcamera to check for rpi::StatsOutputEnable and rpi::Bcm2835StatsOutput. What I mean with API breakage is when you write code that additionally uses rpi::ScalerCrops. You cannot test if this is available at build time, so code written against the raspberrypi fork using rpi::ScalerCrops will fail to build with the upstream project.

If you need to "deploy" the StatsOutputEnable feature already now, maybe it is better to do this in the rpi::draft namespace and use another macro like LIBCAMERA_HAS_RPI_DRAFT_VENDOR_CONTROLS. This way, the control rpi::draft::StatsOutputEnable can already be used by custom applications but you let users know that this is not yet officially part of the upstream vendor namespace and code using that macro could still be compiled with the upstream and the fork of libcamera.

[naushir]

We will update rpi::ScalerCrops to be put under a LIBCAMERA_HAS_RPI_VENDOR_CONTROLS define in the pipeline handler. Hopefully this will stop users from having build breakages when building upstream libcamera instead of this fork.

Unfortunately, I don't think the mechanics exist to have rpi::draft::*** level vendor controls. I will have a look, but this is likely not an option.

[naushir]

Are there any further questions regarding this topic? If not, feel free to close this.

[christianrauch]

I would still like to keep this open until the rpi::ScalerCrops situation has been resolved.

[naushir]

Resolved as in merged upstream?

[christianrauch]

Yes. Is this ok for you?

[naushir]

Sure.