On 05/04/2022 22:44, Nicolas Dufresne wrote: > These two fields need 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 developers. 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@xxxxxxxxxxxxx> > Reviewed-by: Sebastian Fricke <sebastian.fricke@xxxxxxxxxxxxx> > --- > .../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..49f89b702068 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 references, this should match the derived value PicNum shouldn't 'should' be 'must'? Same elsewhere below. > + (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. I think this last sentence is a bit confusing. How about: "When decoding frames (as opposed to fields) pic_num is the same as FrameNumWrap." > * - __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 neeeded -> needed > + long term references, this should be set to the value of > + long_term_frame_idx described in the dec_ref_pic_marking() syntax. > * - __u8 > - ``fields`` > - Specifies how the DPB entry is referenced. See :ref:`Reference Fields <h264_ref_fields>` Regards, Hans
