[libcamera-devel] [PATCH V2] libcamera: MediaDevice: Make MediaDevice::setupLink account for existing link flags

[Kieran Bingham]

+libcamera-devel

On 27/08/2020 12:11, Dan Scally wrote:
> Hi Kieran
> 
>> Git commit messages should aim to be wrapped at 72 chars, though it can
>> be longer when it makes sense. (the above goes to 96).
>>
> 
>> We can rewrap when applying though.
> 
> Mea culpa - I'll stick to the limit in future. Happy to do a V3 if you prefer, fixing that and the SoB.

No worries, I think it's ok.

Unless there's any comments from anyone else I have it fixed up locally
already.


> 
>> This is missing a Signed-off-by tag from you.
>>
> 
>> Can you confirm you are happy to add the following please?
>> (as per the DCO: https://www.libcamera.org/contributing.html)
>>
> 
>> Signed-off-by: Dan Scally djrscally at protonmail.com
> 
> Yes, absolutely. Sorry - first time patching anything, knew I'd forget something!

no problem, Thank you for contributing!

I'm still interested to know how you hit this?
What platform are you testing on ?

--
Kieran


> 
> Thanks
> Dan
> 
> ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
> On Thursday, 27 August 2020 11:42, Kieran Bingham <kieran.bingham at ideasonboard.com> wrote:
> 
>> Hi Dan,
>>
> 
>> On 26/08/2020 17:05, Dan Scally wrote:
>>
> 
>>> The setupLink function fails (ioctl returns EINVAL) when it passes the MEDIA_LNK_FL_ENABLE flag
>>> to a link that is already flagged with MEDIA_LNK_FL_ENABLE and MEDIA_LNK_FL_IMMUTABLE. Contrast
>>> to media-ctl's equivalent media_setup_link() which ORs the new flags with the existing values.
>>> This patch modifies the behaviour of setupLink() to behave the same as media_setup_link() in
>>> media-ctl.
>>
> 
>> Git commit messages should aim to be wrapped at 72 chars, though it can
>> be longer when it makes sense. (the above goes to 96).
>>
> 
>> We can rewrap when applying though.
>>
> 
>>> ref https://git.linuxtv.org/v4l-utils.git/tree/utils/media-ctl/libmediactl.c#n210
>>
> 
>> This is missing a Signed-off-by tag from you.
>>
> 
>> Can you confirm you are happy to add the following please?
>> (as per the DCO: https://www.libcamera.org/contributing.html)
>>
> 
>> Signed-off-by: Dan Scally djrscally at protonmail.com
>>
> 
>> With that,
>>
> 
>> Reviewed-by: Kieran Bingham kieran.bingham at ideasonboard.com
>>
> 
>> Eeep, git-am had issue applying this mail, possibly due to utf
>> formatting or such, but I've fixed up locally.
>>
> 
>> With confirmation from Dan on the SoB, and another RB tag, I'll handle
>> integration of this patch as I have it locally.
>>
> 
>> ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>>
> 
>> Kieran
>>
> 
>>> Changelog:
>>>
> 
>>>     v2 - Simplified by removing the call to link() to fetch a link that's already passed as a parameter to the function.
>>>     
> 
>>>
> 
>>> src/libcamera/media_device.cpp | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>> diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp
>>> index de18d57..007a45b 100644
>>> --- a/src/libcamera/media_device.cpp
>>> +++ b/src/libcamera/media_device.cpp
>>> @@ -794,7 +794,7 @@ int MediaDevice::setupLink(const MediaLink *link, unsigned int flags)
>>> linkDesc.sink.index = sink->index();
>>> linkDesc.sink.flags = MEDIA_PAD_FL_SINK;
>>>
> 
>>> -   linkDesc.flags = flags;
>>>
> 
>>> -   linkDesc.flags = flags | (link->flags() & MEDIA_LNK_FL_IMMUTABLE);
>>>     int ret = ioctl(fd_, MEDIA_IOC_SETUP_LINK, &linkDesc);
>>>     if (ret) {
>>>     
> 
>>>
> 
>>> libcamera-devel mailing list
>>> libcamera-devel at lists.libcamera.org
>>> https://lists.libcamera.org/listinfo/libcamera-devel
>>
> 
>> --
>>
> 
>> Regards
>>
> 
>> --------
>>
> 
>> Kieran
> 

-- 
Regards
--
Kieran