From: Hans Verkuil <hverkuil-cisco@xs4all.nl>
To: dikshita@codeaurora.org
Cc: linux-media@vger.kernel.org, nicolas@ndufresne.ca,
stanimir.varbanov@linaro.org, linux-kernel@vger.kernel.org,
linux-arm-msm@vger.kernel.org, vgarodia@codeaurora.org
Subject: Re: [PATCH v4] media: v4l2-ctrl: add control for long term reference.
Date: Tue, 8 Dec 2020 17:45:37 +0100 [thread overview]
Message-ID: <6c6f56ce-750b-0706-1ff0-46adefcfe542@xs4all.nl> (raw)
In-Reply-To: <6b044c9be4a99adc18dd76765b089760@codeaurora.org>
On 08/12/2020 17:44, dikshita@codeaurora.org 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@codeaurora.org>
>>> ---
>>> .../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@xs4all.nl/
>>
>> 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@codeaurora.org/
>> https://patchwork.linuxtv.org/project/linux-media/patch/1605682497-29273-2-git-send-email-dikshita@codeaurora.org/
>> https://patchwork.linuxtv.org/project/linux-media/patch/1605682497-29273-3-git-send-email-dikshita@codeaurora.org/
>>
>> 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
prev parent reply other threads:[~2020-12-08 16:46 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-01 8:13 [PATCH v4] media: v4l2-ctrl: add control for long term reference Dikshita Agarwal
2020-12-02 13:48 ` Hans Verkuil
2020-12-08 16:44 ` dikshita
2020-12-08 16:45 ` Hans Verkuil [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=6c6f56ce-750b-0706-1ff0-46adefcfe542@xs4all.nl \
--to=hverkuil-cisco@xs4all.nl \
--cc=dikshita@codeaurora.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=nicolas@ndufresne.ca \
--cc=stanimir.varbanov@linaro.org \
--cc=vgarodia@codeaurora.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).