[PATCH] Documentation: media: improve V4L2_CID_MIN_BUFFERS_FOR_*, doc

[PATCH] Documentation: media: improve V4L2_CID_MIN_BUFFERS_FOR_*, doc

[Hans Verkuil]

Clearly state that the V4L2_CID_MIN_BUFFERS_FOR_OUTPUT and
V4L2_CID_MIN_BUFFERS_FOR_CAPTURE controls are required for
stateful codecs.

Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
---
 Documentation/userspace-api/media/v4l/control.rst | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/Documentation/userspace-api/media/v4l/control.rst b/Documentation/userspace-api/media/v4l/control.rst
index 57893814a1e5..9253cc946f02 100644
--- a/Documentation/userspace-api/media/v4l/control.rst
+++ b/Documentation/userspace-api/media/v4l/control.rst
@@ -290,13 +290,15 @@ Control IDs
     This is a read-only control that can be read by the application and
     used as a hint to determine the number of CAPTURE buffers to pass to
     REQBUFS. The value is the minimum number of CAPTURE buffers that is
-    necessary for hardware to work.
+    necessary for hardware to work. This control is required for stateful
+    decoders.

 ``V4L2_CID_MIN_BUFFERS_FOR_OUTPUT`` ``(integer)``
     This is a read-only control that can be read by the application and
     used as a hint to determine the number of OUTPUT buffers to pass to
     REQBUFS. The value is the minimum number of OUTPUT buffers that is
-    necessary for hardware to work.
+    necessary for hardware to work. This control is required for stateful
+    encoders.

 .. _v4l2-alpha-component:

-- 
2.45.2

Re: [PATCH] Documentation: media: improve V4L2_CID_MIN_BUFFERS_FOR_*, doc

[Laurent Pinchart]

Hi Hans,

Thank you for the patch.

On Thu, Oct 31, 2024 at 08:50:04AM +0100, Hans Verkuil wrote:
> Clearly state that the V4L2_CID_MIN_BUFFERS_FOR_OUTPUT and
> V4L2_CID_MIN_BUFFERS_FOR_CAPTURE controls are required for
> stateful codecs.

Wouldn't it be better for this kind of information to be centralized in
a stateful decoder document ? That would make it easier for developers
to see all they need to implement. Otherwise they would need to read
through the whole documentation to pick the parts of the API they need
to support in their drivers.

> Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
> ---
>  Documentation/userspace-api/media/v4l/control.rst | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/userspace-api/media/v4l/control.rst b/Documentation/userspace-api/media/v4l/control.rst
> index 57893814a1e5..9253cc946f02 100644
> --- a/Documentation/userspace-api/media/v4l/control.rst
> +++ b/Documentation/userspace-api/media/v4l/control.rst
> @@ -290,13 +290,15 @@ Control IDs
>      This is a read-only control that can be read by the application and
>      used as a hint to determine the number of CAPTURE buffers to pass to
>      REQBUFS. The value is the minimum number of CAPTURE buffers that is
> -    necessary for hardware to work.
> +    necessary for hardware to work. This control is required for stateful
> +    decoders.
> 
>  ``V4L2_CID_MIN_BUFFERS_FOR_OUTPUT`` ``(integer)``
>      This is a read-only control that can be read by the application and
>      used as a hint to determine the number of OUTPUT buffers to pass to
>      REQBUFS. The value is the minimum number of OUTPUT buffers that is
> -    necessary for hardware to work.
> +    necessary for hardware to work. This control is required for stateful
> +    encoders.
> 
>  .. _v4l2-alpha-component:
> 

-- 
Regards,

Laurent Pinchart

Re: [PATCH] Documentation: media: improve V4L2_CID_MIN_BUFFERS_FOR_*, doc

[Hans Verkuil]

On 10/31/24 11:11, Laurent Pinchart wrote:
> Hi Hans,
> 
> Thank you for the patch.
> 
> On Thu, Oct 31, 2024 at 08:50:04AM +0100, Hans Verkuil wrote:
>> Clearly state that the V4L2_CID_MIN_BUFFERS_FOR_OUTPUT and
>> V4L2_CID_MIN_BUFFERS_FOR_CAPTURE controls are required for
>> stateful codecs.
> 
> Wouldn't it be better for this kind of information to be centralized in
> a stateful decoder document ? That would make it easier for developers
> to see all they need to implement. Otherwise they would need to read
> through the whole documentation to pick the parts of the API they need
> to support in their drivers.

It's also already mentioned in the documentation for the stateful de/encoders here:

https://linuxtv.org/downloads/v4l-dvb-apis-new/userspace-api/v4l/dev-mem2mem.html

Also, once this vicodec patch is merged:

https://patchwork.linuxtv.org/project/linux-media/patch/1dd09050-40ca-4c5b-b985-819731140388@xs4all.nl/

I plan to push v4l2-compliance patches that explicitly test for the presence of
these controls and fail if they are missing (like they are now in vicodec).

Regards,

	Hans

> 
>> Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
>> ---
>>  Documentation/userspace-api/media/v4l/control.rst | 6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/userspace-api/media/v4l/control.rst b/Documentation/userspace-api/media/v4l/control.rst
>> index 57893814a1e5..9253cc946f02 100644
>> --- a/Documentation/userspace-api/media/v4l/control.rst
>> +++ b/Documentation/userspace-api/media/v4l/control.rst
>> @@ -290,13 +290,15 @@ Control IDs
>>      This is a read-only control that can be read by the application and
>>      used as a hint to determine the number of CAPTURE buffers to pass to
>>      REQBUFS. The value is the minimum number of CAPTURE buffers that is
>> -    necessary for hardware to work.
>> +    necessary for hardware to work. This control is required for stateful
>> +    decoders.
>>
>>  ``V4L2_CID_MIN_BUFFERS_FOR_OUTPUT`` ``(integer)``
>>      This is a read-only control that can be read by the application and
>>      used as a hint to determine the number of OUTPUT buffers to pass to
>>      REQBUFS. The value is the minimum number of OUTPUT buffers that is
>> -    necessary for hardware to work.
>> +    necessary for hardware to work. This control is required for stateful
>> +    encoders.
>>
>>  .. _v4l2-alpha-component:
>>
> 

Re: [PATCH] Documentation: media: improve V4L2_CID_MIN_BUFFERS_FOR_*, doc

[Laurent Pinchart]

On Thu, Oct 31, 2024 at 11:26:47AM +0100, Hans Verkuil wrote:
> On 10/31/24 11:11, Laurent Pinchart wrote:
> > Hi Hans,
> > 
> > Thank you for the patch.
> > 
> > On Thu, Oct 31, 2024 at 08:50:04AM +0100, Hans Verkuil wrote:
> >> Clearly state that the V4L2_CID_MIN_BUFFERS_FOR_OUTPUT and
> >> V4L2_CID_MIN_BUFFERS_FOR_CAPTURE controls are required for
> >> stateful codecs.
> > 
> > Wouldn't it be better for this kind of information to be centralized in
> > a stateful decoder document ? That would make it easier for developers
> > to see all they need to implement. Otherwise they would need to read
> > through the whole documentation to pick the parts of the API they need
> > to support in their drivers.
> 
> It's also already mentioned in the documentation for the stateful de/encoders here:
> 
> https://linuxtv.org/downloads/v4l-dvb-apis-new/userspace-api/v4l/dev-mem2mem.html

OK, then it's fine to have it here too I think.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> Also, once this vicodec patch is merged:
> 
> https://patchwork.linuxtv.org/project/linux-media/patch/1dd09050-40ca-4c5b-b985-819731140388@xs4all.nl/
> 
> I plan to push v4l2-compliance patches that explicitly test for the presence of
> these controls and fail if they are missing (like they are now in vicodec).
>
> >> Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
> >> ---
> >>  Documentation/userspace-api/media/v4l/control.rst | 6 ++++--
> >>  1 file changed, 4 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/Documentation/userspace-api/media/v4l/control.rst b/Documentation/userspace-api/media/v4l/control.rst
> >> index 57893814a1e5..9253cc946f02 100644
> >> --- a/Documentation/userspace-api/media/v4l/control.rst
> >> +++ b/Documentation/userspace-api/media/v4l/control.rst
> >> @@ -290,13 +290,15 @@ Control IDs
> >>      This is a read-only control that can be read by the application and
> >>      used as a hint to determine the number of CAPTURE buffers to pass to
> >>      REQBUFS. The value is the minimum number of CAPTURE buffers that is
> >> -    necessary for hardware to work.
> >> +    necessary for hardware to work. This control is required for stateful
> >> +    decoders.
> >>
> >>  ``V4L2_CID_MIN_BUFFERS_FOR_OUTPUT`` ``(integer)``
> >>      This is a read-only control that can be read by the application and
> >>      used as a hint to determine the number of OUTPUT buffers to pass to
> >>      REQBUFS. The value is the minimum number of OUTPUT buffers that is
> >> -    necessary for hardware to work.
> >> +    necessary for hardware to work. This control is required for stateful
> >> +    encoders.
> >>
> >>  .. _v4l2-alpha-component:
> >>

-- 
Regards,

Laurent Pinchart

Re: [PATCH] Documentation: media: improve V4L2_CID_MIN_BUFFERS_FOR_*, doc

[Nicolas Dufresne]

Le jeudi 31 octobre 2024 à 11:26 +0100, Hans Verkuil a écrit :
> On 10/31/24 11:11, Laurent Pinchart wrote:
> > Hi Hans,
> > 
> > Thank you for the patch.
> > 
> > On Thu, Oct 31, 2024 at 08:50:04AM +0100, Hans Verkuil wrote:
> > > Clearly state that the V4L2_CID_MIN_BUFFERS_FOR_OUTPUT and
> > > V4L2_CID_MIN_BUFFERS_FOR_CAPTURE controls are required for
> > > stateful codecs.
> > 
> > Wouldn't it be better for this kind of information to be centralized in
> > a stateful decoder document ? That would make it easier for developers
> > to see all they need to implement. Otherwise they would need to read
> > through the whole documentation to pick the parts of the API they need
> > to support in their drivers.
> 
> It's also already mentioned in the documentation for the stateful de/encoders here:
> 
> https://linuxtv.org/downloads/v4l-dvb-apis-new/userspace-api/v4l/dev-mem2mem.html
> 
> Also, once this vicodec patch is merged:
> 
> https://patchwork.linuxtv.org/project/linux-media/patch/1dd09050-40ca-4c5b-b985-819731140388@xs4all.nl/
> 
> I plan to push v4l2-compliance patches that explicitly test for the presence of
> these controls and fail if they are missing (like they are now in vicodec).

Thanks, this is a great idea.

Nicolas

> 
> Regards,
> 
> 	Hans
> 
> > 
> > > Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
> > > ---
> > >  Documentation/userspace-api/media/v4l/control.rst | 6 ++++--
> > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/Documentation/userspace-api/media/v4l/control.rst b/Documentation/userspace-api/media/v4l/control.rst
> > > index 57893814a1e5..9253cc946f02 100644
> > > --- a/Documentation/userspace-api/media/v4l/control.rst
> > > +++ b/Documentation/userspace-api/media/v4l/control.rst
> > > @@ -290,13 +290,15 @@ Control IDs
> > >      This is a read-only control that can be read by the application and
> > >      used as a hint to determine the number of CAPTURE buffers to pass to
> > >      REQBUFS. The value is the minimum number of CAPTURE buffers that is
> > > -    necessary for hardware to work.
> > > +    necessary for hardware to work. This control is required for stateful
> > > +    decoders.
> > > 
> > >  ``V4L2_CID_MIN_BUFFERS_FOR_OUTPUT`` ``(integer)``
> > >      This is a read-only control that can be read by the application and
> > >      used as a hint to determine the number of OUTPUT buffers to pass to
> > >      REQBUFS. The value is the minimum number of OUTPUT buffers that is
> > > -    necessary for hardware to work.
> > > +    necessary for hardware to work. This control is required for stateful
> > > +    encoders.
> > > 
> > >  .. _v4l2-alpha-component:
> > > 
> > 
>