On 08/12/2020 17:44, dikshita@xxxxxxxxxxxxxx wrote:
> Hi Hans,
>
> On 2020-12-02 19:18, Hans Verkuil wrote:
>> On 01/12/2020 09:13, Dikshita Agarwal wrote:
>>> Long Term Reference (LTR) frames are the frames that are encoded
>>> sometime in the past and stored in the DPB buffer list to be used
>>> as reference to encode future frames.
>>> This change adds controls to enable this feature.
>>>
>>> Signed-off-by: Dikshita Agarwal <dikshita@xxxxxxxxxxxxxx>
>>> ---
>>> .../userspace-api/media/v4l/ext-ctrls-codec.rst | 18
>>> ++++++++++++++++++
>>> drivers/media/v4l2-core/v4l2-ctrls.c | 14
>>> ++++++++++++++
>>> include/uapi/linux/v4l2-controls.h | 3 +++
>>> 3 files changed, 35 insertions(+)
>>>
>>> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>>> b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>>> index 3b86959..40634f8 100644
>>> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>>> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
>>> @@ -4450,3 +4450,21 @@ enum v4l2_mpeg_video_hevc_size_of_length_field
>>> -
>>> - Selecting this value specifies that HEVC slices are expected
>>> to be prefixed by Annex B start codes. According to
>>> :ref:`hevc`
>>> valid start codes can be 3-bytes 0x000001 or 4-bytes
>>> 0x00000001.
>>> +
>>> +``V4L2_CID_MPEG_VIDEO_LTR_COUNT (integer)``
>>> + Specifies the number of Long Term Reference (LTR) frames
>>> encoder needs
>>> + to generate or keep. This is applicable to H264 and HEVC
>>> encoder.
>>> +
>>> +``V4L2_CID_MPEG_VIDEO_FRAME_LTR_INDEX (integer)``
>>> + The current frame is marked as a Long Term Reference (LTR)
>>> frame
>>> + and given this LTR index which ranges from 0 to LTR_COUNT-1.
>>> + This is applicable to H264 and HEVC encoder and can be applied
>>> using
>>> + Request Api.
>>> + Source Rec. ITU-T H.264 (06/2019); Table 7.9
>>> +
>>> +``V4L2_CID_MPEG_VIDEO_USE_LTR_FRAMES (bitmask)``
>>> + Specifies the Long Term Reference (LTR) frame(s) to be used
>>> for
>>> + encoding the current frame.
>>> + This provides a bitmask which consists of bits [0,
>>> LTR_COUNT-1].
>>> + This is applicable to H264 and HEVC encoder and can be applied
>>> using
>>> + Request Api.
>>> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c
>>> b/drivers/media/v4l2-core/v4l2-ctrls.c
>>> index e5b726f..0b81b39 100644
>>> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
>>> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
>>> @@ -958,6 +958,9 @@ const char *v4l2_ctrl_get_name(u32 id)
>>> case V4L2_CID_MPEG_VIDEO_MV_V_SEARCH_RANGE: return "Vertical MV
>>> Search Range";
>>> case V4L2_CID_MPEG_VIDEO_REPEAT_SEQ_HEADER: return "Repeat Sequence
>>> Header";
>>> case V4L2_CID_MPEG_VIDEO_FORCE_KEY_FRAME: return "Force Key Frame";
>>> + case V4L2_CID_MPEG_VIDEO_LTR_COUNT: return "LTR Count";
>>> + case V4L2_CID_MPEG_VIDEO_FRAME_LTR_INDEX: return "frame LTR index";
>>
>> "Frame LTR Index"
>>
>>> + case V4L2_CID_MPEG_VIDEO_USE_LTR_FRAMES: return "Use LTR Frame(s)";
>>
>> Just: "Use LTR Frames". No '(s)'.
>>
>>> case V4L2_CID_MPEG_VIDEO_MPEG2_SLICE_PARAMS: return "MPEG-2 Slice
>>> Parameters";
>>> case V4L2_CID_MPEG_VIDEO_MPEG2_QUANTIZATION: return "MPEG-2
>>> Quantization Matrices";
>>> case V4L2_CID_MPEG_VIDEO_FWHT_PARAMS: return "FWHT Stateless
>>> Parameters";
>>> @@ -1273,6 +1276,17 @@ void v4l2_ctrl_fill(u32 id, const char **name,
>>> enum v4l2_ctrl_type *type,
>>> case V4L2_CID_MPEG_VIDEO_MV_V_SEARCH_RANGE:
>>> *type = V4L2_CTRL_TYPE_INTEGER;
>>> break;
>>> + case V4L2_CID_MPEG_VIDEO_LTR_COUNT:
>>> + *type = V4L2_CTRL_TYPE_INTEGER;
>>> + break;
>>> + case V4L2_CID_MPEG_VIDEO_FRAME_LTR_INDEX:
>>> + *type = V4L2_CTRL_TYPE_INTEGER;
>>> + *flags |= V4L2_CTRL_FLAG_EXECUTE_ON_WRITE;
>>> + break;
>>> + case V4L2_CID_MPEG_VIDEO_USE_LTR_FRAMES:
>>> + *type = V4L2_CTRL_TYPE_BITMASK;
>>> + *flags |= V4L2_CTRL_FLAG_EXECUTE_ON_WRITE;
>>> + break;
>>> case V4L2_CID_MPEG_VIDEO_FORCE_KEY_FRAME:
>>> case V4L2_CID_PAN_RESET:
>>> case V4L2_CID_TILT_RESET:
>>> diff --git a/include/uapi/linux/v4l2-controls.h
>>> b/include/uapi/linux/v4l2-controls.h
>>> index e37b85f..710bc53 100644
>>> --- a/include/uapi/linux/v4l2-controls.h
>>> +++ b/include/uapi/linux/v4l2-controls.h
>>> @@ -421,6 +421,9 @@ enum v4l2_mpeg_video_multi_slice_mode {
>>> #define
>>> V4L2_CID_MPEG_VIDEO_MV_H_SEARCH_RANGE (V4L2_CID_MPEG_BASE+227)
>>> #define
>>> V4L2_CID_MPEG_VIDEO_MV_V_SEARCH_RANGE (V4L2_CID_MPEG_BASE+228)
>>> #define V4L2_CID_MPEG_VIDEO_FORCE_KEY_FRAME (V4L2_CID_MPEG_BASE+229)
>>> +#define V4L2_CID_MPEG_VIDEO_LTR_COUNT
>>> (V4L2_CID_MPEG_BASE + 230)
>>> +#define V4L2_CID_MPEG_VIDEO_FRAME_LTR_INDEX
>>> (V4L2_CID_MPEG_BASE + 231)
>>> +#define V4L2_CID_MPEG_VIDEO_USE_LTR_FRAMES
>>> (V4L2_CID_MPEG_BASE + 232)
>>>
>>> /* CIDs for the MPEG-2 Part 2 (H.262) codec */
>>> #define V4L2_CID_MPEG_VIDEO_MPEG2_LEVEL (V4L2_CID_MPEG_BASE+270)
>>>
>>
>> Note that this and the other patches from you that add
>> V4L2_CID_MPEG_VIDEO controls
>> will need to be rebased as soon as this PR is merged:
>>
>> https://patchwork.linuxtv.org/project/linux-media/patch/d68da172-b251-000f-653d-38a8a4c7b715@xxxxxxxxx/
>>
>> I recommend waiting until that's in.
>>
>> I currently have the following patches from you adding new controls
>> besides this one:
>>
>> https://patchwork.linuxtv.org/project/linux-media/patch/1606121442-31074-1-git-send-email-dikshita@xxxxxxxxxxxxxx/
>> https://patchwork.linuxtv.org/project/linux-media/patch/1605682497-29273-2-git-send-email-dikshita@xxxxxxxxxxxxxx/
>> https://patchwork.linuxtv.org/project/linux-media/patch/1605682497-29273-3-git-send-email-dikshita@xxxxxxxxxxxxxx/
>>
>> What is missing is driver support for these new controls. I recommend
>> that, once
>> the PR mentioned above is merged, you make a new series combining all
>> three
>> patches + patches that add support for this to the venus driver.
>>
> I will address all comments in the next version and will include driver
> implementation as well.
> Once all the driver changes are reviewed, I will make a series with all
> final patches, Hope that's fine.
That sounds great!
Regards,
Hans
>
> Thanks,
> Dikshita
>
>> Regards,
>>
>> Hans
