| Message ID | 93d078e5-deba-4060-a32e-94bce677453c@xs4all.nl (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Documentation: media: improve V4L2_CID_MIN_BUFFERS_FOR_*, doc | expand |
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: >
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: >> >
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: > >>
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: > > > > > >
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:
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(-)