| Message ID | 20220328195936.82552-3-nicolas.dufresne@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v1,01/24] media: h264: Increase reference lists size to 32 | expand |
On 28.03.2022 15:59, Nicolas Dufresne wrote: >These two fields needs documentation as they have dual meaning. It is also s/needs/need/ >confusing since pic_num is a derived value from frame_num, so this should >help application developpers. If we ever need to make a V2 of this API, I s/developpers/developers/ (seems to be a common typo among native French speakers ;)) >would suggest to remove pic_num entirely. I think that suggestion should be placed as a FIXME/TODO comment into the source code as it is way easier to find for future developers creating a V2. > >Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com> Reviewed-by: Sebastian Fricke <sebastian.fricke@collabora.com> >--- > .../media/v4l/ext-ctrls-codec-stateless.rst | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > >diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec-stateless.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-codec-stateless.rst >index 6541e4c32b26..f634f20bcfbe 100644 >--- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec-stateless.rst >+++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec-stateless.rst >@@ -649,10 +649,16 @@ Stateless Codec Control ID > :c:type:`timeval` in struct :c:type:`v4l2_buffer` to a __u64. > * - __u32 > - ``pic_num`` >- - >+ - For short term reference, this should match the derived value PicNum >+ (8-28) and for long term references it should match the derived value Minor suggestion, for short term you use singular (reference) and for long term you use plural (references), I would stick to one of both. (Below you only use plural so maybe stick to plural here as well) >+ LongTermPicNum (8-29). Note that pic_num is the same as FrameNumWrap >+ for frame decoding. > * - __u16 > - ``frame_num`` >- - >+ - For short term references, this should match the frame_num value from >+ the slice header syntax (the driver will wrap the value if neeeded). For >+ long term references, this should be set to the value of >+ long_term_frame_idx describes in the dec_ref_pic_marking() syntax. s/describes/described/ Greetings, Sebastian > * - __u8 > - ``fields`` > - Specifies how the DPB entry is referenced. See :ref:`Reference Fields <h264_ref_fields>` >-- >2.34.1 >
diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec-stateless.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-codec-stateless.rst index 6541e4c32b26..f634f20bcfbe 100644 --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec-stateless.rst +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec-stateless.rst @@ -649,10 +649,16 @@ Stateless Codec Control ID :c:type:`timeval` in struct :c:type:`v4l2_buffer` to a __u64. * - __u32 - ``pic_num`` - - + - For short term reference, this should match the derived value PicNum + (8-28) and for long term references it should match the derived value + LongTermPicNum (8-29). Note that pic_num is the same as FrameNumWrap + for frame decoding. * - __u16 - ``frame_num`` - - + - For short term references, this should match the frame_num value from + the slice header syntax (the driver will wrap the value if neeeded). For + long term references, this should be set to the value of + long_term_frame_idx describes in the dec_ref_pic_marking() syntax. * - __u8 - ``fields`` - Specifies how the DPB entry is referenced. See :ref:`Reference Fields <h264_ref_fields>`
These two fields needs documentation as they have dual meaning. It is also confusing since pic_num is a derived value from frame_num, so this should help application developpers. If we ever need to make a V2 of this API, I would suggest to remove pic_num entirely. Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com> --- .../media/v4l/ext-ctrls-codec-stateless.rst | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)