Re: [PATCH v3 05/15] media: mtk-vcodec: vdec: support stateless API
Le mercredi 17 mars 2021 à 12:13 +0900, Alexandre Courbot a écrit :
> On Tue, Mar 16, 2021 at 6:45 AM Ezequiel Garcia
> wrote:
> >
> > Hi Alexandre,
> >
> > On Mon, 15 Mar 2021 at 08:28, Alexandre Courbot
> > wrote:
> > >
> > > Hi Ezequiel, thanks for the feedback!
> > >
> > > On Thu, Mar 4, 2021 at 6:30 AM Ezequiel Garcia
> > > wrote:
> > > >
> > > > Hello Alex,
> > > >
> > > > Thanks for the patch.
> > > >
> > > > On Fri, 26 Feb 2021 at 07:06, Alexandre Courbot
> > > > wrote:
> > > > >
> > > > > From: Yunfei Dong
> > > > >
> > > > > Support the stateless codec API that will be used by MT8183.
> > > > >
> > > > > Signed-off-by: Yunfei Dong
> > > > > [acourbot: refactor, cleanup and split]
> > > > > Co-developed-by: Alexandre Courbot
> > > > > Signed-off-by: Alexandre Courbot
> > > > > ---
> > > > > drivers/media/platform/mtk-vcodec/Makefile | 1 +
> > > > > .../platform/mtk-vcodec/mtk_vcodec_dec.c | 66 ++-
> > > > > .../platform/mtk-vcodec/mtk_vcodec_dec.h | 9 +-
> > > > > .../mtk-vcodec/mtk_vcodec_dec_stateless.c | 427
> > > > > ++
> > > > > .../platform/mtk-vcodec/mtk_vcodec_drv.h | 3 +
> > > > > 5 files changed, 503 insertions(+), 3 deletions(-)
> > > > > create mode 100644 drivers/media/platform/mtk-
> > > > > vcodec/mtk_vcodec_dec_stateless.c
> > > > >
> > > > [..]
> > > >
> > > > > +
> > > > > +static const struct mtk_stateless_control mtk_stateless_controls[] =
> > > > > {
> > > > > + {
> > > > > + .cfg = {
> > > > > + .id = V4L2_CID_STATELESS_H264_SPS,
> > > > > + },
> > > > > + .codec_type = V4L2_PIX_FMT_H264_SLICE,
> > > > > + .needed_in_request = true,
> > > >
> > > > This "needed_in_request" is not really required, as controls
> > > > are not volatile, and their value is stored per-context (per-fd).
> > > >
> > > > It's perfectly valid for an application to pass the SPS control
> > > > at the beginning of the sequence, and then omit it
> > > > in further requests.
> > >
> > > If I understand how v4l2_ctrl_request_hdl_ctrl_find() works with
> > > requests, this boolean only checks that the control has been provided
> > > at least once, and not that it is provided with every request. Without
> > > it we could send a frame to the firmware without e.g. setting an SPS,
> > > which would be a problem.
> > >
> >
> > As Nicolas points out, in V4L2 controls have an initial value,
> > so no control can be unset.
>
> I see. So I guess the expectation is that failure will occur later as
> the firmware reports it cannot decode properly (or returns a corrupted
> frame). Thanks for the precision.
That is identical to userspace passing bad values. We just don't want to force
userspace to pass controls that haven't changed. The control framework isn't
exactly free in CPU time.
>
> >
> > > >
> > > > > + },
> > > > > + {
> > > > > + .cfg = {
> > > > > + .id = V4L2_CID_STATELESS_H264_PPS,
> > > > > + },
> > > > > + .codec_type = V4L2_PIX_FMT_H264_SLICE,
> > > > > + .needed_in_request = true,
> > > > > + },
> > > > > + {
> > > > > + .cfg = {
> > > > > + .id = V4L2_CID_STATELESS_H264_SCALING_MATRIX,
> > > > > + },
> > > > > + .codec_type = V4L2_PIX_FMT_H264_SLICE,
> > > > > + .needed_in_request = true,
> > > > > + },
> > > > > + {
> > > > > + .cfg = {
> > > > > + .id = V4L2_CID_STATELESS_H264_DECODE_PARAMS,
> > > > > + },
> > > > > + .codec_type = V4L2_PIX_FMT_H264_SLICE,
> > > > > + .needed_in_request = true,
> > > > > + },
> > > > > + {
> > > > > + .cfg = {
> > > > > + .id = V4L2_CID_MPEG_VIDEO_H264_PROFILE,
> > > > > + .def = V4L2_MPEG_VIDEO_H264_PROFILE_MAIN,
> > > > > + .max = V4L2_MPEG_VIDEO_H264_PROFILE_HIGH,
> > > > > + .menu_skip_mask =
> > > > > +
> > > > > BIT(V4L2_MPEG_VIDEO_H264_PROFILE_BASELINE) |
> > > > > +
> > > > > BIT(V4L2_MPEG_VIDEO_H264_PROFILE_EXTENDED),
> > > > > + },
> > > > > + .codec_type = V4L2_PIX_FMT_H264_SLICE,
> > > > > + },
> > > > > + {
> > > > > + .cfg = {
> > > > > + .id = V4L2_CID_STATELESS_H264_DECODE_MODE,
> > > > > + .min =
> > > > > V4L2_STATELESS_H264_DECODE_MODE_FRAME_BASED,
> > > > > + .def =
> > > > > V4L2_STATELESS_H264_DECODE_MODE_FRAME_BASED,
> > > > > + .max =
> > > > > V4L2_STATELESS_H264_DECODE_MODE_FRAME_BASED,
> > > > > + },
> > > > > + .codec_type = V4L2_PIX_FMT_H264_SLICE,
> > > > > + },
> >
Re: [PATCH v3 05/15] media: mtk-vcodec: vdec: support stateless API
On Tue, Mar 16, 2021 at 6:45 AM Ezequiel Garcia
wrote:
>
> Hi Alexandre,
>
> On Mon, 15 Mar 2021 at 08:28, Alexandre Courbot wrote:
> >
> > Hi Ezequiel, thanks for the feedback!
> >
> > On Thu, Mar 4, 2021 at 6:30 AM Ezequiel Garcia
> > wrote:
> > >
> > > Hello Alex,
> > >
> > > Thanks for the patch.
> > >
> > > On Fri, 26 Feb 2021 at 07:06, Alexandre Courbot
> > > wrote:
> > > >
> > > > From: Yunfei Dong
> > > >
> > > > Support the stateless codec API that will be used by MT8183.
> > > >
> > > > Signed-off-by: Yunfei Dong
> > > > [acourbot: refactor, cleanup and split]
> > > > Co-developed-by: Alexandre Courbot
> > > > Signed-off-by: Alexandre Courbot
> > > > ---
> > > > drivers/media/platform/mtk-vcodec/Makefile| 1 +
> > > > .../platform/mtk-vcodec/mtk_vcodec_dec.c | 66 ++-
> > > > .../platform/mtk-vcodec/mtk_vcodec_dec.h | 9 +-
> > > > .../mtk-vcodec/mtk_vcodec_dec_stateless.c | 427 ++
> > > > .../platform/mtk-vcodec/mtk_vcodec_drv.h | 3 +
> > > > 5 files changed, 503 insertions(+), 3 deletions(-)
> > > > create mode 100644
> > > > drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_stateless.c
> > > >
> > > [..]
> > >
> > > > +
> > > > +static const struct mtk_stateless_control mtk_stateless_controls[] = {
> > > > + {
> > > > + .cfg = {
> > > > + .id = V4L2_CID_STATELESS_H264_SPS,
> > > > + },
> > > > + .codec_type = V4L2_PIX_FMT_H264_SLICE,
> > > > + .needed_in_request = true,
> > >
> > > This "needed_in_request" is not really required, as controls
> > > are not volatile, and their value is stored per-context (per-fd).
> > >
> > > It's perfectly valid for an application to pass the SPS control
> > > at the beginning of the sequence, and then omit it
> > > in further requests.
> >
> > If I understand how v4l2_ctrl_request_hdl_ctrl_find() works with
> > requests, this boolean only checks that the control has been provided
> > at least once, and not that it is provided with every request. Without
> > it we could send a frame to the firmware without e.g. setting an SPS,
> > which would be a problem.
> >
>
> As Nicolas points out, in V4L2 controls have an initial value,
> so no control can be unset.
I see. So I guess the expectation is that failure will occur later as
the firmware reports it cannot decode properly (or returns a corrupted
frame). Thanks for the precision.
>
> > >
> > > > + },
> > > > + {
> > > > + .cfg = {
> > > > + .id = V4L2_CID_STATELESS_H264_PPS,
> > > > + },
> > > > + .codec_type = V4L2_PIX_FMT_H264_SLICE,
> > > > + .needed_in_request = true,
> > > > + },
> > > > + {
> > > > + .cfg = {
> > > > + .id = V4L2_CID_STATELESS_H264_SCALING_MATRIX,
> > > > + },
> > > > + .codec_type = V4L2_PIX_FMT_H264_SLICE,
> > > > + .needed_in_request = true,
> > > > + },
> > > > + {
> > > > + .cfg = {
> > > > + .id = V4L2_CID_STATELESS_H264_DECODE_PARAMS,
> > > > + },
> > > > + .codec_type = V4L2_PIX_FMT_H264_SLICE,
> > > > + .needed_in_request = true,
> > > > + },
> > > > + {
> > > > + .cfg = {
> > > > + .id = V4L2_CID_MPEG_VIDEO_H264_PROFILE,
> > > > + .def = V4L2_MPEG_VIDEO_H264_PROFILE_MAIN,
> > > > + .max = V4L2_MPEG_VIDEO_H264_PROFILE_HIGH,
> > > > + .menu_skip_mask =
> > > > +
> > > > BIT(V4L2_MPEG_VIDEO_H264_PROFILE_BASELINE) |
> > > > +
> > > > BIT(V4L2_MPEG_VIDEO_H264_PROFILE_EXTENDED),
> > > > + },
> > > > + .codec_type = V4L2_PIX_FMT_H264_SLICE,
> > > > + },
> > > > + {
> > > > + .cfg = {
> > > > + .id = V4L2_CID_STATELESS_H264_DECODE_MODE,
> > > > + .min =
> > > > V4L2_STATELESS_H264_DECODE_MODE_FRAME_BASED,
> > > > + .def =
> > > > V4L2_STATELESS_H264_DECODE_MODE_FRAME_BASED,
> > > > + .max =
> > > > V4L2_STATELESS_H264_DECODE_MODE_FRAME_BASED,
> > > > + },
> > > > + .codec_type = V4L2_PIX_FMT_H264_SLICE,
> > > > + },
> > > > +};
> > >
> > > Applications also need to know which V4L2_CID_STATELESS_H264_START_CODE
> > > the driver supports. From a next patch, this case seems to be
> > > V4L2_STATELESS_H264_START_CODE_ANNEX_B.
> >
> > Indeed - I've added the control, thanks for catching this!
> >
> > >
> > > > +#define NUM_CTRLS ARRAY_SIZE(mtk_stateless_controls)
> > > > +
> > > > +static const struct mtk_video_fmt mtk_video_formats[] = {
> > > > + {
> > > > + .fourcc = V4L2_PIX_FMT_H264_SLICE,
> > > > + .
Re: [PATCH v3 05/15] media: mtk-vcodec: vdec: support stateless API
Hi Alexandre,
On Mon, 15 Mar 2021 at 08:28, Alexandre Courbot wrote:
>
> Hi Ezequiel, thanks for the feedback!
>
> On Thu, Mar 4, 2021 at 6:30 AM Ezequiel Garcia
> wrote:
> >
> > Hello Alex,
> >
> > Thanks for the patch.
> >
> > On Fri, 26 Feb 2021 at 07:06, Alexandre Courbot
> > wrote:
> > >
> > > From: Yunfei Dong
> > >
> > > Support the stateless codec API that will be used by MT8183.
> > >
> > > Signed-off-by: Yunfei Dong
> > > [acourbot: refactor, cleanup and split]
> > > Co-developed-by: Alexandre Courbot
> > > Signed-off-by: Alexandre Courbot
> > > ---
> > > drivers/media/platform/mtk-vcodec/Makefile| 1 +
> > > .../platform/mtk-vcodec/mtk_vcodec_dec.c | 66 ++-
> > > .../platform/mtk-vcodec/mtk_vcodec_dec.h | 9 +-
> > > .../mtk-vcodec/mtk_vcodec_dec_stateless.c | 427 ++
> > > .../platform/mtk-vcodec/mtk_vcodec_drv.h | 3 +
> > > 5 files changed, 503 insertions(+), 3 deletions(-)
> > > create mode 100644
> > > drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_stateless.c
> > >
> > [..]
> >
> > > +
> > > +static const struct mtk_stateless_control mtk_stateless_controls[] = {
> > > + {
> > > + .cfg = {
> > > + .id = V4L2_CID_STATELESS_H264_SPS,
> > > + },
> > > + .codec_type = V4L2_PIX_FMT_H264_SLICE,
> > > + .needed_in_request = true,
> >
> > This "needed_in_request" is not really required, as controls
> > are not volatile, and their value is stored per-context (per-fd).
> >
> > It's perfectly valid for an application to pass the SPS control
> > at the beginning of the sequence, and then omit it
> > in further requests.
>
> If I understand how v4l2_ctrl_request_hdl_ctrl_find() works with
> requests, this boolean only checks that the control has been provided
> at least once, and not that it is provided with every request. Without
> it we could send a frame to the firmware without e.g. setting an SPS,
> which would be a problem.
>
As Nicolas points out, in V4L2 controls have an initial value,
so no control can be unset.
> >
> > > + },
> > > + {
> > > + .cfg = {
> > > + .id = V4L2_CID_STATELESS_H264_PPS,
> > > + },
> > > + .codec_type = V4L2_PIX_FMT_H264_SLICE,
> > > + .needed_in_request = true,
> > > + },
> > > + {
> > > + .cfg = {
> > > + .id = V4L2_CID_STATELESS_H264_SCALING_MATRIX,
> > > + },
> > > + .codec_type = V4L2_PIX_FMT_H264_SLICE,
> > > + .needed_in_request = true,
> > > + },
> > > + {
> > > + .cfg = {
> > > + .id = V4L2_CID_STATELESS_H264_DECODE_PARAMS,
> > > + },
> > > + .codec_type = V4L2_PIX_FMT_H264_SLICE,
> > > + .needed_in_request = true,
> > > + },
> > > + {
> > > + .cfg = {
> > > + .id = V4L2_CID_MPEG_VIDEO_H264_PROFILE,
> > > + .def = V4L2_MPEG_VIDEO_H264_PROFILE_MAIN,
> > > + .max = V4L2_MPEG_VIDEO_H264_PROFILE_HIGH,
> > > + .menu_skip_mask =
> > > +
> > > BIT(V4L2_MPEG_VIDEO_H264_PROFILE_BASELINE) |
> > > +
> > > BIT(V4L2_MPEG_VIDEO_H264_PROFILE_EXTENDED),
> > > + },
> > > + .codec_type = V4L2_PIX_FMT_H264_SLICE,
> > > + },
> > > + {
> > > + .cfg = {
> > > + .id = V4L2_CID_STATELESS_H264_DECODE_MODE,
> > > + .min =
> > > V4L2_STATELESS_H264_DECODE_MODE_FRAME_BASED,
> > > + .def =
> > > V4L2_STATELESS_H264_DECODE_MODE_FRAME_BASED,
> > > + .max =
> > > V4L2_STATELESS_H264_DECODE_MODE_FRAME_BASED,
> > > + },
> > > + .codec_type = V4L2_PIX_FMT_H264_SLICE,
> > > + },
> > > +};
> >
> > Applications also need to know which V4L2_CID_STATELESS_H264_START_CODE
> > the driver supports. From a next patch, this case seems to be
> > V4L2_STATELESS_H264_START_CODE_ANNEX_B.
>
> Indeed - I've added the control, thanks for catching this!
>
> >
> > > +#define NUM_CTRLS ARRAY_SIZE(mtk_stateless_controls)
> > > +
> > > +static const struct mtk_video_fmt mtk_video_formats[] = {
> > > + {
> > > + .fourcc = V4L2_PIX_FMT_H264_SLICE,
> > > + .type = MTK_FMT_DEC,
> > > + .num_planes = 1,
> > > + },
> > > + {
> > > + .fourcc = V4L2_PIX_FMT_MM21,
> > > + .type = MTK_FMT_FRAME,
> > > + .num_planes = 2,
> > > + },
> > > +};
> > > +#define NUM_FORMATS ARRAY_SIZE(mtk_video_formats)
> > > +#define DEFAULT_OUT_FMT_IDX0
> > > +#define DEFAULT_CAP_FMT_IDX1
> > > +
> > > +static const struct mtk_codec_framesizes mtk_vdec_framesizes[] = {
> > > +
Re: [PATCH v3 05/15] media: mtk-vcodec: vdec: support stateless API
Le lundi 15 mars 2021 à 20:28 +0900, Alexandre Courbot a écrit :
> Hi Ezequiel, thanks for the feedback!
>
> On Thu, Mar 4, 2021 at 6:30 AM Ezequiel Garcia
> wrote:
> >
> > Hello Alex,
> >
> > Thanks for the patch.
> >
> > On Fri, 26 Feb 2021 at 07:06, Alexandre Courbot
> > wrote:
> > >
> > > From: Yunfei Dong
> > >
> > > Support the stateless codec API that will be used by MT8183.
> > >
> > > Signed-off-by: Yunfei Dong
> > > [acourbot: refactor, cleanup and split]
> > > Co-developed-by: Alexandre Courbot
> > > Signed-off-by: Alexandre Courbot
> > > ---
> > > drivers/media/platform/mtk-vcodec/Makefile | 1 +
> > > .../platform/mtk-vcodec/mtk_vcodec_dec.c | 66 ++-
> > > .../platform/mtk-vcodec/mtk_vcodec_dec.h | 9 +-
> > > .../mtk-vcodec/mtk_vcodec_dec_stateless.c | 427 ++
> > > .../platform/mtk-vcodec/mtk_vcodec_drv.h | 3 +
> > > 5 files changed, 503 insertions(+), 3 deletions(-)
> > > create mode 100644 drivers/media/platform/mtk-
> > > vcodec/mtk_vcodec_dec_stateless.c
> > >
> > [..]
> >
> > > +
> > > +static const struct mtk_stateless_control mtk_stateless_controls[] = {
> > > + {
> > > + .cfg = {
> > > + .id = V4L2_CID_STATELESS_H264_SPS,
> > > + },
> > > + .codec_type = V4L2_PIX_FMT_H264_SLICE,
> > > + .needed_in_request = true,
> >
> > This "needed_in_request" is not really required, as controls
> > are not volatile, and their value is stored per-context (per-fd).
> >
> > It's perfectly valid for an application to pass the SPS control
> > at the beginning of the sequence, and then omit it
> > in further requests.
>
> If I understand how v4l2_ctrl_request_hdl_ctrl_find() works with
> requests, this boolean only checks that the control has been provided
> at least once, and not that it is provided with every request. Without
> it we could send a frame to the firmware without e.g. setting an SPS,
> which would be a problem.
In other drivers (Cedrus and RKVDEC) this was actually checking if the control
was part of the request, I doubt the framework have a state for "being set
once", as control have no set/unset state. Did you wrote this code and tested
this intended behaviour or borred that code from somewhere else ?
>
> >
> > > + },
> > > + {
> > > + .cfg = {
> > > + .id = V4L2_CID_STATELESS_H264_PPS,
> > > + },
> > > + .codec_type = V4L2_PIX_FMT_H264_SLICE,
> > > + .needed_in_request = true,
> > > + },
> > > + {
> > > + .cfg = {
> > > + .id = V4L2_CID_STATELESS_H264_SCALING_MATRIX,
> > > + },
> > > + .codec_type = V4L2_PIX_FMT_H264_SLICE,
> > > + .needed_in_request = true,
> > > + },
> > > + {
> > > + .cfg = {
> > > + .id = V4L2_CID_STATELESS_H264_DECODE_PARAMS,
> > > + },
> > > + .codec_type = V4L2_PIX_FMT_H264_SLICE,
> > > + .needed_in_request = true,
> > > + },
> > > + {
> > > + .cfg = {
> > > + .id = V4L2_CID_MPEG_VIDEO_H264_PROFILE,
> > > + .def = V4L2_MPEG_VIDEO_H264_PROFILE_MAIN,
> > > + .max = V4L2_MPEG_VIDEO_H264_PROFILE_HIGH,
> > > + .menu_skip_mask =
> > > + BIT(V4L2_MPEG_VIDEO_H264_PROFILE_BASELINE)
> > > |
> > > +
> > > BIT(V4L2_MPEG_VIDEO_H264_PROFILE_EXTENDED),
> > > + },
> > > + .codec_type = V4L2_PIX_FMT_H264_SLICE,
> > > + },
> > > + {
> > > + .cfg = {
> > > + .id = V4L2_CID_STATELESS_H264_DECODE_MODE,
> > > + .min =
> > > V4L2_STATELESS_H264_DECODE_MODE_FRAME_BASED,
> > > + .def =
> > > V4L2_STATELESS_H264_DECODE_MODE_FRAME_BASED,
> > > + .max =
> > > V4L2_STATELESS_H264_DECODE_MODE_FRAME_BASED,
> > > + },
> > > + .codec_type = V4L2_PIX_FMT_H264_SLICE,
> > > + },
> > > +};
> >
> > Applications also need to know which V4L2_CID_STATELESS_H264_START_CODE
> > the driver supports. From a next patch, this case seems to be
> > V4L2_STATELESS_H264_START_CODE_ANNEX_B.
>
> Indeed - I've added the control, thanks for catching this!
>
> >
> > > +#define NUM_CTRLS ARRAY_SIZE(mtk_stateless_controls)
> > > +
> > > +static const struct mtk_video_fmt mtk_video_formats[] = {
> > > + {
> > > + .fourcc = V4L2_PIX_FMT_H264_SLICE,
> > > + .type = MTK_FMT_DEC,
> > > + .num_planes = 1,
> > > + },
> > > + {
> > > + .fourcc = V4L2_PIX_FMT_MM21,
> > > + .type = MTK_FMT_FRAME,
> > > + .num_planes = 2,
> > > + },
> > > +};
> > > +#define NU
Re: [PATCH v3 05/15] media: mtk-vcodec: vdec: support stateless API
Hi Ezequiel, thanks for the feedback!
On Thu, Mar 4, 2021 at 6:30 AM Ezequiel Garcia
wrote:
>
> Hello Alex,
>
> Thanks for the patch.
>
> On Fri, 26 Feb 2021 at 07:06, Alexandre Courbot wrote:
> >
> > From: Yunfei Dong
> >
> > Support the stateless codec API that will be used by MT8183.
> >
> > Signed-off-by: Yunfei Dong
> > [acourbot: refactor, cleanup and split]
> > Co-developed-by: Alexandre Courbot
> > Signed-off-by: Alexandre Courbot
> > ---
> > drivers/media/platform/mtk-vcodec/Makefile| 1 +
> > .../platform/mtk-vcodec/mtk_vcodec_dec.c | 66 ++-
> > .../platform/mtk-vcodec/mtk_vcodec_dec.h | 9 +-
> > .../mtk-vcodec/mtk_vcodec_dec_stateless.c | 427 ++
> > .../platform/mtk-vcodec/mtk_vcodec_drv.h | 3 +
> > 5 files changed, 503 insertions(+), 3 deletions(-)
> > create mode 100644
> > drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_stateless.c
> >
> [..]
>
> > +
> > +static const struct mtk_stateless_control mtk_stateless_controls[] = {
> > + {
> > + .cfg = {
> > + .id = V4L2_CID_STATELESS_H264_SPS,
> > + },
> > + .codec_type = V4L2_PIX_FMT_H264_SLICE,
> > + .needed_in_request = true,
>
> This "needed_in_request" is not really required, as controls
> are not volatile, and their value is stored per-context (per-fd).
>
> It's perfectly valid for an application to pass the SPS control
> at the beginning of the sequence, and then omit it
> in further requests.
If I understand how v4l2_ctrl_request_hdl_ctrl_find() works with
requests, this boolean only checks that the control has been provided
at least once, and not that it is provided with every request. Without
it we could send a frame to the firmware without e.g. setting an SPS,
which would be a problem.
>
> > + },
> > + {
> > + .cfg = {
> > + .id = V4L2_CID_STATELESS_H264_PPS,
> > + },
> > + .codec_type = V4L2_PIX_FMT_H264_SLICE,
> > + .needed_in_request = true,
> > + },
> > + {
> > + .cfg = {
> > + .id = V4L2_CID_STATELESS_H264_SCALING_MATRIX,
> > + },
> > + .codec_type = V4L2_PIX_FMT_H264_SLICE,
> > + .needed_in_request = true,
> > + },
> > + {
> > + .cfg = {
> > + .id = V4L2_CID_STATELESS_H264_DECODE_PARAMS,
> > + },
> > + .codec_type = V4L2_PIX_FMT_H264_SLICE,
> > + .needed_in_request = true,
> > + },
> > + {
> > + .cfg = {
> > + .id = V4L2_CID_MPEG_VIDEO_H264_PROFILE,
> > + .def = V4L2_MPEG_VIDEO_H264_PROFILE_MAIN,
> > + .max = V4L2_MPEG_VIDEO_H264_PROFILE_HIGH,
> > + .menu_skip_mask =
> > + BIT(V4L2_MPEG_VIDEO_H264_PROFILE_BASELINE) |
> > + BIT(V4L2_MPEG_VIDEO_H264_PROFILE_EXTENDED),
> > + },
> > + .codec_type = V4L2_PIX_FMT_H264_SLICE,
> > + },
> > + {
> > + .cfg = {
> > + .id = V4L2_CID_STATELESS_H264_DECODE_MODE,
> > + .min = V4L2_STATELESS_H264_DECODE_MODE_FRAME_BASED,
> > + .def = V4L2_STATELESS_H264_DECODE_MODE_FRAME_BASED,
> > + .max = V4L2_STATELESS_H264_DECODE_MODE_FRAME_BASED,
> > + },
> > + .codec_type = V4L2_PIX_FMT_H264_SLICE,
> > + },
> > +};
>
> Applications also need to know which V4L2_CID_STATELESS_H264_START_CODE
> the driver supports. From a next patch, this case seems to be
> V4L2_STATELESS_H264_START_CODE_ANNEX_B.
Indeed - I've added the control, thanks for catching this!
>
> > +#define NUM_CTRLS ARRAY_SIZE(mtk_stateless_controls)
> > +
> > +static const struct mtk_video_fmt mtk_video_formats[] = {
> > + {
> > + .fourcc = V4L2_PIX_FMT_H264_SLICE,
> > + .type = MTK_FMT_DEC,
> > + .num_planes = 1,
> > + },
> > + {
> > + .fourcc = V4L2_PIX_FMT_MM21,
> > + .type = MTK_FMT_FRAME,
> > + .num_planes = 2,
> > + },
> > +};
> > +#define NUM_FORMATS ARRAY_SIZE(mtk_video_formats)
> > +#define DEFAULT_OUT_FMT_IDX0
> > +#define DEFAULT_CAP_FMT_IDX1
> > +
> > +static const struct mtk_codec_framesizes mtk_vdec_framesizes[] = {
> > + {
> > + .fourcc = V4L2_PIX_FMT_H264_SLICE,
> > + .stepwise = {
> > + MTK_VDEC_MIN_W, MTK_VDEC_MAX_W, 16,
> > + MTK_VDEC_MIN_H, MTK_VDEC_MAX_H, 16,
> > + },
> > + },
> > +};
> > +
> > +#define NUM_SUPPORTED_FRAMESIZE ARRAY_SIZE(mtk_vdec_framesizes)
> > +
> > +static void mtk_vdec_stateless_set_dst_payload(struct mtk_vcodec_ctx *ctx,
> > +
Re: [PATCH v3 05/15] media: mtk-vcodec: vdec: support stateless API
Hello Alex,
Thanks for the patch.
On Fri, 26 Feb 2021 at 07:06, Alexandre Courbot wrote:
>
> From: Yunfei Dong
>
> Support the stateless codec API that will be used by MT8183.
>
> Signed-off-by: Yunfei Dong
> [acourbot: refactor, cleanup and split]
> Co-developed-by: Alexandre Courbot
> Signed-off-by: Alexandre Courbot
> ---
> drivers/media/platform/mtk-vcodec/Makefile| 1 +
> .../platform/mtk-vcodec/mtk_vcodec_dec.c | 66 ++-
> .../platform/mtk-vcodec/mtk_vcodec_dec.h | 9 +-
> .../mtk-vcodec/mtk_vcodec_dec_stateless.c | 427 ++
> .../platform/mtk-vcodec/mtk_vcodec_drv.h | 3 +
> 5 files changed, 503 insertions(+), 3 deletions(-)
> create mode 100644
> drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_stateless.c
>
[..]
> +
> +static const struct mtk_stateless_control mtk_stateless_controls[] = {
> + {
> + .cfg = {
> + .id = V4L2_CID_STATELESS_H264_SPS,
> + },
> + .codec_type = V4L2_PIX_FMT_H264_SLICE,
> + .needed_in_request = true,
This "needed_in_request" is not really required, as controls
are not volatile, and their value is stored per-context (per-fd).
It's perfectly valid for an application to pass the SPS control
at the beginning of the sequence, and then omit it
in further requests.
> + },
> + {
> + .cfg = {
> + .id = V4L2_CID_STATELESS_H264_PPS,
> + },
> + .codec_type = V4L2_PIX_FMT_H264_SLICE,
> + .needed_in_request = true,
> + },
> + {
> + .cfg = {
> + .id = V4L2_CID_STATELESS_H264_SCALING_MATRIX,
> + },
> + .codec_type = V4L2_PIX_FMT_H264_SLICE,
> + .needed_in_request = true,
> + },
> + {
> + .cfg = {
> + .id = V4L2_CID_STATELESS_H264_DECODE_PARAMS,
> + },
> + .codec_type = V4L2_PIX_FMT_H264_SLICE,
> + .needed_in_request = true,
> + },
> + {
> + .cfg = {
> + .id = V4L2_CID_MPEG_VIDEO_H264_PROFILE,
> + .def = V4L2_MPEG_VIDEO_H264_PROFILE_MAIN,
> + .max = V4L2_MPEG_VIDEO_H264_PROFILE_HIGH,
> + .menu_skip_mask =
> + BIT(V4L2_MPEG_VIDEO_H264_PROFILE_BASELINE) |
> + BIT(V4L2_MPEG_VIDEO_H264_PROFILE_EXTENDED),
> + },
> + .codec_type = V4L2_PIX_FMT_H264_SLICE,
> + },
> + {
> + .cfg = {
> + .id = V4L2_CID_STATELESS_H264_DECODE_MODE,
> + .min = V4L2_STATELESS_H264_DECODE_MODE_FRAME_BASED,
> + .def = V4L2_STATELESS_H264_DECODE_MODE_FRAME_BASED,
> + .max = V4L2_STATELESS_H264_DECODE_MODE_FRAME_BASED,
> + },
> + .codec_type = V4L2_PIX_FMT_H264_SLICE,
> + },
> +};
Applications also need to know which V4L2_CID_STATELESS_H264_START_CODE
the driver supports. From a next patch, this case seems to be
V4L2_STATELESS_H264_START_CODE_ANNEX_B.
> +#define NUM_CTRLS ARRAY_SIZE(mtk_stateless_controls)
> +
> +static const struct mtk_video_fmt mtk_video_formats[] = {
> + {
> + .fourcc = V4L2_PIX_FMT_H264_SLICE,
> + .type = MTK_FMT_DEC,
> + .num_planes = 1,
> + },
> + {
> + .fourcc = V4L2_PIX_FMT_MM21,
> + .type = MTK_FMT_FRAME,
> + .num_planes = 2,
> + },
> +};
> +#define NUM_FORMATS ARRAY_SIZE(mtk_video_formats)
> +#define DEFAULT_OUT_FMT_IDX0
> +#define DEFAULT_CAP_FMT_IDX1
> +
> +static const struct mtk_codec_framesizes mtk_vdec_framesizes[] = {
> + {
> + .fourcc = V4L2_PIX_FMT_H264_SLICE,
> + .stepwise = {
> + MTK_VDEC_MIN_W, MTK_VDEC_MAX_W, 16,
> + MTK_VDEC_MIN_H, MTK_VDEC_MAX_H, 16,
> + },
> + },
> +};
> +
> +#define NUM_SUPPORTED_FRAMESIZE ARRAY_SIZE(mtk_vdec_framesizes)
> +
> +static void mtk_vdec_stateless_set_dst_payload(struct mtk_vcodec_ctx *ctx,
> + struct vdec_fb *fb)
> +{
> + struct mtk_video_dec_buf *vdec_frame_buf =
> + container_of(fb, struct mtk_video_dec_buf, frame_buffer);
> + struct vb2_v4l2_buffer *vb = &vdec_frame_buf->m2m_buf.vb;
> + unsigned int cap_y_size = ctx->q_data[MTK_Q_DATA_DST].sizeimage[0];
> +
> + vb2_set_plane_payload(&vb->vb2_buf, 0, cap_y_size);
> + if (ctx->q_data[MTK_Q_DATA_DST].fmt->num_planes == 2) {
> + unsigned int cap_c_size =
> + ctx->q_data[MTK_Q_DATA_DST].sizeimage[1];
> +
> + vb2_set_plane_payload(&vb->vb2_buf, 1, cap_c_size);
> + }
> +}
> +
> +static struct vdec_fb *v
[PATCH v3 05/15] media: mtk-vcodec: vdec: support stateless API
From: Yunfei Dong
Support the stateless codec API that will be used by MT8183.
Signed-off-by: Yunfei Dong
[acourbot: refactor, cleanup and split]
Co-developed-by: Alexandre Courbot
Signed-off-by: Alexandre Courbot
---
drivers/media/platform/mtk-vcodec/Makefile| 1 +
.../platform/mtk-vcodec/mtk_vcodec_dec.c | 66 ++-
.../platform/mtk-vcodec/mtk_vcodec_dec.h | 9 +-
.../mtk-vcodec/mtk_vcodec_dec_stateless.c | 427 ++
.../platform/mtk-vcodec/mtk_vcodec_drv.h | 3 +
5 files changed, 503 insertions(+), 3 deletions(-)
create mode 100644 drivers/media/platform/mtk-vcodec/mtk_vcodec_dec_stateless.c
diff --git a/drivers/media/platform/mtk-vcodec/Makefile
b/drivers/media/platform/mtk-vcodec/Makefile
index 9c3cbb5b800e..4ba93d838ab6 100644
--- a/drivers/media/platform/mtk-vcodec/Makefile
+++ b/drivers/media/platform/mtk-vcodec/Makefile
@@ -12,6 +12,7 @@ mtk-vcodec-dec-y := vdec/vdec_h264_if.o \
vdec_vpu_if.o \
mtk_vcodec_dec.o \
mtk_vcodec_dec_stateful.o \
+ mtk_vcodec_dec_stateless.o \
mtk_vcodec_dec_pm.o \
mtk-vcodec-enc-y := venc/venc_vp8_if.o \
diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c
b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c
index 4a91d294002b..c286cc0f239f 100644
--- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c
+++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_dec.c
@@ -46,6 +46,13 @@ static struct mtk_q_data *mtk_vdec_get_q_data(struct
mtk_vcodec_ctx *ctx,
static int vidioc_try_decoder_cmd(struct file *file, void *priv,
struct v4l2_decoder_cmd *cmd)
{
+ struct mtk_vcodec_ctx *ctx = fh_to_ctx(priv);
+
+ /* Use M2M stateless helper if relevant */
+ if (ctx->dev->vdec_pdata->uses_stateless_api)
+ return v4l2_m2m_ioctl_stateless_try_decoder_cmd(file, priv,
+ cmd);
+
switch (cmd->cmd) {
case V4L2_DEC_CMD_STOP:
case V4L2_DEC_CMD_START:
@@ -72,6 +79,10 @@ static int vidioc_decoder_cmd(struct file *file, void *priv,
if (ret)
return ret;
+ /* Use M2M stateless helper if relevant */
+ if (ctx->dev->vdec_pdata->uses_stateless_api)
+ return v4l2_m2m_ioctl_stateless_decoder_cmd(file, priv, cmd);
+
mtk_v4l2_debug(1, "decoder cmd=%u", cmd->cmd);
dst_vq = v4l2_m2m_get_vq(ctx->m2m_ctx,
V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
@@ -414,7 +425,8 @@ static int vidioc_vdec_s_fmt(struct file *file, void *priv,
* Setting OUTPUT format after OUTPUT buffers are allocated is invalid
* if using the stateful API.
*/
- if ((f->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) &&
+ if (!dec_pdata->uses_stateless_api &&
+ (f->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) &&
vb2_is_busy(&ctx->m2m_ctx->out_q_ctx.q)) {
mtk_v4l2_err("out_q_ctx buffers already requested");
ret = -EBUSY;
@@ -457,6 +469,7 @@ static int vidioc_vdec_s_fmt(struct file *file, void *priv,
ctx->quantization = pix_mp->quantization;
ctx->xfer_func = pix_mp->xfer_func;
+ ctx->current_codec = fmt->fourcc;
if (ctx->state == MTK_STATE_FREE) {
ret = vdec_if_init(ctx, q_data->fmt->fourcc);
if (ret) {
@@ -468,6 +481,49 @@ static int vidioc_vdec_s_fmt(struct file *file, void *priv,
}
}
+ /*
+* If using the stateless API, S_FMT should have the effect of setting
+* the CAPTURE queue resolution no matter which queue it was called on.
+*/
+ if (dec_pdata->uses_stateless_api) {
+ ctx->picinfo.pic_w = pix_mp->width;
+ ctx->picinfo.pic_h = pix_mp->height;
+
+ ret = vdec_if_get_param(ctx, GET_PARAM_PIC_INFO, &ctx->picinfo);
+ if (ret) {
+ mtk_v4l2_err("[%d]Error!! Get GET_PARAM_PICTURE_INFO
Fail",
+ ctx->id);
+ return -EINVAL;
+ }
+
+ ctx->last_decoded_picinfo = ctx->picinfo;
+
+ if (ctx->q_data[MTK_Q_DATA_DST].fmt->num_planes == 1) {
+ ctx->q_data[MTK_Q_DATA_DST].sizeimage[0] =
+ ctx->picinfo.fb_sz[0] +
+ ctx->picinfo.fb_sz[1];
+ ctx->q_data[MTK_Q_DATA_DST].bytesperline[0] =
+ ctx->picinfo.buf_w;
+ } else {
+ ctx->q_data[MTK_Q_DATA_DST].sizeimage[0] =
+ ctx->picinfo.fb_sz[0];
+ ctx->q_data[MTK_Q_DATA_DST].bytesperline[0] =
+ ctx->picinfo.buf_w;
+ ctx->q_data[MTK_Q_DATA_DST].s
