| Message ID | 20211012143552.661751-1-benjamin.gaignard@collabora.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | media: HEVC: RPS clean up | expand |
Hi Benjamin! Dne torek, 12. oktober 2021 ob 16:35:48 CEST je Benjamin Gaignard napisal(a): > This series aims to clean up Reference Picture Set usage and flags. > > Long term flag was named with RPS prefix while it is not used for RPS > but for mark long term references in DBP. Remane it and remove the two > other useless RPS flags. > > Clarify documentation about RPS lists content and make sure that Hantro > driver use them correctly (i.e without look up in DBP). > > These patches are the last in my backlog impacting HEVC uAPI. > From my point of view, once they get merged, you could start talking > about how move HEVC uAPI to stable. With your changes, HEVC uAPI controls still won't be complete. Cedrus needs entry point control, which in turn needs dynamic array support. I'm a bit lazy implementing that control, but I guess I can take a look in a month or so. rkvdec also needs more fields for HEVC. With patches collected here: https://github.com/LibreELEC/LibreELEC.tv/blob/master/projects/Rockchip/ patches/linux/default/linux-2001-v4l-wip-rkvdec-hevc.patch fluster HEVC test score is reportedly 121/135 (8-bit tests only). I would certainly wait with moving HEVC uAPI to stable. Best regards, Jernej > > version 2: > - change DPB field name from rps to flags > > Please note that the only purpose of commits 3 and 4 is to allow to test > G2 hardware block for IMX8MQ until a proper solution isuing power domain > can be found. Do not merge them. > > GStreamer HEVC plugin merge request can be found here: > https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/1079 > > With those piece of code fluster score is 77/147. > > Benjamin > > Benjamin Gaignard (4): > media: hevc: Remove RPS named flags > media: hevc: Embedded indexes in RPS > media: hantro: Use syscon instead of 'ctrl' register > arm64: dts: imx8mq: Add node to G2 hardware > > .../media/v4l/ext-ctrls-codec.rst | 14 +++--- > arch/arm64/boot/dts/freescale/imx8mq.dtsi | 43 +++++++++++++---- > drivers/staging/media/hantro/hantro.h | 5 +- > .../staging/media/hantro/hantro_g2_hevc_dec.c | 27 +++-------- > drivers/staging/media/hantro/imx8m_vpu_hw.c | 48 ++++++++++++------- > .../staging/media/sunxi/cedrus/cedrus_h265.c | 2 +- > include/media/hevc-ctrls.h | 6 +-- > 7 files changed, 84 insertions(+), 61 deletions(-) > > -- > 2.30.2 > >
Le 12/10/2021 à 17:34, Jernej Škrabec a écrit : > Hi Benjamin! > > Dne torek, 12. oktober 2021 ob 16:35:48 CEST je Benjamin Gaignard napisal(a): >> This series aims to clean up Reference Picture Set usage and flags. >> >> Long term flag was named with RPS prefix while it is not used for RPS >> but for mark long term references in DBP. Remane it and remove the two >> other useless RPS flags. >> >> Clarify documentation about RPS lists content and make sure that Hantro >> driver use them correctly (i.e without look up in DBP). >> >> These patches are the last in my backlog impacting HEVC uAPI. >> From my point of view, once they get merged, you could start talking >> about how move HEVC uAPI to stable. > With your changes, HEVC uAPI controls still won't be complete. Cedrus needs > entry point control, which in turn needs dynamic array support. I'm a bit lazy > implementing that control, but I guess I can take a look in a month or so. > rkvdec also needs more fields for HEVC. With patches collected here: > https://github.com/LibreELEC/LibreELEC.tv/blob/master/projects/Rockchip/ > patches/linux/default/linux-2001-v4l-wip-rkvdec-hevc.patch > fluster HEVC test score is reportedly 121/135 (8-bit tests only). Hi Jernej, Thanks for your feedback, getting a list of missing items in HEVC uAPI will definitively help to fill the hope. The patch you mention for rkvdec are already merged in mainline kernel (at least for uAPI part). Cedrus needs are about num_entry_point_offsets, offset_len_minus1 and entry_point_offset_minus1[ i ] in HEVC specifications ? Regards, Benjamin > > I would certainly wait with moving HEVC uAPI to stable. > > Best regards, > Jernej > >> version 2: >> - change DPB field name from rps to flags >> >> Please note that the only purpose of commits 3 and 4 is to allow to test >> G2 hardware block for IMX8MQ until a proper solution isuing power domain >> can be found. Do not merge them. >> >> GStreamer HEVC plugin merge request can be found here: >> https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/1079 >> >> With those piece of code fluster score is 77/147. >> >> Benjamin >> >> Benjamin Gaignard (4): >> media: hevc: Remove RPS named flags >> media: hevc: Embedded indexes in RPS >> media: hantro: Use syscon instead of 'ctrl' register >> arm64: dts: imx8mq: Add node to G2 hardware >> >> .../media/v4l/ext-ctrls-codec.rst | 14 +++--- >> arch/arm64/boot/dts/freescale/imx8mq.dtsi | 43 +++++++++++++---- >> drivers/staging/media/hantro/hantro.h | 5 +- >> .../staging/media/hantro/hantro_g2_hevc_dec.c | 27 +++-------- >> drivers/staging/media/hantro/imx8m_vpu_hw.c | 48 ++++++++++++------- >> .../staging/media/sunxi/cedrus/cedrus_h265.c | 2 +- >> include/media/hevc-ctrls.h | 6 +-- >> 7 files changed, 84 insertions(+), 61 deletions(-) >> >> -- >> 2.30.2 >> >> >
CC: Alex Bee Alex, please take a look to these patches too. Dne torek, 12. oktober 2021 ob 17:57:50 CEST je Benjamin Gaignard napisal(a): > > Le 12/10/2021 à 17:34, Jernej Škrabec a écrit : > > Hi Benjamin! > > > > Dne torek, 12. oktober 2021 ob 16:35:48 CEST je Benjamin Gaignard napisal(a): > >> This series aims to clean up Reference Picture Set usage and flags. > >> > >> Long term flag was named with RPS prefix while it is not used for RPS > >> but for mark long term references in DBP. Remane it and remove the two > >> other useless RPS flags. > >> > >> Clarify documentation about RPS lists content and make sure that Hantro > >> driver use them correctly (i.e without look up in DBP). > >> > >> These patches are the last in my backlog impacting HEVC uAPI. > >> From my point of view, once they get merged, you could start talking > >> about how move HEVC uAPI to stable. > > With your changes, HEVC uAPI controls still won't be complete. Cedrus needs > > entry point control, which in turn needs dynamic array support. I'm a bit lazy > > implementing that control, but I guess I can take a look in a month or so. > > rkvdec also needs more fields for HEVC. With patches collected here: > > https://github.com/LibreELEC/LibreELEC.tv/blob/master/projects/Rockchip/ > > patches/linux/default/linux-2001-v4l-wip-rkvdec-hevc.patch > > fluster HEVC test score is reportedly 121/135 (8-bit tests only). > > Hi Jernej, > > Thanks for your feedback, getting a list of missing items in HEVC uAPI > will definitively help to fill the hope. > The patch you mention for rkvdec are already merged in mainline kernel (at > least for uAPI part). Are they? What about: video_parameter_set_id seq_parameter_set_id pic_parameter_set_id short_term_ref_pic_set_size long_term_ref_pic_set_size At least I don't see them in linux-next. Maybe that information can be obtained in some other way? > Cedrus needs are about num_entry_point_offsets, offset_len_minus1 and entry_point_offset_minus1[ i ] > in HEVC specifications ? Yes, Cedrus needs to know whole list of entry points. I don't think we need to worry about offset_len_minus1, list could be pre-processed - just number of entry points and their values. Best regards, Jernej > > Regards, > Benjamin > > > > > I would certainly wait with moving HEVC uAPI to stable. > > > > Best regards, > > Jernej > > > >> version 2: > >> - change DPB field name from rps to flags > >> > >> Please note that the only purpose of commits 3 and 4 is to allow to test > >> G2 hardware block for IMX8MQ until a proper solution isuing power domain > >> can be found. Do not merge them. > >> > >> GStreamer HEVC plugin merge request can be found here: > >> https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/1079 > >> > >> With those piece of code fluster score is 77/147. > >> > >> Benjamin > >> > >> Benjamin Gaignard (4): > >> media: hevc: Remove RPS named flags > >> media: hevc: Embedded indexes in RPS > >> media: hantro: Use syscon instead of 'ctrl' register > >> arm64: dts: imx8mq: Add node to G2 hardware > >> > >> .../media/v4l/ext-ctrls-codec.rst | 14 +++--- > >> arch/arm64/boot/dts/freescale/imx8mq.dtsi | 43 +++++++++++++---- > >> drivers/staging/media/hantro/hantro.h | 5 +- > >> .../staging/media/hantro/hantro_g2_hevc_dec.c | 27 +++-------- > >> drivers/staging/media/hantro/imx8m_vpu_hw.c | 48 ++++++++++++------- > >> .../staging/media/sunxi/cedrus/cedrus_h265.c | 2 +- > >> include/media/hevc-ctrls.h | 6 +-- > >> 7 files changed, 84 insertions(+), 61 deletions(-) > >> > >> -- > >> 2.30.2 > >> > >> > > >
Hi Benjamin, Jernej Am 12.10.21 um 18:08 schrieb Jernej Škrabec: > CC: Alex Bee > > Alex, please take a look to these patches too. These patches don't remove anything that would be need for rkvdec hevc - but indeed - we need some more: https://github.com/LibreELEC/LibreELEC.tv/blob/master/projects/Rockchip/patches/linux/default/linux-2001-v4l-wip-rkvdec-hevc.patch#L242-L305 v4l2_ctrl_hevc_sps: __u8 video_parameter_set_id __u8 seq_parameter_set_id v4l2_ctrl_hevc_pps: __u8 pic_parameter_set_id __u16 short_term_ref_pic_set_size __u16 long_term_ref_pic_set_size As far as I can see, they are all part of the spec and should be therefore good to go in the uapi. As you might now, even rkvdec is a frame-based decoder, it doesn't fully parse slice headers in HW for HEVC and we need to set references in SW which requires looping over the slices. Downstream we have a hack to give num_slices in v4l2_ctrl_hevc_sps for doing that. That could fully go away, if V4L2_CID_MPEG_VIDEO_HEVC_SLICE_PARAMS could get dynamic array control support and would make upstreaming this a lot easier - as far as I'm concered this would be required for RPi HEVC decoder as well. As a last resort we could also implement a HW specifc control à la V4L2_CID_HANTRO_HEVC_SLICE_HEADER_SKIP - but I'd like to avoid that, knowing it would certainly be better from performance pov. Alex. > > Dne torek, 12. oktober 2021 ob 17:57:50 CEST je Benjamin Gaignard napisal(a): >> >> Le 12/10/2021 à 17:34, Jernej Škrabec a écrit : >>> Hi Benjamin! >>> >>> Dne torek, 12. oktober 2021 ob 16:35:48 CEST je Benjamin Gaignard > napisal(a): >>>> This series aims to clean up Reference Picture Set usage and flags. >>>> >>>> Long term flag was named with RPS prefix while it is not used for RPS >>>> but for mark long term references in DBP. Remane it and remove the two >>>> other useless RPS flags. >>>> >>>> Clarify documentation about RPS lists content and make sure that Hantro >>>> driver use them correctly (i.e without look up in DBP). >>>> >>>> These patches are the last in my backlog impacting HEVC uAPI. >>>> From my point of view, once they get merged, you could start talking >>>> about how move HEVC uAPI to stable. >>> With your changes, HEVC uAPI controls still won't be complete. Cedrus > needs >>> entry point control, which in turn needs dynamic array support. I'm a bit > lazy >>> implementing that control, but I guess I can take a look in a month or so. >>> rkvdec also needs more fields for HEVC. With patches collected here: >>> https://github.com/LibreELEC/LibreELEC.tv/blob/master/projects/Rockchip/ >>> patches/linux/default/linux-2001-v4l-wip-rkvdec-hevc.patch >>> fluster HEVC test score is reportedly 121/135 (8-bit tests only). >> >> Hi Jernej, >> >> Thanks for your feedback, getting a list of missing items in HEVC uAPI >> will definitively help to fill the hope. >> The patch you mention for rkvdec are already merged in mainline kernel (at >> least for uAPI part). > > Are they? What about: > video_parameter_set_id > seq_parameter_set_id > pic_parameter_set_id > short_term_ref_pic_set_size > long_term_ref_pic_set_size > > At least I don't see them in linux-next. Maybe that information can be > obtained in some other way? > >> Cedrus needs are about num_entry_point_offsets, offset_len_minus1 and > entry_point_offset_minus1[ i ] >> in HEVC specifications ? > > Yes, Cedrus needs to know whole list of entry points. I don't think we need to > worry about offset_len_minus1, list could be pre-processed - just number of > entry points and their values. > > Best regards, > Jernej > >> >> Regards, >> Benjamin >> >>> >>> I would certainly wait with moving HEVC uAPI to stable. >>> >>> Best regards, >>> Jernej >>> >>>> version 2: >>>> - change DPB field name from rps to flags >>>> >>>> Please note that the only purpose of commits 3 and 4 is to allow to test >>>> G2 hardware block for IMX8MQ until a proper solution isuing power domain >>>> can be found. Do not merge them. >>>> >>>> GStreamer HEVC plugin merge request can be found here: >>>> https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/1079 >>>> >>>> With those piece of code fluster score is 77/147. >>>> >>>> Benjamin >>>> >>>> Benjamin Gaignard (4): >>>> media: hevc: Remove RPS named flags >>>> media: hevc: Embedded indexes in RPS >>>> media: hantro: Use syscon instead of 'ctrl' register >>>> arm64: dts: imx8mq: Add node to G2 hardware >>>> >>>> .../media/v4l/ext-ctrls-codec.rst | 14 +++--- >>>> arch/arm64/boot/dts/freescale/imx8mq.dtsi | 43 +++++++++++++---- >>>> drivers/staging/media/hantro/hantro.h | 5 +- >>>> .../staging/media/hantro/hantro_g2_hevc_dec.c | 27 +++-------- >>>> drivers/staging/media/hantro/imx8m_vpu_hw.c | 48 ++++++++++++------- >>>> .../staging/media/sunxi/cedrus/cedrus_h265.c | 2 +- >>>> include/media/hevc-ctrls.h | 6 +-- >>>> 7 files changed, 84 insertions(+), 61 deletions(-) >>>> >>>> -- >>>> 2.30.2 >>>> >>>> >>> >> > >
Le 15/10/2021 à 12:33, Alex Bee a écrit : > Hi Benjamin, Jernej > Am 12.10.21 um 18:08 schrieb Jernej Škrabec: >> CC: Alex Bee >> >> Alex, please take a look to these patches too. > These patches don't remove anything that would be need for rkvdec hevc > - but indeed - we need some more: > https://github.com/LibreELEC/LibreELEC.tv/blob/master/projects/Rockchip/patches/linux/default/linux-2001-v4l-wip-rkvdec-hevc.patch#L242-L305 > > > v4l2_ctrl_hevc_sps: > __u8 video_parameter_set_id > __u8 seq_parameter_set_id > > v4l2_ctrl_hevc_pps: > __u8 pic_parameter_set_id > __u16 short_term_ref_pic_set_size > __u16 long_term_ref_pic_set_size > > As far as I can see, they are all part of the spec and should be > therefore good to go in the uapi. Do you have any plan to upstream these fields in HEVC uAPI ? Regards, Benjamin > > As you might now, even rkvdec is a frame-based decoder, it doesn't > fully parse slice headers in HW for HEVC and we need to set references > in SW which requires looping over the slices. Downstream we have a > hack to give num_slices in v4l2_ctrl_hevc_sps for doing that. > That could fully go away, if V4L2_CID_MPEG_VIDEO_HEVC_SLICE_PARAMS > could get dynamic array control support and would make upstreaming > this a lot easier - as far as I'm concered this would be required for > RPi HEVC decoder as well. > As a last resort we could also implement a HW specifc control à la > V4L2_CID_HANTRO_HEVC_SLICE_HEADER_SKIP - but I'd like to avoid that, > knowing it would certainly be better from performance pov. > > Alex. >> >> Dne torek, 12. oktober 2021 ob 17:57:50 CEST je Benjamin Gaignard >> napisal(a): >>> >>> Le 12/10/2021 à 17:34, Jernej Škrabec a écrit : >>>> Hi Benjamin! >>>> >>>> Dne torek, 12. oktober 2021 ob 16:35:48 CEST je Benjamin Gaignard >> napisal(a): >>>>> This series aims to clean up Reference Picture Set usage and flags. >>>>> >>>>> Long term flag was named with RPS prefix while it is not used for RPS >>>>> but for mark long term references in DBP. Remane it and remove the >>>>> two >>>>> other useless RPS flags. >>>>> >>>>> Clarify documentation about RPS lists content and make sure that >>>>> Hantro >>>>> driver use them correctly (i.e without look up in DBP). >>>>> >>>>> These patches are the last in my backlog impacting HEVC uAPI. >>>>> From my point of view, once they get merged, you could start >>>>> talking >>>>> about how move HEVC uAPI to stable. >>>> With your changes, HEVC uAPI controls still won't be complete. Cedrus >> needs >>>> entry point control, which in turn needs dynamic array support. I'm >>>> a bit >> lazy >>>> implementing that control, but I guess I can take a look in a month >>>> or so. >>>> rkvdec also needs more fields for HEVC. With patches collected here: >>>> https://github.com/LibreELEC/LibreELEC.tv/blob/master/projects/Rockchip/ >>>> >>>> patches/linux/default/linux-2001-v4l-wip-rkvdec-hevc.patch >>>> fluster HEVC test score is reportedly 121/135 (8-bit tests only). >>> >>> Hi Jernej, >>> >>> Thanks for your feedback, getting a list of missing items in HEVC uAPI >>> will definitively help to fill the hope. >>> The patch you mention for rkvdec are already merged in mainline >>> kernel (at >>> least for uAPI part). >> >> Are they? What about: >> video_parameter_set_id >> seq_parameter_set_id >> pic_parameter_set_id >> short_term_ref_pic_set_size >> long_term_ref_pic_set_size >> >> At least I don't see them in linux-next. Maybe that information can be >> obtained in some other way? >> >>> Cedrus needs are about num_entry_point_offsets, offset_len_minus1 and >> entry_point_offset_minus1[ i ] >>> in HEVC specifications ? >> >> Yes, Cedrus needs to know whole list of entry points. I don't think >> we need to >> worry about offset_len_minus1, list could be pre-processed - just >> number of >> entry points and their values. >> >> Best regards, >> Jernej >> >>> >>> Regards, >>> Benjamin >>> >>>> >>>> I would certainly wait with moving HEVC uAPI to stable. >>>> >>>> Best regards, >>>> Jernej >>>> >>>>> version 2: >>>>> - change DPB field name from rps to flags >>>>> >>>>> Please note that the only purpose of commits 3 and 4 is to allow >>>>> to test >>>>> G2 hardware block for IMX8MQ until a proper solution isuing power >>>>> domain >>>>> can be found. Do not merge them. >>>>> >>>>> GStreamer HEVC plugin merge request can be found here: >>>>> https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/1079 >>>>> >>>>> >>>>> With those piece of code fluster score is 77/147. >>>>> >>>>> Benjamin >>>>> >>>>> Benjamin Gaignard (4): >>>>> media: hevc: Remove RPS named flags >>>>> media: hevc: Embedded indexes in RPS >>>>> media: hantro: Use syscon instead of 'ctrl' register >>>>> arm64: dts: imx8mq: Add node to G2 hardware >>>>> >>>>> .../media/v4l/ext-ctrls-codec.rst | 14 +++--- >>>>> arch/arm64/boot/dts/freescale/imx8mq.dtsi | 43 >>>>> +++++++++++++---- >>>>> drivers/staging/media/hantro/hantro.h | 5 +- >>>>> .../staging/media/hantro/hantro_g2_hevc_dec.c | 27 +++-------- >>>>> drivers/staging/media/hantro/imx8m_vpu_hw.c | 48 >>>>> ++++++++++++------- >>>>> .../staging/media/sunxi/cedrus/cedrus_h265.c | 2 +- >>>>> include/media/hevc-ctrls.h | 6 +-- >>>>> 7 files changed, 84 insertions(+), 61 deletions(-) >>>>> >>>>> -- >>>>> 2.30.2 >>>>> >>>>> >>>> >>> >> >> >
Am 15.10.21 um 16:06 schrieb Benjamin Gaignard: > > Le 15/10/2021 à 12:33, Alex Bee a écrit : >> Hi Benjamin, Jernej >> Am 12.10.21 um 18:08 schrieb Jernej Škrabec: >>> CC: Alex Bee >>> >>> Alex, please take a look to these patches too. >> These patches don't remove anything that would be need for rkvdec >> hevc - but indeed - we need some more: >> https://github.com/LibreELEC/LibreELEC.tv/blob/master/projects/Rockchip/patches/linux/default/linux-2001-v4l-wip-rkvdec-hevc.patch#L242-L305 >> >> >> v4l2_ctrl_hevc_sps: >> __u8 video_parameter_set_id >> __u8 seq_parameter_set_id >> >> v4l2_ctrl_hevc_pps: >> __u8 pic_parameter_set_id >> __u16 short_term_ref_pic_set_size >> __u16 long_term_ref_pic_set_size >> >> As far as I can see, they are all part of the spec and should be >> therefore good to go in the uapi. > > Do you have any plan to upstream these fields in HEVC uAPI ? I might be upstreaming them at some point, yes. With this I just wanted to underline Jernej said: HEVC uapi is NOT ready to get unstaged yet. > > Regards, > Benjamin > >> >> As you might now, even rkvdec is a frame-based decoder, it doesn't >> fully parse slice headers in HW for HEVC and we need to set >> references in SW which requires looping over the slices. Downstream >> we have a hack to give num_slices in v4l2_ctrl_hevc_sps for doing that. >> That could fully go away, if V4L2_CID_MPEG_VIDEO_HEVC_SLICE_PARAMS >> could get dynamic array control support and would make upstreaming >> this a lot easier - as far as I'm concered this would be required for >> RPi HEVC decoder as well. >> As a last resort we could also implement a HW specifc control à la >> V4L2_CID_HANTRO_HEVC_SLICE_HEADER_SKIP - but I'd like to avoid that, >> knowing it would certainly be better from performance pov. >> >> Alex. >>> >>> Dne torek, 12. oktober 2021 ob 17:57:50 CEST je Benjamin Gaignard >>> napisal(a): >>>> >>>> Le 12/10/2021 à 17:34, Jernej Škrabec a écrit : >>>>> Hi Benjamin! >>>>> >>>>> Dne torek, 12. oktober 2021 ob 16:35:48 CEST je Benjamin Gaignard >>> napisal(a): >>>>>> This series aims to clean up Reference Picture Set usage and flags. >>>>>> >>>>>> Long term flag was named with RPS prefix while it is not used for >>>>>> RPS >>>>>> but for mark long term references in DBP. Remane it and remove >>>>>> the two >>>>>> other useless RPS flags. >>>>>> >>>>>> Clarify documentation about RPS lists content and make sure that >>>>>> Hantro >>>>>> driver use them correctly (i.e without look up in DBP). >>>>>> >>>>>> These patches are the last in my backlog impacting HEVC uAPI. >>>>>> From my point of view, once they get merged, you could start >>>>>> talking >>>>>> about how move HEVC uAPI to stable. >>>>> With your changes, HEVC uAPI controls still won't be complete. Cedrus >>> needs >>>>> entry point control, which in turn needs dynamic array support. >>>>> I'm a bit >>> lazy >>>>> implementing that control, but I guess I can take a look in a >>>>> month or so. >>>>> rkvdec also needs more fields for HEVC. With patches collected here: >>>>> https://github.com/LibreELEC/LibreELEC.tv/blob/master/projects/Rockchip/ >>>>> >>>>> patches/linux/default/linux-2001-v4l-wip-rkvdec-hevc.patch >>>>> fluster HEVC test score is reportedly 121/135 (8-bit tests only). >>>> >>>> Hi Jernej, >>>> >>>> Thanks for your feedback, getting a list of missing items in HEVC uAPI >>>> will definitively help to fill the hope. >>>> The patch you mention for rkvdec are already merged in mainline >>>> kernel (at >>>> least for uAPI part). >>> >>> Are they? What about: >>> video_parameter_set_id >>> seq_parameter_set_id >>> pic_parameter_set_id >>> short_term_ref_pic_set_size >>> long_term_ref_pic_set_size >>> >>> At least I don't see them in linux-next. Maybe that information can be >>> obtained in some other way? >>> >>>> Cedrus needs are about num_entry_point_offsets, offset_len_minus1 and >>> entry_point_offset_minus1[ i ] >>>> in HEVC specifications ? >>> >>> Yes, Cedrus needs to know whole list of entry points. I don't think >>> we need to >>> worry about offset_len_minus1, list could be pre-processed - just >>> number of >>> entry points and their values. >>> >>> Best regards, >>> Jernej >>> >>>> >>>> Regards, >>>> Benjamin >>>> >>>>> >>>>> I would certainly wait with moving HEVC uAPI to stable. >>>>> >>>>> Best regards, >>>>> Jernej >>>>> >>>>>> version 2: >>>>>> - change DPB field name from rps to flags >>>>>> >>>>>> Please note that the only purpose of commits 3 and 4 is to allow >>>>>> to test >>>>>> G2 hardware block for IMX8MQ until a proper solution isuing power >>>>>> domain >>>>>> can be found. Do not merge them. >>>>>> >>>>>> GStreamer HEVC plugin merge request can be found here: >>>>>> https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/1079 >>>>>> >>>>>> >>>>>> With those piece of code fluster score is 77/147. >>>>>> >>>>>> Benjamin >>>>>> >>>>>> Benjamin Gaignard (4): >>>>>> media: hevc: Remove RPS named flags >>>>>> media: hevc: Embedded indexes in RPS >>>>>> media: hantro: Use syscon instead of 'ctrl' register >>>>>> arm64: dts: imx8mq: Add node to G2 hardware >>>>>> >>>>>> .../media/v4l/ext-ctrls-codec.rst | 14 +++--- >>>>>> arch/arm64/boot/dts/freescale/imx8mq.dtsi | 43 >>>>>> +++++++++++++---- >>>>>> drivers/staging/media/hantro/hantro.h | 5 +- >>>>>> .../staging/media/hantro/hantro_g2_hevc_dec.c | 27 +++-------- >>>>>> drivers/staging/media/hantro/imx8m_vpu_hw.c | 48 >>>>>> ++++++++++++------- >>>>>> .../staging/media/sunxi/cedrus/cedrus_h265.c | 2 +- >>>>>> include/media/hevc-ctrls.h | 6 +-- >>>>>> 7 files changed, 84 insertions(+), 61 deletions(-) >>>>>> >>>>>> -- >>>>>> 2.30.2 >>>>>> >>>>>> >>>>> >>>> >>> >>> >>
Le 15/10/2021 à 16:14, Alex Bee a écrit : > > Am 15.10.21 um 16:06 schrieb Benjamin Gaignard: >> >> Le 15/10/2021 à 12:33, Alex Bee a écrit : >>> Hi Benjamin, Jernej >>> Am 12.10.21 um 18:08 schrieb Jernej Škrabec: >>>> CC: Alex Bee >>>> >>>> Alex, please take a look to these patches too. >>> These patches don't remove anything that would be need for rkvdec >>> hevc - but indeed - we need some more: >>> https://github.com/LibreELEC/LibreELEC.tv/blob/master/projects/Rockchip/patches/linux/default/linux-2001-v4l-wip-rkvdec-hevc.patch#L242-L305 >>> >>> >>> v4l2_ctrl_hevc_sps: >>> __u8 video_parameter_set_id >>> __u8 seq_parameter_set_id >>> >>> v4l2_ctrl_hevc_pps: >>> __u8 pic_parameter_set_id >>> __u16 short_term_ref_pic_set_size >>> __u16 long_term_ref_pic_set_size >>> >>> As far as I can see, they are all part of the spec and should be >>> therefore good to go in the uapi. >> >> Do you have any plan to upstream these fields in HEVC uAPI ? > > I might be upstreaming them at some point, yes. > > With this I just wanted to underline Jernej said: HEVC uapi is NOT ready > to get unstaged yet. Ok but the question is how to get it unstaged ? Should we continue to do changes in staged uAPI ? or send a RFC to move it to stable and review all the missing parts at that time ? Regards, Benjamin > >> >> Regards, >> Benjamin >> >>> >>> As you might now, even rkvdec is a frame-based decoder, it doesn't >>> fully parse slice headers in HW for HEVC and we need to set >>> references in SW which requires looping over the slices. Downstream >>> we have a hack to give num_slices in v4l2_ctrl_hevc_sps for doing that. >>> That could fully go away, if V4L2_CID_MPEG_VIDEO_HEVC_SLICE_PARAMS >>> could get dynamic array control support and would make upstreaming >>> this a lot easier - as far as I'm concered this would be required >>> for RPi HEVC decoder as well. >>> As a last resort we could also implement a HW specifc control à la >>> V4L2_CID_HANTRO_HEVC_SLICE_HEADER_SKIP - but I'd like to avoid that, >>> knowing it would certainly be better from performance pov. >>> >>> Alex. >>>> >>>> Dne torek, 12. oktober 2021 ob 17:57:50 CEST je Benjamin Gaignard >>>> napisal(a): >>>>> >>>>> Le 12/10/2021 à 17:34, Jernej Škrabec a écrit : >>>>>> Hi Benjamin! >>>>>> >>>>>> Dne torek, 12. oktober 2021 ob 16:35:48 CEST je Benjamin Gaignard >>>> napisal(a): >>>>>>> This series aims to clean up Reference Picture Set usage and flags. >>>>>>> >>>>>>> Long term flag was named with RPS prefix while it is not used >>>>>>> for RPS >>>>>>> but for mark long term references in DBP. Remane it and remove >>>>>>> the two >>>>>>> other useless RPS flags. >>>>>>> >>>>>>> Clarify documentation about RPS lists content and make sure that >>>>>>> Hantro >>>>>>> driver use them correctly (i.e without look up in DBP). >>>>>>> >>>>>>> These patches are the last in my backlog impacting HEVC uAPI. >>>>>>> From my point of view, once they get merged, you could start >>>>>>> talking >>>>>>> about how move HEVC uAPI to stable. >>>>>> With your changes, HEVC uAPI controls still won't be complete. >>>>>> Cedrus >>>> needs >>>>>> entry point control, which in turn needs dynamic array support. >>>>>> I'm a bit >>>> lazy >>>>>> implementing that control, but I guess I can take a look in a >>>>>> month or so. >>>>>> rkvdec also needs more fields for HEVC. With patches collected here: >>>>>> https://github.com/LibreELEC/LibreELEC.tv/blob/master/projects/Rockchip/ >>>>>> >>>>>> patches/linux/default/linux-2001-v4l-wip-rkvdec-hevc.patch >>>>>> fluster HEVC test score is reportedly 121/135 (8-bit tests only). >>>>> >>>>> Hi Jernej, >>>>> >>>>> Thanks for your feedback, getting a list of missing items in HEVC >>>>> uAPI >>>>> will definitively help to fill the hope. >>>>> The patch you mention for rkvdec are already merged in mainline >>>>> kernel (at >>>>> least for uAPI part). >>>> >>>> Are they? What about: >>>> video_parameter_set_id >>>> seq_parameter_set_id >>>> pic_parameter_set_id >>>> short_term_ref_pic_set_size >>>> long_term_ref_pic_set_size >>>> >>>> At least I don't see them in linux-next. Maybe that information can be >>>> obtained in some other way? >>>> >>>>> Cedrus needs are about num_entry_point_offsets, offset_len_minus1 and >>>> entry_point_offset_minus1[ i ] >>>>> in HEVC specifications ? >>>> >>>> Yes, Cedrus needs to know whole list of entry points. I don't think >>>> we need to >>>> worry about offset_len_minus1, list could be pre-processed - just >>>> number of >>>> entry points and their values. >>>> >>>> Best regards, >>>> Jernej >>>> >>>>> >>>>> Regards, >>>>> Benjamin >>>>> >>>>>> >>>>>> I would certainly wait with moving HEVC uAPI to stable. >>>>>> >>>>>> Best regards, >>>>>> Jernej >>>>>> >>>>>>> version 2: >>>>>>> - change DPB field name from rps to flags >>>>>>> >>>>>>> Please note that the only purpose of commits 3 and 4 is to allow >>>>>>> to test >>>>>>> G2 hardware block for IMX8MQ until a proper solution isuing >>>>>>> power domain >>>>>>> can be found. Do not merge them. >>>>>>> >>>>>>> GStreamer HEVC plugin merge request can be found here: >>>>>>> https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/1079 >>>>>>> >>>>>>> >>>>>>> With those piece of code fluster score is 77/147. >>>>>>> >>>>>>> Benjamin >>>>>>> >>>>>>> Benjamin Gaignard (4): >>>>>>> media: hevc: Remove RPS named flags >>>>>>> media: hevc: Embedded indexes in RPS >>>>>>> media: hantro: Use syscon instead of 'ctrl' register >>>>>>> arm64: dts: imx8mq: Add node to G2 hardware >>>>>>> >>>>>>> .../media/v4l/ext-ctrls-codec.rst | 14 +++--- >>>>>>> arch/arm64/boot/dts/freescale/imx8mq.dtsi | 43 >>>>>>> +++++++++++++---- >>>>>>> drivers/staging/media/hantro/hantro.h | 5 +- >>>>>>> .../staging/media/hantro/hantro_g2_hevc_dec.c | 27 +++-------- >>>>>>> drivers/staging/media/hantro/imx8m_vpu_hw.c | 48 >>>>>>> ++++++++++++------- >>>>>>> .../staging/media/sunxi/cedrus/cedrus_h265.c | 2 +- >>>>>>> include/media/hevc-ctrls.h | 6 +-- >>>>>>> 7 files changed, 84 insertions(+), 61 deletions(-) >>>>>>> >>>>>>> -- >>>>>>> 2.30.2 >>>>>>> >>>>>>> >>>>>> >>>>> >>>> >>>> >>>
Le 12/10/2021 à 16:35, Benjamin Gaignard a écrit : > This series aims to clean up Reference Picture Set usage and flags. > > Long term flag was named with RPS prefix while it is not used for RPS > but for mark long term references in DBP. Remane it and remove the two > other useless RPS flags. > > Clarify documentation about RPS lists content and make sure that Hantro > driver use them correctly (i.e without look up in DBP). This series generates feedback about what is missing to move HEVC uAPI out of staging but lack of remarks about the proposed changes themselves. Does someone has an opinion about this RPS flag clean up ? Regards, Benjamin > > These patches are the last in my backlog impacting HEVC uAPI. > From my point of view, once they get merged, you could start talking > about how move HEVC uAPI to stable. > > version 2: > - change DPB field name from rps to flags > > Please note that the only purpose of commits 3 and 4 is to allow to test > G2 hardware block for IMX8MQ until a proper solution isuing power domain > can be found. Do not merge them. > > GStreamer HEVC plugin merge request can be found here: > https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/1079 > > With those piece of code fluster score is 77/147. > > Benjamin > > Benjamin Gaignard (4): > media: hevc: Remove RPS named flags > media: hevc: Embedded indexes in RPS > media: hantro: Use syscon instead of 'ctrl' register > arm64: dts: imx8mq: Add node to G2 hardware > > .../media/v4l/ext-ctrls-codec.rst | 14 +++--- > arch/arm64/boot/dts/freescale/imx8mq.dtsi | 43 +++++++++++++---- > drivers/staging/media/hantro/hantro.h | 5 +- > .../staging/media/hantro/hantro_g2_hevc_dec.c | 27 +++-------- > drivers/staging/media/hantro/imx8m_vpu_hw.c | 48 ++++++++++++------- > .../staging/media/sunxi/cedrus/cedrus_h265.c | 2 +- > include/media/hevc-ctrls.h | 6 +-- > 7 files changed, 84 insertions(+), 61 deletions(-) >