| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <dd9cd9dd-3947-4ea9-bdbc-0c655238af46@oss.qualcomm.com>
Date: Mon, 26 Jan 2026 16:12:01 +0800
From: Wangao Wang <wangao.wang@....qualcomm.com>
To: Dikshita Agarwal <dikshita.agarwal@....qualcomm.com>,
Vikash Garodia <vikash.garodia@....qualcomm.com>,
Abhinav Kumar <abhinav.kumar@...ux.dev>,
Bryan O'Donoghue <bod@...nel.org>,
Mauro Carvalho Chehab <mchehab@...nel.org>
Cc: wangao.wang@....qualcomm.com, quic_qiweil@...cinc.com,
Renjiang Han <renjiang.han@....qualcomm.com>,
linux-media@...r.kernel.org, linux-arm-msm@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 4/6] media: qcom: iris: Add hierarchical coding support
for encoder
On 2026/1/22 17:38, Dikshita Agarwal wrote:
>> @@ -116,6 +116,40 @@ static enum platform_inst_fw_cap_type iris_get_cap_id(u32 id)
>> return MARK_LTR;
>> case V4L2_CID_MPEG_VIDEO_B_FRAMES:
>> return B_FRAME;
>> + case V4L2_CID_MPEG_VIDEO_H264_HIERARCHICAL_CODING:
>> + return LAYER_ENABLE;
>
> Will the same control be used for HEVC as well? I think this is applicable
> for only H264 encoders.
>
H264 flow:
V4L2_CID_MPEG_VIDEO_H264_HIERARCHICAL_CODING,
V4L2_CID_MPEG_VIDEO_H264_HIERARCHICAL_CODING_TYPE,
V4L2_CID_MPEG_VIDEO_H264_HIERARCHICAL_CODING_LAYER
HEVC flow:
V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_TYPE,
V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_LAYER
LAYER_ENABLE is used for H.264. In the HEVC flow, this flag is
considered redundant—once the type is set, layer encoding is implicitly
enabled.
>> +int iris_set_bitrate_gen1(struct iris_inst *inst, enum platform_inst_fw_cap_type cap_id)
>> +{
>> + const struct iris_hfi_command_ops *hfi_ops = inst->core->hfi_ops;
>> + u32 entropy_mode = inst->fw_caps[ENTROPY_MODE].value;
>> + u32 bitrate = inst->fw_caps[cap_id].value;
>> + u32 hfi_id = inst->fw_caps[cap_id].hfi_id;
>> + struct hfi_bitrate hfi_val;
>> + u32 max_bitrate;
>> +
>> + if (!(inst->fw_caps[cap_id].flags & CAP_FLAG_CLIENT_SET) && cap_id != BITRATE)
>> + return -EINVAL;
>
> Can you pls explain what is this check for?
>
> The layer bitrate should only be set if layer encoding is enabled, isn't it?
>
This flag is used to confirm whether the corresponding ctrl has been
invoked. A check should also be added to determine whether layer
encoding is enabled, while excluding bitrate configuration in
non–layer‑encoding scenarios.
>> + if (inst->codec == V4L2_PIX_FMT_H264) {
>> + if (!layer_enable || !inst->fw_caps[LAYER_COUNT_H264].value)
>> + return -EINVAL;
>> +
>> + if (inst->fw_caps[LAYER_TYPE_H264].value ==
>> + V4L2_MPEG_VIDEO_H264_HIERARCHICAL_CODING_P) {
>> + if (inst->hfi_rc_type == HFI_RC_VBR_CFR)
>> + layer_type = HFI_HIER_P_HYBRID_LTR;
>> + else
>> + layer_type = HFI_HIER_P_SLIDING_WINDOW;
>> + } else if (inst->fw_caps[LAYER_TYPE_HEVC].value ==
>> + V4L2_MPEG_VIDEO_HEVC_HIERARCHICAL_CODING_B) {
>
> why are you checking HEVC layer type for H264 codec? seems like a bug.
>
This is a bug and will be fixed in v4.
>> +int iris_set_layer_count_gen1(struct iris_inst *inst, enum platform_inst_fw_cap_type cap_id)
>> +{
>> + const struct iris_hfi_command_ops *hfi_ops = inst->core->hfi_ops;
>> + struct vb2_queue *sq = v4l2_m2m_get_src_vq(inst->m2m_ctx);
>> + struct vb2_queue *dq = v4l2_m2m_get_dst_vq(inst->m2m_ctx);
>> + u32 layer_enable = inst->fw_caps[LAYER_ENABLE].value;
>> + u32 layer_count = inst->fw_caps[cap_id].value;
>> + u32 hfi_id, ret;
>> +
>> + if (!layer_enable || !layer_count)
>> + return -EINVAL;
>> +
>> + inst->hfi_layer_count = layer_count;
>> +
>> + if (!vb2_is_streaming(sq) && !vb2_is_streaming(dq)) {
>> + hfi_id = HFI_PROPERTY_PARAM_VENC_HIER_P_MAX_NUM_ENH_LAYER;
>
> why the streaming check? and what's the significance of this setting? why
> this prop is set under streaming check?
>
This property needs to be set to the firmware before streaming. It
represents the maximum layer count and is static; any dynamically
configured layer count later must not exceed this maximum.
>> +
>> +int iris_set_layer_bitrate(struct iris_inst *inst, enum platform_inst_fw_cap_type cap_id)
>> +{
>> + const struct iris_hfi_command_ops *hfi_ops = inst->core->hfi_ops;
>> + u32 hfi_id = inst->fw_caps[cap_id].hfi_id;
>> + u32 bitrate = inst->fw_caps[cap_id].value;
>> +
>> + /* ignore layer bitrate when total bitrate is set */
>> + if (inst->fw_caps[BITRATE].flags & CAP_FLAG_CLIENT_SET)
>> + return 0;
>> +
>
> any streaming check required here?
>
A streaming check will be added here.
>> + {
>> + .cap_id = LAYER_TYPE_HEVC,
>> + .min = V4L2_MPEG_VIDEO_HEVC_HIERARCHICAL_CODING_B,
>> + .max = V4L2_MPEG_VIDEO_HEVC_HIERARCHICAL_CODING_P,
>> + .step_or_mask = BIT(V4L2_MPEG_VIDEO_HEVC_HIERARCHICAL_CODING_B) |
>> + BIT(V4L2_MPEG_VIDEO_HEVC_HIERARCHICAL_CODING_P),
>> + .value = V4L2_MPEG_VIDEO_HEVC_HIERARCHICAL_CODING_P,
>> + .hfi_id = HFI_PROP_LAYER_ENCODING_TYPE,
>> + .flags = CAP_FLAG_OUTPUT_PORT | CAP_FLAG_MENU,
>> + .set = iris_set_layer_type,
>
> layer type depends on layer count so shouldn't you have layer count before
> layer type in caps? or handle both in same set API?
>
In the iris_set_layer_type API, there is a check for the layer count. If
the count is 0, layer type will not be set to the firmware.
>> + /*
>> + * The expression (1 << layers - 2) + 1 accounts for the number of reference
>> + * frames in the Adaptive Hierarchical B-frame encoding case. In this scheme,
>> + * the number of frames in a sub-GOP is related to (2^(number of layers) - 1),
>> + * hence the use of the shift operation.
>> + */
>> + if (layer_type == HFI_HIER_B) {
>> + if (inst->codec == V4L2_PIX_FMT_HEVC)
>> + num_ref = layer_count;
>> + else
>> + num_ref = (1 << (layer_count - 2)) + 1;
>> + }
>
> were you able to test these different scenarios?
>
Okay, I will test the other scenarios.
--
Best Regards,
Wangao
Powered by blists - more mailing lists