[libcamera-devel] [libcamera-internal] [PATCH] android/mm/cros_camera_buffer: Mark cameraBuffer unused parameter

[Umang Jain]

Hi Hiro

On 8/5/21 2:01 PM, Hirokazu Honda wrote:
> Hi Uman,
>
> On Thu, Aug 5, 2021 at 5:16 PM Umang Jain <umang.jain at ideasonboard.com> wrote:
>> Hi Hiro,
>>
>> Apologies for the unnecessary churn, it seems we are not catching
>> -Wunused-parameter somehow
>>
>> I had to:
>>
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -43,6 +43,7 @@ endif
>>
>>    common_arguments = [
>>        '-Wshadow',
>> +    '-Wunused-parameter',
>>        '-include', 'config.h',
>>    ]
>>
>> to reproduce. So:
>>
> Oh, I thought we had it as we have been using [[maybe_unused]].
Same here. I too assumed that (hence the initial resistance early on)
> Thanks for finding it. Would you mind uploading patch series of adding
> the parameter?

I re-worded your patch as well and posted on the list :)

Thanks for noticing.

>
> -Hiro
>> Reviewed-by: Umang Jain <umang.jain at ideasonboard.com>
>>
>> On 8/5/21 1:31 PM, Hirokazu Honda wrote:
>>> Hi Umang,
>>>
>>> On Thu, Aug 5, 2021 at 4:59 PM Umang Jain <umang.jain at ideasonboard.com> wrote:
>>>> Hi Hiro,
>>>>
>>>> On 8/5/21 12:31 PM, Hirokazu Honda wrote:
>>>>> Sorry, I sent the wrong mailing list. Resend.
>>>>>
>>>>> On Wed, Aug 4, 2021 at 1:48 PM Hirokazu Honda <hiroh at chromium.org> wrote:
>>>>>> Hi Umang,
>>>>>>
>>>>>> Yes, I am building inside CrOS chroot. It makes me compiling
>>>>>> cros_camera_buffer.cpp, not generic_camera_buffer.cpp.
>>>> yes, cros_camera_buffer.cpp is what's supposed to get compiled given
>>>> what we have in the ebuild.
>>>>>> The compilation failure is caused by Wunused-parameter.
>>>>>> I expect the warning flag is also used in a standalone build too.
>>>>>> I attached the build log.
>>>>>>
>>>>>> -Hiro
>>>>>>
>>>>>> On Wed, Aug 4, 2021 at 1:25 PM Umang Jain <umang.jain at ideasonboard.com> wrote:
>>>>>>> Hi Hiro,
>>>>>>>
>>>>>>> On 8/4/21 9:09 AM, Hirokazu Honda wrote:
>>>>>>>> This fixes the compilation error introduced by the commit
>>>>>>>> 33dd4fab9d39726be4fcbd300a27f2640be1cd6f.
>>>>>>> Would you mind sharing your build log? Are you building inside CrOS
>>>>>>> chroot or standalone?
>>>>>>>
>>>>>>> I haven't seen any compilation failures recently, especially on master
>>>>>>> commits.
>>>>>>>
>>>>>>> Thanks!
>>>>>>>
>>>>>>>> Signed-off-by: Hirokazu Honda <hiroh at chromium.org>
>>>>>>>> ---
>>>>>>>>      src/android/mm/cros_camera_buffer.cpp | 2 +-
>>>>>>>>      1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>>>
>>>>>>>> diff --git a/src/android/mm/cros_camera_buffer.cpp b/src/android/mm/cros_camera_buffer.cpp
>>>>>>>> index 437502fb..e8783ff8 100644
>>>>>>>> --- a/src/android/mm/cros_camera_buffer.cpp
>>>>>>>> +++ b/src/android/mm/cros_camera_buffer.cpp
>>>>>>>> @@ -44,7 +44,7 @@ private:
>>>>>>>>          } mem;
>>>>>>>>      };
>>>>>>>>
>>>>>>>> -CameraBuffer::Private::Private(CameraBuffer *cameraBuffer,
>>>>>>>> +CameraBuffer::Private::Private([[maybe_unused]] CameraBuffer *cameraBuffer,
>>>> I suspect you have a dirty commit lingering around? What I see on master
>>>> is that the cameraBuffer is indeed used, which is why [[maybe_unused]]
>>>> is absent in the first place.
>>>>
>>>> https://chromium.googlesource.com/chromiumos/third_party/libcamera/+/refs/heads/master/src/android/mm/cros_camera_buffer.cpp#55
>>>>
>>> It is camera3Buffer, not cameraBuffer. ;)
>>>
>>>>>>>>                                 buffer_handle_t camera3Buffer,
>>>>>>>>                                 [[maybe_unused]] int flags)
>>>>>>>>          : handle_(camera3Buffer), numPlanes_(0), valid_(false),