From: Stanimir Varbanov <stanimir.varbanov@linaro.org>
To: mgottam@codeaurora.org, Stanimir Varbanov <stanimir.varbanov@linaro.org>
Cc: linux-media@vger.kernel.org,
Mauro Carvalho Chehab <mchehab@kernel.org>,
Hans Verkuil <hverkuil@xs4all.nl>,
linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org,
Vikash Garodia <vgarodia@codeaurora.org>,
Tomasz Figa <tfiga@chromium.org>,
Alexandre Courbot <acourbot@chromium.org>
Subject: Re: [PATCH 10/10] venus: dec: make decoder compliant with stateful codec API
Date: Thu, 24 Jan 2019 12:24:00 +0200 [thread overview]
Message-ID: <1663ea51-1fa0-6e29-715d-9d67aabb5ca9@linaro.org> (raw)
In-Reply-To: <d6aeca13e4f3da78fd00c69258e115d1@codeaurora.org>
Hi Malathi,
On 1/21/19 1:20 PM, mgottam@codeaurora.org wrote:
> On 2019-01-17 21:50, Stanimir Varbanov wrote:
>> This refactored code for start/stop streaming vb2 operations and
>> adds a state machine handling similar to the one in stateful codec
>> API documentation. One major change is that now the HFI session is
>> started on STREAMON(OUTPUT) and stopped on REQBUF(OUTPUT,count=0),
>> during that time streamoff(cap,out) just flush buffers but doesn't
>> stop the session. The other major change is that now the capture
>> and output queues are completely separated.
>>
>> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
>> ---
>> drivers/media/platform/qcom/venus/core.h | 20 +-
>> drivers/media/platform/qcom/venus/helpers.c | 23 +-
>> drivers/media/platform/qcom/venus/helpers.h | 5 +
>> drivers/media/platform/qcom/venus/vdec.c | 449 ++++++++++++++++----
>> 4 files changed, 389 insertions(+), 108 deletions(-)
>>
>> diff --git a/drivers/media/platform/qcom/venus/core.h
>> b/drivers/media/platform/qcom/venus/core.h
>> index 79c7e816c706..5a133c203455 100644
>> --- a/drivers/media/platform/qcom/venus/core.h
>> +++ b/drivers/media/platform/qcom/venus/core.h
>> @@ -218,6 +218,15 @@ struct venus_buffer {
>>
>> #define to_venus_buffer(ptr) container_of(ptr, struct
>> venus_buffer, vb)
>>
>> +#define DEC_STATE_UNINIT 0
>> +#define DEC_STATE_INIT 1
>> +#define DEC_STATE_CAPTURE_SETUP 2
>> +#define DEC_STATE_STOPPED 3
>> +#define DEC_STATE_SEEK 4
>> +#define DEC_STATE_DRAIN 5
>> +#define DEC_STATE_DECODING 6
>> +#define DEC_STATE_DRC 7
>> +
>> /**
>> * struct venus_inst - holds per instance paramerters
>> *
>> @@ -241,6 +250,10 @@ struct venus_buffer {
>> * @colorspace: current color space
>> * @quantization: current quantization
>> * @xfer_func: current xfer function
>> + * @codec_state: current codec API state (see DEC/ENC_STATE_)
>> + * @reconf_wait: wait queue for resolution change event
>> + * @ten_bits: does new stream is 10bits depth
>> + * @buf_count: used to count number number of buffers (reqbuf(0))
>> * @fps: holds current FPS
>> * @timeperframe: holds current time per frame structure
>> * @fmt_out: a reference to output format structure
>> @@ -255,8 +268,6 @@ struct venus_buffer {
>> * @opb_buftype: output picture buffer type
>> * @opb_fmt: output picture buffer raw format
>> * @reconfig: a flag raised by decoder when the stream resolution
>> changed
>> - * @reconfig_width: holds the new width
>> - * @reconfig_height: holds the new height
>> * @hfi_codec: current codec for this instance in HFI space
>> * @sequence_cap: a sequence counter for capture queue
>> * @sequence_out: a sequence counter for output queue
>> @@ -296,6 +307,9 @@ struct venus_inst {
>> u8 ycbcr_enc;
>> u8 quantization;
>> u8 xfer_func;
>> + unsigned int codec_state;
>> + wait_queue_head_t reconf_wait;
>> + int buf_count;
>> u64 fps;
>> struct v4l2_fract timeperframe;
>> const struct venus_format *fmt_out;
>> @@ -310,8 +324,6 @@ struct venus_inst {
>> u32 opb_buftype;
>> u32 opb_fmt;
>> bool reconfig;
>> - u32 reconfig_width;
>> - u32 reconfig_height;
>> u32 hfi_codec;
>> u32 sequence_cap;
>> u32 sequence_out;
>> diff --git a/drivers/media/platform/qcom/venus/helpers.c
>> b/drivers/media/platform/qcom/venus/helpers.c
>> index 637ce7b82d94..25d8cceccae4 100644
>> --- a/drivers/media/platform/qcom/venus/helpers.c
>> +++ b/drivers/media/platform/qcom/venus/helpers.c
>> @@ -1030,16 +1030,15 @@ void venus_helper_vb2_buf_queue(struct
>> vb2_buffer *vb)
>>
>> v4l2_m2m_buf_queue(m2m_ctx, vbuf);
>>
>> - if (!(inst->streamon_out & inst->streamon_cap))
>> - goto unlock;
>> -
>> - ret = is_buf_refed(inst, vbuf);
>> - if (ret)
>> - goto unlock;
>> + if (IS_OUT(vb->vb2_queue, inst) || IS_CAP(vb->vb2_queue, inst)) {
>> + ret = is_buf_refed(inst, vbuf);
>> + if (ret)
>> + goto unlock;
>>
>> - ret = session_process_buf(inst, vbuf);
>> - if (ret)
>> - return_buf_error(inst, vbuf);
>> + ret = session_process_buf(inst, vbuf);
>> + if (ret)
>> + return_buf_error(inst, vbuf);
>> + }
>>
>> unlock:
>> mutex_unlock(&inst->lock);
>
> Hi Stan,
>
> In case of encoder, we are queuing buffers only after both planes are
> streamed on.
> As we don’t have any reconfig event in case of encoder,
> it’s better if we stick to the earlier implementation of queuing buffers.
>
> So I would recommend to add a check for the same in the below way :
>
> diff --git a/drivers/media/platform/qcom/venus/helpers.c
> b/drivers/media/platform/qcom/venus/helpers.c
> index 25d8cce..cc490fe2 100644
> --- a/drivers/media/platform/qcom/venus/helpers.c
> +++ b/drivers/media/platform/qcom/venus/helpers.c
> @@ -1029,6 +1029,8 @@ void venus_helper_vb2_buf_queue(struct vb2_buffer
> *vb)
> mutex_lock(&inst->lock);
>
> v4l2_m2m_buf_queue(m2m_ctx, vbuf);
> + if (inst->session_type == VIDC_SESSION_TYPE_ENC &&
> !(inst->streamon_out & inst->streamon_cap))
> + goto unlock;
>
> if (IS_OUT(vb->vb2_queue, inst) || IS_CAP(vb->vb2_queue, inst)) {
> ret = is_buf_refed(inst, vbuf);
>
> Please provide your view.
I agree that this change is needed for encoder and will incorporate such
a change in next version.
--
regards,
Stan
next prev parent reply other threads:[~2019-01-24 10:24 UTC|newest]
Thread overview: 76+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-01-17 16:19 [PATCH 00/10] Venus stateful Codec API Stanimir Varbanov
2019-01-17 16:19 ` [PATCH 01/10] venus: hfi_cmds: add more not-implemented properties Stanimir Varbanov
2019-01-17 16:20 ` [PATCH 02/10] venus: helpers: fix dynamic buffer mode for v4 Stanimir Varbanov
2019-01-17 16:20 ` [PATCH 03/10] venus: helpers: export few helper functions Stanimir Varbanov
2019-01-24 8:43 ` Alexandre Courbot
2019-01-24 8:48 ` Stanimir Varbanov
2019-01-17 16:20 ` [PATCH 04/10] venus: hfi: add type argument to hfi flush function Stanimir Varbanov
2019-01-17 16:20 ` [PATCH 05/10] venus: hfi: export few HFI functions Stanimir Varbanov
2019-01-17 16:20 ` [PATCH 06/10] venus: hfi: return an error if session_init is already called Stanimir Varbanov
2019-01-17 16:20 ` [PATCH 07/10] venus: helpers: add three more helper functions Stanimir Varbanov
2019-01-24 8:43 ` Alexandre Courbot
2019-01-24 8:54 ` Stanimir Varbanov
2019-01-25 5:47 ` Alexandre Courbot
2019-01-17 16:20 ` [PATCH 08/10] venus: vdec_ctrls: get real minimum buffers for capture Stanimir Varbanov
2019-01-24 8:43 ` Alexandre Courbot
2019-01-24 9:59 ` Stanimir Varbanov
2019-01-17 16:20 ` [PATCH 09/10] venus: vdec: allow bigger sizeimage set by clients Stanimir Varbanov
2019-01-24 8:43 ` Alexandre Courbot
2019-01-24 10:05 ` Stanimir Varbanov
2019-01-25 5:36 ` Alexandre Courbot
2019-01-17 16:20 ` [PATCH 10/10] venus: dec: make decoder compliant with stateful codec API Stanimir Varbanov
2019-01-21 11:20 ` mgottam
2019-01-21 11:29 ` mgottam
2019-01-24 10:24 ` Stanimir Varbanov [this message]
2019-01-24 8:44 ` Alexandre Courbot
2019-01-24 12:34 ` Stanimir Varbanov
2019-01-25 5:44 ` Alexandre Courbot
2019-01-25 7:59 ` Tomasz Figa
2019-01-25 10:25 ` Stanimir Varbanov
2019-01-28 7:38 ` Tomasz Figa
2019-01-28 16:28 ` Stanimir Varbanov
2019-01-29 8:24 ` Tomasz Figa
2019-01-30 3:18 ` Nicolas Dufresne
2019-01-30 3:38 ` Tomasz Figa
2019-01-30 4:21 ` Nicolas Dufresne
2019-01-30 6:17 ` Tomasz Figa
2019-01-30 15:32 ` Nicolas Dufresne
2019-01-31 12:42 ` Philipp Zabel
2019-01-31 13:34 ` Tomasz Figa
2019-01-31 15:18 ` Nicolas Dufresne
2019-02-05 6:26 ` Tomasz Figa
2019-02-05 9:00 ` Hans Verkuil
2019-02-05 9:31 ` Tomasz Figa
2019-02-05 10:35 ` Hans Verkuil
2019-02-07 7:33 ` Tomasz Figa
2019-02-14 2:43 ` Tomasz Figa
2019-02-14 16:19 ` Nicolas Dufresne
2019-04-09 9:59 ` Tomasz Figa
2019-04-09 9:59 ` Tomasz Figa
2019-04-09 16:30 ` Nicolas Dufresne
2019-04-09 16:30 ` Nicolas Dufresne
2019-04-10 2:32 ` Tomasz Figa
2019-04-10 2:32 ` Tomasz Figa
2019-02-15 13:44 ` Hans Verkuil
2019-02-15 16:27 ` Nicolas Dufresne
2019-02-15 16:40 ` Hans Verkuil
2019-04-24 12:15 ` Stanimir Varbanov
2019-04-24 12:15 ` Stanimir Varbanov
2019-04-24 12:39 ` Tomasz Figa
2019-04-24 12:39 ` Tomasz Figa
2019-05-20 14:47 ` Stanimir Varbanov
2019-05-21 9:09 ` Tomasz Figa
2019-05-21 12:27 ` Hans Verkuil
2019-05-27 3:51 ` Tomasz Figa
2019-05-27 7:39 ` Hans Verkuil
2019-05-27 8:18 ` Tomasz Figa
2019-05-31 8:01 ` Stanimir Varbanov
2019-05-31 13:53 ` Nicolas Dufresne
2019-06-03 7:25 ` Hans Verkuil
2019-06-03 7:30 ` Tomasz Figa
2019-01-24 8:43 ` [PATCH 00/10] Venus stateful Codec API Alexandre Courbot
2019-01-24 10:12 ` Stanimir Varbanov
2019-01-25 5:34 ` Alexandre Courbot
2019-01-25 10:35 ` Stanimir Varbanov
2019-01-28 4:16 ` Alexandre Courbot
2019-01-28 4:30 ` Tomasz Figa
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=1663ea51-1fa0-6e29-715d-9d67aabb5ca9@linaro.org \
--to=stanimir.varbanov@linaro.org \
--cc=acourbot@chromium.org \
--cc=hverkuil@xs4all.nl \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=mchehab@kernel.org \
--cc=mgottam@codeaurora.org \
--cc=tfiga@chromium.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).