[libcamera-devel] [PATCH v2 3/8] libcamera: v4l2_device: streamOff() when releasing buffers

[Kieran Bingham]

Hi Laurent,

On 13/02/2019 15:41, Laurent Pinchart wrote:
> Hi Kieran,
> 
> Thank you for the patch.
> 
> On Wed, Feb 13, 2019 at 03:10:22PM +0000, Kieran Bingham wrote:
>> We must ensure that the stream is disabled before releasing buffers. It will
>> not hurt to call streamOff() even if it is already off before releasing any
>> buffers back to the device.
> 
> I'm not sure about this one. Doesn't it indicate a problem in the caller
> instead ?

Perhaps. It means (in the cases that I've hit) that the error paths have
not called streamOff().

Is it more preferable to leave buffers 'un-released'?

Perhaps would you prefer to see releaseBuffers return an error if it
fails? As this is likely to be on cleanup paths, and destruction - what
would you expect the application to do in the event of failure to
releaseBuffers? (I guess - fix their code...)


Either way - I'll just drop this patch for now.

--
Kieran


>> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>> ---
>>  src/libcamera/v4l2_device.cpp | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
>> index 23c0da295905..83073c28b817 100644
>> --- a/src/libcamera/v4l2_device.cpp
>> +++ b/src/libcamera/v4l2_device.cpp
>> @@ -642,6 +642,8 @@ int V4L2Device::releaseBuffers()
>>  {
>>  	LOG(V4L2, Debug) << "Releasing bufferPool";
>>  
>> +	streamOff();
>> +
>>  	requestBuffers(0);
>>  	bufferPool_ = nullptr;
>>  
> 

-- 
Regards
--
Kieran