| Message ID | 20191007145909.29979-1-mjourdan@baylibre.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | media: meson: vdec: Add compliant H264 support | expand |
On 10/7/19 4:59 PM, Maxime Jourdan wrote: > Hello, > > This patch series aims to bring H.264 support as well as compliance update > to the amlogic stateful video decoder driver. > > There is 1 issue that remains currently: > > - The following codepath had to be commented out from v4l2-compliance as > it led to stalling: > > if (node->codec_mask & STATEFUL_DECODER) { > struct v4l2_decoder_cmd cmd; > buffer buf_cap(m2m_q); > > memset(&cmd, 0, sizeof(cmd)); > cmd.cmd = V4L2_DEC_CMD_STOP; > > /* No buffers are queued, call STREAMON, then STOP */ > fail_on_test(node->streamon(q.g_type())); > fail_on_test(node->streamon(m2m_q.g_type())); > fail_on_test(doioctl(node, VIDIOC_DECODER_CMD, &cmd)); > > fail_on_test(buf_cap.querybuf(node, 0)); > fail_on_test(buf_cap.qbuf(node)); > fail_on_test(buf_cap.dqbuf(node)); > fail_on_test(!(buf_cap.g_flags() & V4L2_BUF_FLAG_LAST)); > for (unsigned p = 0; p < buf_cap.g_num_planes(); p++) > fail_on_test(buf_cap.g_bytesused(p)); > fail_on_test(node->streamoff(q.g_type())); > fail_on_test(node->streamoff(m2m_q.g_type())); > > /* Call STREAMON, queue one CAPTURE buffer, then STOP */ > fail_on_test(node->streamon(q.g_type())); > fail_on_test(node->streamon(m2m_q.g_type())); > fail_on_test(buf_cap.querybuf(node, 0)); > fail_on_test(buf_cap.qbuf(node)); > fail_on_test(doioctl(node, VIDIOC_DECODER_CMD, &cmd)); > > fail_on_test(buf_cap.dqbuf(node)); > fail_on_test(!(buf_cap.g_flags() & V4L2_BUF_FLAG_LAST)); > for (unsigned p = 0; p < buf_cap.g_num_planes(); p++) > fail_on_test(buf_cap.g_bytesused(p)); > fail_on_test(node->streamoff(q.g_type())); > fail_on_test(node->streamoff(m2m_q.g_type())); > } > > The reason for this is because the driver has a limitation where all > capturebuffers must be queued to the driver before STREAMON is effective. > The firmware needs to know in advance what all the buffers are before > starting to decode. > This limitation is enforced via q->min_buffers_needed. > As such, in this compliance codepath, STREAMON is never actually called > driver-side and there is a stall on fail_on_test(buf_cap.dqbuf(node)); That's interesting. I will have to look more closely at this. > > > One last detail: V4L2_FMT_FLAG_DYN_RESOLUTION is currently not recognized > by v4l2-compliance, so it was left out for the test. However, it is > present in the patch series. It is definitely recognized by v4l2-compliance. > > The second patch has 3 "Alignment should match open parenthesis" lines > where I preferred to keep them that way. > > Thanks Stanimir for sharing your HDR file creation tools, this was very > helpful :). > > Maxime > > # v4l2-compliance --stream-from-hdr test-25fps.h264.hdr -s250 > v4l2-compliance SHA: a162244d47d4bb01d0692da879dce5a070f118e7, 64 bits But this SHA isn't in the v4l-utils repo, so this makes me wonder where you got this repo from. Regards, Hans > > Compliance test for meson-vdec device /dev/video0: > > Driver Info: > Driver name : meson-vdec > Card type : Amlogic Video Decoder > Bus info : platform:meson-vdec > Driver version : 5.4.0 > Capabilities : 0x84204000 > Video Memory-to-Memory Multiplanar > Streaming > Extended Pix Format > Device Capabilities > Device Caps : 0x04204000 > Video Memory-to-Memory Multiplanar > Streaming > Extended Pix Format > Detected Stateful Decoder > > Required ioctls: > test VIDIOC_QUERYCAP: OK > > Allow for multiple opens: > test second /dev/video0 open: OK > test VIDIOC_QUERYCAP: OK > test VIDIOC_G/S_PRIORITY: OK > test for unlimited opens: OK > > Debug ioctls: > test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported) > test VIDIOC_LOG_STATUS: OK (Not Supported) > > Input ioctls: > test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported) > test VIDIOC_G/S_FREQUENCY: OK (Not Supported) > test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported) > test VIDIOC_ENUMAUDIO: OK (Not Supported) > test VIDIOC_G/S/ENUMINPUT: OK (Not Supported) > test VIDIOC_G/S_AUDIO: OK (Not Supported) > Inputs: 0 Audio Inputs: 0 Tuners: 0 > > Output ioctls: > test VIDIOC_G/S_MODULATOR: OK (Not Supported) > test VIDIOC_G/S_FREQUENCY: OK (Not Supported) > test VIDIOC_ENUMAUDOUT: OK (Not Supported) > test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported) > test VIDIOC_G/S_AUDOUT: OK (Not Supported) > Outputs: 0 Audio Outputs: 0 Modulators: 0 > > Input/Output configuration ioctls: > test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported) > test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported) > test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported) > test VIDIOC_G/S_EDID: OK (Not Supported) > > Control ioctls: > test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK > test VIDIOC_QUERYCTRL: OK > test VIDIOC_G/S_CTRL: OK > test VIDIOC_G/S/TRY_EXT_CTRLS: OK > test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK > test VIDIOC_G/S_JPEGCOMP: OK (Not Supported) > Standard Controls: 2 Private Controls: 0 > > Format ioctls: > test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK > test VIDIOC_G/S_PARM: OK (Not Supported) > test VIDIOC_G_FBUF: OK (Not Supported) > test VIDIOC_G_FMT: OK > test VIDIOC_TRY_FMT: OK > test VIDIOC_S_FMT: OK > test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported) > test Cropping: OK (Not Supported) > test Composing: OK (Not Supported) > test Scaling: OK > > Codec ioctls: > test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported) > test VIDIOC_G_ENC_INDEX: OK (Not Supported) > test VIDIOC_(TRY_)DECODER_CMD: OK > > Buffer ioctls: > test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK > test VIDIOC_EXPBUF: OK > test Requests: OK (Not Supported) > > Test input 0: > > Streaming ioctls: > test read/write: OK (Not Supported) > test blocking wait: OK > Video Capture Multiplanar: Captured 250 buffers > test MMAP (select): OK > Video Capture Multiplanar: Captured 250 buffers > test MMAP (epoll): OK > test USERPTR (select): OK (Not Supported) > test DMABUF: Cannot test, specify --expbuf-device > > Total for meson-vdec device /dev/video0: 49, Succeeded: 49, Failed: 0, Warnings: 0 > > Maxime Jourdan (2): > media: meson: vdec: bring up to compliance > media: meson: vdec: add H.264 decoding support > > drivers/staging/media/meson/vdec/Makefile | 2 +- > drivers/staging/media/meson/vdec/codec_h264.c | 482 ++++++++++++++++++ > drivers/staging/media/meson/vdec/codec_h264.h | 14 + > drivers/staging/media/meson/vdec/esparser.c | 34 +- > drivers/staging/media/meson/vdec/vdec.c | 70 ++- > drivers/staging/media/meson/vdec/vdec.h | 14 +- > .../staging/media/meson/vdec/vdec_helpers.c | 85 ++- > .../staging/media/meson/vdec/vdec_helpers.h | 6 +- > .../staging/media/meson/vdec/vdec_platform.c | 43 ++ > 9 files changed, 654 insertions(+), 96 deletions(-) > create mode 100644 drivers/staging/media/meson/vdec/codec_h264.c > create mode 100644 drivers/staging/media/meson/vdec/codec_h264.h >
On 07/10/2019 17:12, Hans Verkuil wrote: > On 10/7/19 4:59 PM, Maxime Jourdan wrote: >> Hello, >> >> This patch series aims to bring H.264 support as well as compliance update >> to the amlogic stateful video decoder driver. >> >> There is 1 issue that remains currently: >> >> - The following codepath had to be commented out from v4l2-compliance as >> it led to stalling: >> >> if (node->codec_mask & STATEFUL_DECODER) { >> struct v4l2_decoder_cmd cmd; >> buffer buf_cap(m2m_q); >> >> memset(&cmd, 0, sizeof(cmd)); >> cmd.cmd = V4L2_DEC_CMD_STOP; >> >> /* No buffers are queued, call STREAMON, then STOP */ >> fail_on_test(node->streamon(q.g_type())); >> fail_on_test(node->streamon(m2m_q.g_type())); >> fail_on_test(doioctl(node, VIDIOC_DECODER_CMD, &cmd)); >> >> fail_on_test(buf_cap.querybuf(node, 0)); >> fail_on_test(buf_cap.qbuf(node)); >> fail_on_test(buf_cap.dqbuf(node)); >> fail_on_test(!(buf_cap.g_flags() & V4L2_BUF_FLAG_LAST)); >> for (unsigned p = 0; p < buf_cap.g_num_planes(); p++) >> fail_on_test(buf_cap.g_bytesused(p)); >> fail_on_test(node->streamoff(q.g_type())); >> fail_on_test(node->streamoff(m2m_q.g_type())); >> >> /* Call STREAMON, queue one CAPTURE buffer, then STOP */ >> fail_on_test(node->streamon(q.g_type())); >> fail_on_test(node->streamon(m2m_q.g_type())); >> fail_on_test(buf_cap.querybuf(node, 0)); >> fail_on_test(buf_cap.qbuf(node)); >> fail_on_test(doioctl(node, VIDIOC_DECODER_CMD, &cmd)); >> >> fail_on_test(buf_cap.dqbuf(node)); >> fail_on_test(!(buf_cap.g_flags() & V4L2_BUF_FLAG_LAST)); >> for (unsigned p = 0; p < buf_cap.g_num_planes(); p++) >> fail_on_test(buf_cap.g_bytesused(p)); >> fail_on_test(node->streamoff(q.g_type())); >> fail_on_test(node->streamoff(m2m_q.g_type())); >> } >> >> The reason for this is because the driver has a limitation where all >> capturebuffers must be queued to the driver before STREAMON is effective. >> The firmware needs to know in advance what all the buffers are before >> starting to decode. >> This limitation is enforced via q->min_buffers_needed. >> As such, in this compliance codepath, STREAMON is never actually called >> driver-side and there is a stall on fail_on_test(buf_cap.dqbuf(node)); > > That's interesting. I will have to look more closely at this. > >> >> >> One last detail: V4L2_FMT_FLAG_DYN_RESOLUTION is currently not recognized >> by v4l2-compliance, so it was left out for the test. However, it is >> present in the patch series. > > It is definitely recognized by v4l2-compliance. > >> >> The second patch has 3 "Alignment should match open parenthesis" lines >> where I preferred to keep them that way. >> >> Thanks Stanimir for sharing your HDR file creation tools, this was very >> helpful :). >> >> Maxime >> >> # v4l2-compliance --stream-from-hdr test-25fps.h264.hdr -s250 >> v4l2-compliance SHA: a162244d47d4bb01d0692da879dce5a070f118e7, 64 bits > > But this SHA isn't in the v4l-utils repo, so this makes me wonder where you > got this repo from. > I am based off the hverkuil/vicodec branch. The SHA I am on is actually 05387265053bc6f9 ("test-media: add vicodec tests"), but it wasn't updated as I found out it requires a new bootstrap to refresh the SHA. Maybe some rebasing at some point got rid of a162244d. I started fresh and ran it again. As you can see, V4L2_FMT_FLAG_DYN_RESOLUTION is still problematic (removing it makes all the checks pass): ------------------------------- # v4l2-compliance --stream-from-hdr test-25fps.h264.hdr -s250 v4l2-compliance SHA: 05387265053bc6f9c8c98e112543adb28ae39cfa, 64 bits Compliance test for meson-vdec device /dev/video0: Driver Info: Driver name : meson-vdec Card type : Amlogic Video Decoder Bus info : platform:meson-vdec Driver version : 5.4.0 Capabilities : 0x84204000 Video Memory-to-Memory Multiplanar Streaming Extended Pix Format Device Capabilities Device Caps : 0x04204000 Video Memory-to-Memory Multiplanar Streaming Extended Pix Format Detected Stateful Decoder Required ioctls: test VIDIOC_QUERYCAP: OK Allow for multiple opens: test second /dev/video0 open: OK test VIDIOC_QUERYCAP: OK test VIDIOC_G/S_PRIORITY: OK test for unlimited opens: OK Debug ioctls: test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported) test VIDIOC_LOG_STATUS: OK (Not Supported) Input ioctls: test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported) test VIDIOC_G/S_FREQUENCY: OK (Not Supported) test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported) test VIDIOC_ENUMAUDIO: OK (Not Supported) test VIDIOC_G/S/ENUMINPUT: OK (Not Supported) test VIDIOC_G/S_AUDIO: OK (Not Supported) Inputs: 0 Audio Inputs: 0 Tuners: 0 Output ioctls: test VIDIOC_G/S_MODULATOR: OK (Not Supported) test VIDIOC_G/S_FREQUENCY: OK (Not Supported) test VIDIOC_ENUMAUDOUT: OK (Not Supported) test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported) test VIDIOC_G/S_AUDOUT: OK (Not Supported) Outputs: 0 Audio Outputs: 0 Modulators: 0 Input/Output configuration ioctls: test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported) test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported) test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported) test VIDIOC_G/S_EDID: OK (Not Supported) Control ioctls: test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK test VIDIOC_QUERYCTRL: OK test VIDIOC_G/S_CTRL: OK test VIDIOC_G/S/TRY_EXT_CTRLS: OK test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK test VIDIOC_G/S_JPEGCOMP: OK (Not Supported) Standard Controls: 2 Private Controls: 0 Format ioctls: fail: v4l2-test-formats.cpp(263): unknown flag 00000009 returned test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: FAIL test VIDIOC_G/S_PARM: OK (Not Supported) test VIDIOC_G_FBUF: OK (Not Supported) fail: v4l2-test-formats.cpp(457): pixelformat 34363248 (H264) for buftype 10 not reported by ENUM_FMT test VIDIOC_G_FMT: FAIL fail: v4l2-test-formats.cpp(457): pixelformat 34363248 (H264) for buftype 10 not reported by ENUM_FMT test VIDIOC_TRY_FMT: FAIL fail: v4l2-test-formats.cpp(457): pixelformat 3247504d (MPG2) for buftype 10 not reported by ENUM_FMT test VIDIOC_S_FMT: FAIL test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported) test Cropping: OK (Not Supported) test Composing: OK (Not Supported) test Scaling: OK Codec ioctls: test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported) test VIDIOC_G_ENC_INDEX: OK (Not Supported) test VIDIOC_(TRY_)DECODER_CMD: OK Buffer ioctls: test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK test VIDIOC_EXPBUF: OK test Requests: OK (Not Supported) Test input 0: Streaming ioctls: test read/write: OK (Not Supported) test blocking wait: OK Video Capture Multiplanar: Captured 250 buffers test MMAP (select): OK Video Capture Multiplanar: Captured 250 buffers test MMAP (epoll): OK test USERPTR (select): OK (Not Supported) test DMABUF: Cannot test, specify --expbuf-device Total for meson-vdec device /dev/video0: 49, Succeeded: 45, Failed: 4, Warnings: 0 ------------------------------- Should I be using another branch than vicodec ? > Regards, > > Hans >
On 10/7/19 6:24 PM, Maxime Jourdan wrote: > On 07/10/2019 17:12, Hans Verkuil wrote: >> On 10/7/19 4:59 PM, Maxime Jourdan wrote: >>> Hello, >>> >>> This patch series aims to bring H.264 support as well as compliance update >>> to the amlogic stateful video decoder driver. >>> >>> There is 1 issue that remains currently: >>> >>> - The following codepath had to be commented out from v4l2-compliance as >>> it led to stalling: >>> >>> if (node->codec_mask & STATEFUL_DECODER) { >>> struct v4l2_decoder_cmd cmd; >>> buffer buf_cap(m2m_q); >>> >>> memset(&cmd, 0, sizeof(cmd)); >>> cmd.cmd = V4L2_DEC_CMD_STOP; >>> >>> /* No buffers are queued, call STREAMON, then STOP */ >>> fail_on_test(node->streamon(q.g_type())); >>> fail_on_test(node->streamon(m2m_q.g_type())); >>> fail_on_test(doioctl(node, VIDIOC_DECODER_CMD, &cmd)); >>> >>> fail_on_test(buf_cap.querybuf(node, 0)); >>> fail_on_test(buf_cap.qbuf(node)); >>> fail_on_test(buf_cap.dqbuf(node)); >>> fail_on_test(!(buf_cap.g_flags() & V4L2_BUF_FLAG_LAST)); >>> for (unsigned p = 0; p < buf_cap.g_num_planes(); p++) >>> fail_on_test(buf_cap.g_bytesused(p)); >>> fail_on_test(node->streamoff(q.g_type())); >>> fail_on_test(node->streamoff(m2m_q.g_type())); >>> >>> /* Call STREAMON, queue one CAPTURE buffer, then STOP */ >>> fail_on_test(node->streamon(q.g_type())); >>> fail_on_test(node->streamon(m2m_q.g_type())); >>> fail_on_test(buf_cap.querybuf(node, 0)); >>> fail_on_test(buf_cap.qbuf(node)); >>> fail_on_test(doioctl(node, VIDIOC_DECODER_CMD, &cmd)); >>> >>> fail_on_test(buf_cap.dqbuf(node)); >>> fail_on_test(!(buf_cap.g_flags() & V4L2_BUF_FLAG_LAST)); >>> for (unsigned p = 0; p < buf_cap.g_num_planes(); p++) >>> fail_on_test(buf_cap.g_bytesused(p)); >>> fail_on_test(node->streamoff(q.g_type())); >>> fail_on_test(node->streamoff(m2m_q.g_type())); >>> } >>> >>> The reason for this is because the driver has a limitation where all >>> capturebuffers must be queued to the driver before STREAMON is effective. >>> The firmware needs to know in advance what all the buffers are before >>> starting to decode. >>> This limitation is enforced via q->min_buffers_needed. >>> As such, in this compliance codepath, STREAMON is never actually called >>> driver-side and there is a stall on fail_on_test(buf_cap.dqbuf(node)); >> >> That's interesting. I will have to look more closely at this. >> >>> >>> >>> One last detail: V4L2_FMT_FLAG_DYN_RESOLUTION is currently not recognized >>> by v4l2-compliance, so it was left out for the test. However, it is >>> present in the patch series. >> >> It is definitely recognized by v4l2-compliance. >> >>> >>> The second patch has 3 "Alignment should match open parenthesis" lines >>> where I preferred to keep them that way. >>> >>> Thanks Stanimir for sharing your HDR file creation tools, this was very >>> helpful :). >>> >>> Maxime >>> >>> # v4l2-compliance --stream-from-hdr test-25fps.h264.hdr -s250 >>> v4l2-compliance SHA: a162244d47d4bb01d0692da879dce5a070f118e7, 64 bits >> >> But this SHA isn't in the v4l-utils repo, so this makes me wonder where you >> got this repo from. >> > > I am based off the hverkuil/vicodec branch. The SHA I am on is actually 05387265053bc6f9 ("test-media: add vicodec tests"), but it wasn't updated as I found out it requires a new bootstrap to refresh > the SHA. Maybe some rebasing at some point got rid of a162244d. Don't use the hverkuil/vicodec branch. Everything there has been merged into the regular v4l-utils repo some time ago. So just clone git://linuxtv.org/v4l-utils.git and use that. Regards, Hans > > I started fresh and ran it again. As you can see, V4L2_FMT_FLAG_DYN_RESOLUTION is still problematic (removing it makes all the checks pass): > > ------------------------------- > # v4l2-compliance --stream-from-hdr test-25fps.h264.hdr -s250 > v4l2-compliance SHA: 05387265053bc6f9c8c98e112543adb28ae39cfa, 64 bits > > Compliance test for meson-vdec device /dev/video0: > > Driver Info: > Driver name : meson-vdec > Card type : Amlogic Video Decoder > Bus info : platform:meson-vdec > Driver version : 5.4.0 > Capabilities : 0x84204000 > Video Memory-to-Memory Multiplanar > Streaming > Extended Pix Format > Device Capabilities > Device Caps : 0x04204000 > Video Memory-to-Memory Multiplanar > Streaming > Extended Pix Format > Detected Stateful Decoder > > Required ioctls: > test VIDIOC_QUERYCAP: OK > > Allow for multiple opens: > test second /dev/video0 open: OK > test VIDIOC_QUERYCAP: OK > test VIDIOC_G/S_PRIORITY: OK > test for unlimited opens: OK > > Debug ioctls: > test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported) > test VIDIOC_LOG_STATUS: OK (Not Supported) > > Input ioctls: > test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported) > test VIDIOC_G/S_FREQUENCY: OK (Not Supported) > test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported) > test VIDIOC_ENUMAUDIO: OK (Not Supported) > test VIDIOC_G/S/ENUMINPUT: OK (Not Supported) > test VIDIOC_G/S_AUDIO: OK (Not Supported) > Inputs: 0 Audio Inputs: 0 Tuners: 0 > > Output ioctls: > test VIDIOC_G/S_MODULATOR: OK (Not Supported) > test VIDIOC_G/S_FREQUENCY: OK (Not Supported) > test VIDIOC_ENUMAUDOUT: OK (Not Supported) > test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported) > test VIDIOC_G/S_AUDOUT: OK (Not Supported) > Outputs: 0 Audio Outputs: 0 Modulators: 0 > > Input/Output configuration ioctls: > test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported) > test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported) > test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported) > test VIDIOC_G/S_EDID: OK (Not Supported) > > Control ioctls: > test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK > test VIDIOC_QUERYCTRL: OK > test VIDIOC_G/S_CTRL: OK > test VIDIOC_G/S/TRY_EXT_CTRLS: OK > test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK > test VIDIOC_G/S_JPEGCOMP: OK (Not Supported) > Standard Controls: 2 Private Controls: 0 > > Format ioctls: > fail: v4l2-test-formats.cpp(263): unknown flag 00000009 returned > test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: FAIL > test VIDIOC_G/S_PARM: OK (Not Supported) > test VIDIOC_G_FBUF: OK (Not Supported) > fail: v4l2-test-formats.cpp(457): pixelformat 34363248 (H264) for buftype 10 not reported by ENUM_FMT > test VIDIOC_G_FMT: FAIL > fail: v4l2-test-formats.cpp(457): pixelformat 34363248 (H264) for buftype 10 not reported by ENUM_FMT > test VIDIOC_TRY_FMT: FAIL > fail: v4l2-test-formats.cpp(457): pixelformat 3247504d (MPG2) for buftype 10 not reported by ENUM_FMT > test VIDIOC_S_FMT: FAIL > test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported) > test Cropping: OK (Not Supported) > test Composing: OK (Not Supported) > test Scaling: OK > > Codec ioctls: > test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported) > test VIDIOC_G_ENC_INDEX: OK (Not Supported) > test VIDIOC_(TRY_)DECODER_CMD: OK > > Buffer ioctls: > test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK > test VIDIOC_EXPBUF: OK > test Requests: OK (Not Supported) > > Test input 0: > > Streaming ioctls: > test read/write: OK (Not Supported) > test blocking wait: OK > Video Capture Multiplanar: Captured 250 buffers > test MMAP (select): OK > Video Capture Multiplanar: Captured 250 buffers > test MMAP (epoll): OK > test USERPTR (select): OK (Not Supported) > test DMABUF: Cannot test, specify --expbuf-device > > Total for meson-vdec device /dev/video0: 49, Succeeded: 45, Failed: 4, Warnings: 0 > > ------------------------------- > > Should I be using another branch than vicodec ? > > >> Regards, >> >> Hans >>
On 07/10/2019 18:39, Hans Verkuil wrote: > On 10/7/19 6:24 PM, Maxime Jourdan wrote: >> On 07/10/2019 17:12, Hans Verkuil wrote: >>> On 10/7/19 4:59 PM, Maxime Jourdan wrote: >>>> Hello, >>>> >>>> This patch series aims to bring H.264 support as well as compliance update >>>> to the amlogic stateful video decoder driver. >>>> >>>> There is 1 issue that remains currently: >>>> >>>> - The following codepath had to be commented out from v4l2-compliance as >>>> it led to stalling: >>>> >>>> if (node->codec_mask & STATEFUL_DECODER) { >>>> struct v4l2_decoder_cmd cmd; >>>> buffer buf_cap(m2m_q); >>>> >>>> memset(&cmd, 0, sizeof(cmd)); >>>> cmd.cmd = V4L2_DEC_CMD_STOP; >>>> >>>> /* No buffers are queued, call STREAMON, then STOP */ >>>> fail_on_test(node->streamon(q.g_type())); >>>> fail_on_test(node->streamon(m2m_q.g_type())); >>>> fail_on_test(doioctl(node, VIDIOC_DECODER_CMD, &cmd)); >>>> >>>> fail_on_test(buf_cap.querybuf(node, 0)); >>>> fail_on_test(buf_cap.qbuf(node)); >>>> fail_on_test(buf_cap.dqbuf(node)); >>>> fail_on_test(!(buf_cap.g_flags() & V4L2_BUF_FLAG_LAST)); >>>> for (unsigned p = 0; p < buf_cap.g_num_planes(); p++) >>>> fail_on_test(buf_cap.g_bytesused(p)); >>>> fail_on_test(node->streamoff(q.g_type())); >>>> fail_on_test(node->streamoff(m2m_q.g_type())); >>>> >>>> /* Call STREAMON, queue one CAPTURE buffer, then STOP */ >>>> fail_on_test(node->streamon(q.g_type())); >>>> fail_on_test(node->streamon(m2m_q.g_type())); >>>> fail_on_test(buf_cap.querybuf(node, 0)); >>>> fail_on_test(buf_cap.qbuf(node)); >>>> fail_on_test(doioctl(node, VIDIOC_DECODER_CMD, &cmd)); >>>> >>>> fail_on_test(buf_cap.dqbuf(node)); >>>> fail_on_test(!(buf_cap.g_flags() & V4L2_BUF_FLAG_LAST)); >>>> for (unsigned p = 0; p < buf_cap.g_num_planes(); p++) >>>> fail_on_test(buf_cap.g_bytesused(p)); >>>> fail_on_test(node->streamoff(q.g_type())); >>>> fail_on_test(node->streamoff(m2m_q.g_type())); >>>> } >>>> >>>> The reason for this is because the driver has a limitation where all >>>> capturebuffers must be queued to the driver before STREAMON is effective. >>>> The firmware needs to know in advance what all the buffers are before >>>> starting to decode. >>>> This limitation is enforced via q->min_buffers_needed. >>>> As such, in this compliance codepath, STREAMON is never actually called >>>> driver-side and there is a stall on fail_on_test(buf_cap.dqbuf(node)); >>> >>> That's interesting. I will have to look more closely at this. >>> >>>> >>>> >>>> One last detail: V4L2_FMT_FLAG_DYN_RESOLUTION is currently not recognized >>>> by v4l2-compliance, so it was left out for the test. However, it is >>>> present in the patch series. >>> >>> It is definitely recognized by v4l2-compliance. >>> >>>> >>>> The second patch has 3 "Alignment should match open parenthesis" lines >>>> where I preferred to keep them that way. >>>> >>>> Thanks Stanimir for sharing your HDR file creation tools, this was very >>>> helpful :). >>>> >>>> Maxime >>>> >>>> # v4l2-compliance --stream-from-hdr test-25fps.h264.hdr -s250 >>>> v4l2-compliance SHA: a162244d47d4bb01d0692da879dce5a070f118e7, 64 bits >>> >>> But this SHA isn't in the v4l-utils repo, so this makes me wonder where you >>> got this repo from. >>> >> >> I am based off the hverkuil/vicodec branch. The SHA I am on is actually 05387265053bc6f9 ("test-media: add vicodec tests"), but it wasn't updated as I found out it requires a new bootstrap to refresh >> the SHA. Maybe some rebasing at some point got rid of a162244d. > > Don't use the hverkuil/vicodec branch. Everything there has been merged into the > regular v4l-utils repo some time ago. So just clone git://linuxtv.org/v4l-utils.git > and use that. > Here is v4l2-compliance master without removing the flag from the driver this time. I however had to keep the codepath mentionned earlier commented. # v4l2-compliance --stream-from-hdr test-25fps.h264.hdr -s250 v4l2-compliance SHA: fd74ecee9020fcf80b3b9628f277d9311b443395, 64 bits Compliance test for meson-vdec device /dev/video0: Driver Info: Driver name : meson-vdec Card type : Amlogic Video Decoder Bus info : platform:meson-vdec Driver version : 5.4.0 Capabilities : 0x84204000 Video Memory-to-Memory Multiplanar Streaming Extended Pix Format Device Capabilities Device Caps : 0x04204000 Video Memory-to-Memory Multiplanar Streaming Extended Pix Format Detected Stateful Decoder Required ioctls: test VIDIOC_QUERYCAP: OK Allow for multiple opens: test second /dev/video0 open: OK test VIDIOC_QUERYCAP: OK test VIDIOC_G/S_PRIORITY: OK test for unlimited opens: OK Debug ioctls: test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported) test VIDIOC_LOG_STATUS: OK (Not Supported) Input ioctls: test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported) test VIDIOC_G/S_FREQUENCY: OK (Not Supported) test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported) test VIDIOC_ENUMAUDIO: OK (Not Supported) test VIDIOC_G/S/ENUMINPUT: OK (Not Supported) test VIDIOC_G/S_AUDIO: OK (Not Supported) Inputs: 0 Audio Inputs: 0 Tuners: 0 Output ioctls: test VIDIOC_G/S_MODULATOR: OK (Not Supported) test VIDIOC_G/S_FREQUENCY: OK (Not Supported) test VIDIOC_ENUMAUDOUT: OK (Not Supported) test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported) test VIDIOC_G/S_AUDOUT: OK (Not Supported) Outputs: 0 Audio Outputs: 0 Modulators: 0 Input/Output configuration ioctls: test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported) test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported) test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported) test VIDIOC_G/S_EDID: OK (Not Supported) Control ioctls: test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK test VIDIOC_QUERYCTRL: OK test VIDIOC_G/S_CTRL: OK test VIDIOC_G/S/TRY_EXT_CTRLS: OK test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK test VIDIOC_G/S_JPEGCOMP: OK (Not Supported) Standard Controls: 2 Private Controls: 0 Format ioctls: test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK test VIDIOC_G/S_PARM: OK (Not Supported) test VIDIOC_G_FBUF: OK (Not Supported) test VIDIOC_G_FMT: OK test VIDIOC_TRY_FMT: OK test VIDIOC_S_FMT: OK test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported) test Cropping: OK (Not Supported) test Composing: OK (Not Supported) test Scaling: OK Codec ioctls: test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported) test VIDIOC_G_ENC_INDEX: OK (Not Supported) test VIDIOC_(TRY_)DECODER_CMD: OK Buffer ioctls: test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK test VIDIOC_EXPBUF: OK test Requests: OK (Not Supported) Test input 0: Streaming ioctls: test read/write: OK (Not Supported) test blocking wait: OK Video Capture Multiplanar: Captured 250 buffers test MMAP (select): OK Video Capture Multiplanar: Captured 250 buffers test MMAP (epoll): OK test USERPTR (select): OK (Not Supported) test DMABUF: Cannot test, specify --expbuf-device Total for meson-vdec device /dev/video0: 49, Succeeded: 49, Failed: 0, Warnings: 0 Maxime > > Regards, > > Hans >
On 10/8/19 3:40 PM, Maxime Jourdan wrote: > On 07/10/2019 18:39, Hans Verkuil wrote: >> On 10/7/19 6:24 PM, Maxime Jourdan wrote: >>> On 07/10/2019 17:12, Hans Verkuil wrote: >>>> On 10/7/19 4:59 PM, Maxime Jourdan wrote: >>>>> Hello, >>>>> >>>>> This patch series aims to bring H.264 support as well as compliance update >>>>> to the amlogic stateful video decoder driver. >>>>> >>>>> There is 1 issue that remains currently: >>>>> >>>>> - The following codepath had to be commented out from v4l2-compliance as >>>>> it led to stalling: >>>>> >>>>> if (node->codec_mask & STATEFUL_DECODER) { >>>>> struct v4l2_decoder_cmd cmd; >>>>> buffer buf_cap(m2m_q); >>>>> >>>>> memset(&cmd, 0, sizeof(cmd)); >>>>> cmd.cmd = V4L2_DEC_CMD_STOP; >>>>> >>>>> /* No buffers are queued, call STREAMON, then STOP */ >>>>> fail_on_test(node->streamon(q.g_type())); >>>>> fail_on_test(node->streamon(m2m_q.g_type())); >>>>> fail_on_test(doioctl(node, VIDIOC_DECODER_CMD, &cmd)); >>>>> >>>>> fail_on_test(buf_cap.querybuf(node, 0)); >>>>> fail_on_test(buf_cap.qbuf(node)); >>>>> fail_on_test(buf_cap.dqbuf(node)); >>>>> fail_on_test(!(buf_cap.g_flags() & V4L2_BUF_FLAG_LAST)); >>>>> for (unsigned p = 0; p < buf_cap.g_num_planes(); p++) >>>>> fail_on_test(buf_cap.g_bytesused(p)); >>>>> fail_on_test(node->streamoff(q.g_type())); >>>>> fail_on_test(node->streamoff(m2m_q.g_type())); >>>>> >>>>> /* Call STREAMON, queue one CAPTURE buffer, then STOP */ >>>>> fail_on_test(node->streamon(q.g_type())); >>>>> fail_on_test(node->streamon(m2m_q.g_type())); >>>>> fail_on_test(buf_cap.querybuf(node, 0)); >>>>> fail_on_test(buf_cap.qbuf(node)); >>>>> fail_on_test(doioctl(node, VIDIOC_DECODER_CMD, &cmd)); >>>>> >>>>> fail_on_test(buf_cap.dqbuf(node)); >>>>> fail_on_test(!(buf_cap.g_flags() & V4L2_BUF_FLAG_LAST)); >>>>> for (unsigned p = 0; p < buf_cap.g_num_planes(); p++) >>>>> fail_on_test(buf_cap.g_bytesused(p)); >>>>> fail_on_test(node->streamoff(q.g_type())); >>>>> fail_on_test(node->streamoff(m2m_q.g_type())); >>>>> } >>>>> >>>>> The reason for this is because the driver has a limitation where all >>>>> capturebuffers must be queued to the driver before STREAMON is effective. >>>>> The firmware needs to know in advance what all the buffers are before >>>>> starting to decode. >>>>> This limitation is enforced via q->min_buffers_needed. >>>>> As such, in this compliance codepath, STREAMON is never actually called >>>>> driver-side and there is a stall on fail_on_test(buf_cap.dqbuf(node)); >>>> >>>> That's interesting. I will have to look more closely at this. >>>> >>>>> >>>>> >>>>> One last detail: V4L2_FMT_FLAG_DYN_RESOLUTION is currently not recognized >>>>> by v4l2-compliance, so it was left out for the test. However, it is >>>>> present in the patch series. >>>> >>>> It is definitely recognized by v4l2-compliance. >>>> >>>>> >>>>> The second patch has 3 "Alignment should match open parenthesis" lines >>>>> where I preferred to keep them that way. >>>>> >>>>> Thanks Stanimir for sharing your HDR file creation tools, this was very >>>>> helpful :). >>>>> >>>>> Maxime >>>>> >>>>> # v4l2-compliance --stream-from-hdr test-25fps.h264.hdr -s250 >>>>> v4l2-compliance SHA: a162244d47d4bb01d0692da879dce5a070f118e7, 64 bits >>>> >>>> But this SHA isn't in the v4l-utils repo, so this makes me wonder where you >>>> got this repo from. >>>> >>> >>> I am based off the hverkuil/vicodec branch. The SHA I am on is actually 05387265053bc6f9 ("test-media: add vicodec tests"), but it wasn't updated as I found out it requires a new bootstrap to refresh >>> the SHA. Maybe some rebasing at some point got rid of a162244d. >> >> Don't use the hverkuil/vicodec branch. Everything there has been merged into the >> regular v4l-utils repo some time ago. So just clone git://linuxtv.org/v4l-utils.git >> and use that. >> > > Here is v4l2-compliance master without removing the flag from the driver this time. I however had to keep the codepath mentionned earlier commented. > > # v4l2-compliance --stream-from-hdr test-25fps.h264.hdr -s250 > v4l2-compliance SHA: fd74ecee9020fcf80b3b9628f277d9311b443395, 64 bits Ah, that looks much better :-) The stall on 'fail_on_test(buf_cap.dqbuf(node));' I understand, and is something I need to take a closer look at. Regards, Hans > > Compliance test for meson-vdec device /dev/video0: > > Driver Info: > Driver name : meson-vdec > Card type : Amlogic Video Decoder > Bus info : platform:meson-vdec > Driver version : 5.4.0 > Capabilities : 0x84204000 > Video Memory-to-Memory Multiplanar > Streaming > Extended Pix Format > Device Capabilities > Device Caps : 0x04204000 > Video Memory-to-Memory Multiplanar > Streaming > Extended Pix Format > Detected Stateful Decoder > > Required ioctls: > test VIDIOC_QUERYCAP: OK > > Allow for multiple opens: > test second /dev/video0 open: OK > test VIDIOC_QUERYCAP: OK > test VIDIOC_G/S_PRIORITY: OK > test for unlimited opens: OK > > Debug ioctls: > test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported) > test VIDIOC_LOG_STATUS: OK (Not Supported) > > Input ioctls: > test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported) > test VIDIOC_G/S_FREQUENCY: OK (Not Supported) > test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported) > test VIDIOC_ENUMAUDIO: OK (Not Supported) > test VIDIOC_G/S/ENUMINPUT: OK (Not Supported) > test VIDIOC_G/S_AUDIO: OK (Not Supported) > Inputs: 0 Audio Inputs: 0 Tuners: 0 > > Output ioctls: > test VIDIOC_G/S_MODULATOR: OK (Not Supported) > test VIDIOC_G/S_FREQUENCY: OK (Not Supported) > test VIDIOC_ENUMAUDOUT: OK (Not Supported) > test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported) > test VIDIOC_G/S_AUDOUT: OK (Not Supported) > Outputs: 0 Audio Outputs: 0 Modulators: 0 > > Input/Output configuration ioctls: > test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported) > test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported) > test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported) > test VIDIOC_G/S_EDID: OK (Not Supported) > > Control ioctls: > test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK > test VIDIOC_QUERYCTRL: OK > test VIDIOC_G/S_CTRL: OK > test VIDIOC_G/S/TRY_EXT_CTRLS: OK > test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK > test VIDIOC_G/S_JPEGCOMP: OK (Not Supported) > Standard Controls: 2 Private Controls: 0 > > Format ioctls: > test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK > test VIDIOC_G/S_PARM: OK (Not Supported) > test VIDIOC_G_FBUF: OK (Not Supported) > test VIDIOC_G_FMT: OK > test VIDIOC_TRY_FMT: OK > test VIDIOC_S_FMT: OK > test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported) > test Cropping: OK (Not Supported) > test Composing: OK (Not Supported) > test Scaling: OK > > Codec ioctls: > test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported) > test VIDIOC_G_ENC_INDEX: OK (Not Supported) > test VIDIOC_(TRY_)DECODER_CMD: OK > > Buffer ioctls: > test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK > test VIDIOC_EXPBUF: OK > test Requests: OK (Not Supported) > > Test input 0: > > Streaming ioctls: > test read/write: OK (Not Supported) > test blocking wait: OK > Video Capture Multiplanar: Captured 250 buffers > test MMAP (select): OK > Video Capture Multiplanar: Captured 250 buffers > test MMAP (epoll): OK > test USERPTR (select): OK (Not Supported) > test DMABUF: Cannot test, specify --expbuf-device > > Total for meson-vdec device /dev/video0: 49, Succeeded: 49, Failed: 0, Warnings: 0 > > Maxime > >> >> Regards, >> >> Hans >> >
On 10/8/19 3:40 PM, Maxime Jourdan wrote: > On 07/10/2019 18:39, Hans Verkuil wrote: >> On 10/7/19 6:24 PM, Maxime Jourdan wrote: >>> On 07/10/2019 17:12, Hans Verkuil wrote: >>>> On 10/7/19 4:59 PM, Maxime Jourdan wrote: >>>>> Hello, >>>>> >>>>> This patch series aims to bring H.264 support as well as compliance update >>>>> to the amlogic stateful video decoder driver. >>>>> >>>>> There is 1 issue that remains currently: >>>>> >>>>> - The following codepath had to be commented out from v4l2-compliance as >>>>> it led to stalling: >>>>> >>>>> if (node->codec_mask & STATEFUL_DECODER) { >>>>> struct v4l2_decoder_cmd cmd; >>>>> buffer buf_cap(m2m_q); >>>>> >>>>> memset(&cmd, 0, sizeof(cmd)); >>>>> cmd.cmd = V4L2_DEC_CMD_STOP; >>>>> >>>>> /* No buffers are queued, call STREAMON, then STOP */ >>>>> fail_on_test(node->streamon(q.g_type())); >>>>> fail_on_test(node->streamon(m2m_q.g_type())); >>>>> fail_on_test(doioctl(node, VIDIOC_DECODER_CMD, &cmd)); >>>>> >>>>> fail_on_test(buf_cap.querybuf(node, 0)); >>>>> fail_on_test(buf_cap.qbuf(node)); >>>>> fail_on_test(buf_cap.dqbuf(node)); >>>>> fail_on_test(!(buf_cap.g_flags() & V4L2_BUF_FLAG_LAST)); >>>>> for (unsigned p = 0; p < buf_cap.g_num_planes(); p++) >>>>> fail_on_test(buf_cap.g_bytesused(p)); >>>>> fail_on_test(node->streamoff(q.g_type())); >>>>> fail_on_test(node->streamoff(m2m_q.g_type())); >>>>> >>>>> /* Call STREAMON, queue one CAPTURE buffer, then STOP */ >>>>> fail_on_test(node->streamon(q.g_type())); >>>>> fail_on_test(node->streamon(m2m_q.g_type())); >>>>> fail_on_test(buf_cap.querybuf(node, 0)); >>>>> fail_on_test(buf_cap.qbuf(node)); >>>>> fail_on_test(doioctl(node, VIDIOC_DECODER_CMD, &cmd)); >>>>> >>>>> fail_on_test(buf_cap.dqbuf(node)); >>>>> fail_on_test(!(buf_cap.g_flags() & V4L2_BUF_FLAG_LAST)); >>>>> for (unsigned p = 0; p < buf_cap.g_num_planes(); p++) >>>>> fail_on_test(buf_cap.g_bytesused(p)); >>>>> fail_on_test(node->streamoff(q.g_type())); >>>>> fail_on_test(node->streamoff(m2m_q.g_type())); >>>>> } >>>>> >>>>> The reason for this is because the driver has a limitation where all >>>>> capturebuffers must be queued to the driver before STREAMON is effective. >>>>> The firmware needs to know in advance what all the buffers are before >>>>> starting to decode. >>>>> This limitation is enforced via q->min_buffers_needed. >>>>> As such, in this compliance codepath, STREAMON is never actually called >>>>> driver-side and there is a stall on fail_on_test(buf_cap.dqbuf(node)); >>>> >>>> That's interesting. I will have to look more closely at this. This requires a helper function in videobuf2-v4l2.c. In vdec_decoder_cmd you would need code like this: if (!vb2_start_streaming_called(&capture_queue)) { vb2_dequeue_empty_last_buf(&capture_queue); return 0; } The vb2_dequeue_empty_last_buf (function name can probably be improved upon!) does nothing if no capture buffers were queued, otherwise it takes the first buffer, sets the LAST flag and sets bytesused to 0 and marks it as DONE. The driver cannot do this directly, since the buffers were never queued to the driver and are owned by vb2. This is something that needs to be done for any codec driver that sets min_buffers_needed to a value > 1. The vb2 function would look something like this: void vb2_dqbuf_empty_last_buf(struct vb2_queue *q) { struct vb2_buffer *vb; struct vb2_v4l2_buffer *vbuf; unsigned int i; if (WARN_ON(q->is_output)) return; if (list_empty(&q->queued_list)) return; vb = list_first_entry(&q->queued_list, struct vb2_buffer, queued_entry); list_del(&vb->queued_entry); for (i = 0; i < vb->num_planes; i++) vb2_set_plane_payload(vb, i, 0) vbuf = to_vb2_v4l2_buffer(vb); vbuf->flags |= V4L2_BUF_FLAG_LAST; vb2_buffer_done(vb, VB2_BUF_STATE_DONE); } EXPORT_SYMBOL_GPL(vb2_dqbuf_empty_last_buf); Neither compiled, nor tested, and I think this should be in v4l2-mem2mem.c instead of in videobuf2-v4l2.c since this is very m2m specific. So see this as a suggestion :-) Anyway, the key take-away from this is that userspace does not know if your driver behaves the way it does, so STOP should still produce a sane expected result. Which in this is just a single empty capture buffer marked LAST. Regards, Hans
Le lundi 07 octobre 2019 à 16:59 +0200, Maxime Jourdan a écrit : > Hello, > > This patch series aims to bring H.264 support as well as compliance update > to the amlogic stateful video decoder driver. > > There is 1 issue that remains currently: > > - The following codepath had to be commented out from v4l2-compliance as > it led to stalling: > > if (node->codec_mask & STATEFUL_DECODER) { > struct v4l2_decoder_cmd cmd; > buffer buf_cap(m2m_q); > > memset(&cmd, 0, sizeof(cmd)); > cmd.cmd = V4L2_DEC_CMD_STOP; > > /* No buffers are queued, call STREAMON, then STOP */ > fail_on_test(node->streamon(q.g_type())); > fail_on_test(node->streamon(m2m_q.g_type())); > fail_on_test(doioctl(node, VIDIOC_DECODER_CMD, &cmd)); > > fail_on_test(buf_cap.querybuf(node, 0)); > fail_on_test(buf_cap.qbuf(node)); > fail_on_test(buf_cap.dqbuf(node)); > fail_on_test(!(buf_cap.g_flags() & V4L2_BUF_FLAG_LAST)); > for (unsigned p = 0; p < buf_cap.g_num_planes(); p++) > fail_on_test(buf_cap.g_bytesused(p)); > fail_on_test(node->streamoff(q.g_type())); > fail_on_test(node->streamoff(m2m_q.g_type())); > > /* Call STREAMON, queue one CAPTURE buffer, then STOP */ > fail_on_test(node->streamon(q.g_type())); > fail_on_test(node->streamon(m2m_q.g_type())); > fail_on_test(buf_cap.querybuf(node, 0)); > fail_on_test(buf_cap.qbuf(node)); > fail_on_test(doioctl(node, VIDIOC_DECODER_CMD, &cmd)); > > fail_on_test(buf_cap.dqbuf(node)); > fail_on_test(!(buf_cap.g_flags() & V4L2_BUF_FLAG_LAST)); > for (unsigned p = 0; p < buf_cap.g_num_planes(); p++) > fail_on_test(buf_cap.g_bytesused(p)); > fail_on_test(node->streamoff(q.g_type())); > fail_on_test(node->streamoff(m2m_q.g_type())); > } > > The reason for this is because the driver has a limitation where all > capturebuffers must be queued to the driver before STREAMON is effective. > The firmware needs to know in advance what all the buffers are before > starting to decode. > This limitation is enforced via q->min_buffers_needed. > As such, in this compliance codepath, STREAMON is never actually called > driver-side and there is a stall on fail_on_test(buf_cap.dqbuf(node)); > > > One last detail: V4L2_FMT_FLAG_DYN_RESOLUTION is currently not recognized > by v4l2-compliance, so it was left out for the test. However, it is > present in the patch series. > > The second patch has 3 "Alignment should match open parenthesis" lines > where I preferred to keep them that way. > > Thanks Stanimir for sharing your HDR file creation tools, this was very > helpful :). I tried to test this with a pending branch of GStreamer supporting dynamic resolution changes. The even driver mechanism does not seem to work with this driver. I've grepped the code, and don't see any places were the event would be emitted. Then I grepped, and it seems the driver accept source_change subscription but does not set V4L2_FMT_FLAG_DYN_RESOLUTION. I believe these two things are bit redundant and confusing, I'll fix the proposed patch never the less, and see if that makes it work. > > Maxime > > # v4l2-compliance --stream-from-hdr test-25fps.h264.hdr -s250 > v4l2-compliance SHA: a162244d47d4bb01d0692da879dce5a070f118e7, 64 bits > > Compliance test for meson-vdec device /dev/video0: > > Driver Info: > Driver name : meson-vdec > Card type : Amlogic Video Decoder > Bus info : platform:meson-vdec > Driver version : 5.4.0 > Capabilities : 0x84204000 > Video Memory-to-Memory Multiplanar > Streaming > Extended Pix Format > Device Capabilities > Device Caps : 0x04204000 > Video Memory-to-Memory Multiplanar > Streaming > Extended Pix Format > Detected Stateful Decoder > > Required ioctls: > test VIDIOC_QUERYCAP: OK > > Allow for multiple opens: > test second /dev/video0 open: OK > test VIDIOC_QUERYCAP: OK > test VIDIOC_G/S_PRIORITY: OK > test for unlimited opens: OK > > Debug ioctls: > test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported) > test VIDIOC_LOG_STATUS: OK (Not Supported) > > Input ioctls: > test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported) > test VIDIOC_G/S_FREQUENCY: OK (Not Supported) > test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported) > test VIDIOC_ENUMAUDIO: OK (Not Supported) > test VIDIOC_G/S/ENUMINPUT: OK (Not Supported) > test VIDIOC_G/S_AUDIO: OK (Not Supported) > Inputs: 0 Audio Inputs: 0 Tuners: 0 > > Output ioctls: > test VIDIOC_G/S_MODULATOR: OK (Not Supported) > test VIDIOC_G/S_FREQUENCY: OK (Not Supported) > test VIDIOC_ENUMAUDOUT: OK (Not Supported) > test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported) > test VIDIOC_G/S_AUDOUT: OK (Not Supported) > Outputs: 0 Audio Outputs: 0 Modulators: 0 > > Input/Output configuration ioctls: > test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported) > test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported) > test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported) > test VIDIOC_G/S_EDID: OK (Not Supported) > > Control ioctls: > test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK > test VIDIOC_QUERYCTRL: OK > test VIDIOC_G/S_CTRL: OK > test VIDIOC_G/S/TRY_EXT_CTRLS: OK > test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK > test VIDIOC_G/S_JPEGCOMP: OK (Not Supported) > Standard Controls: 2 Private Controls: 0 > > Format ioctls: > test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK > test VIDIOC_G/S_PARM: OK (Not Supported) > test VIDIOC_G_FBUF: OK (Not Supported) > test VIDIOC_G_FMT: OK > test VIDIOC_TRY_FMT: OK > test VIDIOC_S_FMT: OK > test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported) > test Cropping: OK (Not Supported) > test Composing: OK (Not Supported) > test Scaling: OK > > Codec ioctls: > test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported) > test VIDIOC_G_ENC_INDEX: OK (Not Supported) > test VIDIOC_(TRY_)DECODER_CMD: OK > > Buffer ioctls: > test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK > test VIDIOC_EXPBUF: OK > test Requests: OK (Not Supported) > > Test input 0: > > Streaming ioctls: > test read/write: OK (Not Supported) > test blocking wait: OK > Video Capture Multiplanar: Captured 250 buffers > test MMAP (select): OK > Video Capture Multiplanar: Captured 250 buffers > test MMAP (epoll): OK > test USERPTR (select): OK (Not Supported) > test DMABUF: Cannot test, specify --expbuf-device > > Total for meson-vdec device /dev/video0: 49, Succeeded: 49, Failed: 0, Warnings: 0 > > Maxime Jourdan (2): > media: meson: vdec: bring up to compliance > media: meson: vdec: add H.264 decoding support > > drivers/staging/media/meson/vdec/Makefile | 2 +- > drivers/staging/media/meson/vdec/codec_h264.c | 482 ++++++++++++++++++ > drivers/staging/media/meson/vdec/codec_h264.h | 14 + > drivers/staging/media/meson/vdec/esparser.c | 34 +- > drivers/staging/media/meson/vdec/vdec.c | 70 ++- > drivers/staging/media/meson/vdec/vdec.h | 14 +- > .../staging/media/meson/vdec/vdec_helpers.c | 85 ++- > .../staging/media/meson/vdec/vdec_helpers.h | 6 +- > .../staging/media/meson/vdec/vdec_platform.c | 43 ++ > 9 files changed, 654 insertions(+), 96 deletions(-) > create mode 100644 drivers/staging/media/meson/vdec/codec_h264.c > create mode 100644 drivers/staging/media/meson/vdec/codec_h264.h >
Hi Nicolas, On 13/10/2019 03:08, Nicolas Dufresne wrote: > Le lundi 07 octobre 2019 à 16:59 +0200, Maxime Jourdan a écrit : >> Hello, >> >> This patch series aims to bring H.264 support as well as compliance update >> to the amlogic stateful video decoder driver. >> >> There is 1 issue that remains currently: >> >> - The following codepath had to be commented out from v4l2-compliance as >> it led to stalling: >> >> if (node->codec_mask & STATEFUL_DECODER) { >> struct v4l2_decoder_cmd cmd; >> buffer buf_cap(m2m_q); >> >> memset(&cmd, 0, sizeof(cmd)); >> cmd.cmd = V4L2_DEC_CMD_STOP; >> >> /* No buffers are queued, call STREAMON, then STOP */ >> fail_on_test(node->streamon(q.g_type())); >> fail_on_test(node->streamon(m2m_q.g_type())); >> fail_on_test(doioctl(node, VIDIOC_DECODER_CMD, &cmd)); >> >> fail_on_test(buf_cap.querybuf(node, 0)); >> fail_on_test(buf_cap.qbuf(node)); >> fail_on_test(buf_cap.dqbuf(node)); >> fail_on_test(!(buf_cap.g_flags() & V4L2_BUF_FLAG_LAST)); >> for (unsigned p = 0; p < buf_cap.g_num_planes(); p++) >> fail_on_test(buf_cap.g_bytesused(p)); >> fail_on_test(node->streamoff(q.g_type())); >> fail_on_test(node->streamoff(m2m_q.g_type())); >> >> /* Call STREAMON, queue one CAPTURE buffer, then STOP */ >> fail_on_test(node->streamon(q.g_type())); >> fail_on_test(node->streamon(m2m_q.g_type())); >> fail_on_test(buf_cap.querybuf(node, 0)); >> fail_on_test(buf_cap.qbuf(node)); >> fail_on_test(doioctl(node, VIDIOC_DECODER_CMD, &cmd)); >> >> fail_on_test(buf_cap.dqbuf(node)); >> fail_on_test(!(buf_cap.g_flags() & V4L2_BUF_FLAG_LAST)); >> for (unsigned p = 0; p < buf_cap.g_num_planes(); p++) >> fail_on_test(buf_cap.g_bytesused(p)); >> fail_on_test(node->streamoff(q.g_type())); >> fail_on_test(node->streamoff(m2m_q.g_type())); >> } >> >> The reason for this is because the driver has a limitation where all >> capturebuffers must be queued to the driver before STREAMON is effective. >> The firmware needs to know in advance what all the buffers are before >> starting to decode. >> This limitation is enforced via q->min_buffers_needed. >> As such, in this compliance codepath, STREAMON is never actually called >> driver-side and there is a stall on fail_on_test(buf_cap.dqbuf(node)); >> >> >> One last detail: V4L2_FMT_FLAG_DYN_RESOLUTION is currently not recognized >> by v4l2-compliance, so it was left out for the test. However, it is >> present in the patch series. >> >> The second patch has 3 "Alignment should match open parenthesis" lines >> where I preferred to keep them that way. >> >> Thanks Stanimir for sharing your HDR file creation tools, this was very >> helpful :). > > I tried to test this with a pending branch of GStreamer supporting > dynamic resolution changes. The even driver mechanism does not seem to > work with this driver. I've grepped the code, and don't see any places > were the event would be emitted. Thanks for taking the time to test! The event is sent in vdec_helpers.c:434 void amvdec_src_change(struct amvdec_session *sess, u32 width, u32 height, u32 dpb_size) { static const struct v4l2_event ev = { .type = V4L2_EVENT_SOURCE_CHANGE, .u.src_change.changes = V4L2_EVENT_SRC_CH_RESOLUTION }; [..] dev_dbg(sess->core->dev, "Res. changed (%ux%u), DPB size %u\n", width, height, dpb_size); v4l2_event_queue_fh(&sess->fh, &ev); } > > Then I grepped, and it seems the driver accept source_change > subscription but does not set V4L2_FMT_FLAG_DYN_RESOLUTION. I believe > these two things are bit redundant and confusing, I'll fix the proposed > patch never the less, and see if that makes it work. It is set for H.264 if you look at the second patch of the series adding support for it. { .pixfmt = V4L2_PIX_FMT_H264, [..] .flags = V4L2_FMT_FLAG_COMPRESSED | V4L2_FMT_FLAG_DYN_RESOLUTION, }, The reason for this flag is because not all formats within a decoder driver may support dynamic resolution. Your 2 points make me wonder if you used the staging driver + this patch series, or if you used something else ? The various branches I have on github are not up to date with all the compliance work. > >> >> Maxime >> >> # v4l2-compliance --stream-from-hdr test-25fps.h264.hdr -s250 >> v4l2-compliance SHA: a162244d47d4bb01d0692da879dce5a070f118e7, 64 bits >> >> Compliance test for meson-vdec device /dev/video0: >> >> Driver Info: >> Driver name : meson-vdec >> Card type : Amlogic Video Decoder >> Bus info : platform:meson-vdec >> Driver version : 5.4.0 >> Capabilities : 0x84204000 >> Video Memory-to-Memory Multiplanar >> Streaming >> Extended Pix Format >> Device Capabilities >> Device Caps : 0x04204000 >> Video Memory-to-Memory Multiplanar >> Streaming >> Extended Pix Format >> Detected Stateful Decoder >> >> Required ioctls: >> test VIDIOC_QUERYCAP: OK >> >> Allow for multiple opens: >> test second /dev/video0 open: OK >> test VIDIOC_QUERYCAP: OK >> test VIDIOC_G/S_PRIORITY: OK >> test for unlimited opens: OK >> >> Debug ioctls: >> test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported) >> test VIDIOC_LOG_STATUS: OK (Not Supported) >> >> Input ioctls: >> test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported) >> test VIDIOC_G/S_FREQUENCY: OK (Not Supported) >> test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported) >> test VIDIOC_ENUMAUDIO: OK (Not Supported) >> test VIDIOC_G/S/ENUMINPUT: OK (Not Supported) >> test VIDIOC_G/S_AUDIO: OK (Not Supported) >> Inputs: 0 Audio Inputs: 0 Tuners: 0 >> >> Output ioctls: >> test VIDIOC_G/S_MODULATOR: OK (Not Supported) >> test VIDIOC_G/S_FREQUENCY: OK (Not Supported) >> test VIDIOC_ENUMAUDOUT: OK (Not Supported) >> test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported) >> test VIDIOC_G/S_AUDOUT: OK (Not Supported) >> Outputs: 0 Audio Outputs: 0 Modulators: 0 >> >> Input/Output configuration ioctls: >> test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported) >> test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported) >> test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported) >> test VIDIOC_G/S_EDID: OK (Not Supported) >> >> Control ioctls: >> test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK >> test VIDIOC_QUERYCTRL: OK >> test VIDIOC_G/S_CTRL: OK >> test VIDIOC_G/S/TRY_EXT_CTRLS: OK >> test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK >> test VIDIOC_G/S_JPEGCOMP: OK (Not Supported) >> Standard Controls: 2 Private Controls: 0 >> >> Format ioctls: >> test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK >> test VIDIOC_G/S_PARM: OK (Not Supported) >> test VIDIOC_G_FBUF: OK (Not Supported) >> test VIDIOC_G_FMT: OK >> test VIDIOC_TRY_FMT: OK >> test VIDIOC_S_FMT: OK >> test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported) >> test Cropping: OK (Not Supported) >> test Composing: OK (Not Supported) >> test Scaling: OK >> >> Codec ioctls: >> test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported) >> test VIDIOC_G_ENC_INDEX: OK (Not Supported) >> test VIDIOC_(TRY_)DECODER_CMD: OK >> >> Buffer ioctls: >> test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK >> test VIDIOC_EXPBUF: OK >> test Requests: OK (Not Supported) >> >> Test input 0: >> >> Streaming ioctls: >> test read/write: OK (Not Supported) >> test blocking wait: OK >> Video Capture Multiplanar: Captured 250 buffers >> test MMAP (select): OK >> Video Capture Multiplanar: Captured 250 buffers >> test MMAP (epoll): OK >> test USERPTR (select): OK (Not Supported) >> test DMABUF: Cannot test, specify --expbuf-device >> >> Total for meson-vdec device /dev/video0: 49, Succeeded: 49, Failed: 0, Warnings: 0 >> >> Maxime Jourdan (2): >> media: meson: vdec: bring up to compliance >> media: meson: vdec: add H.264 decoding support >> >> drivers/staging/media/meson/vdec/Makefile | 2 +- >> drivers/staging/media/meson/vdec/codec_h264.c | 482 ++++++++++++++++++ >> drivers/staging/media/meson/vdec/codec_h264.h | 14 + >> drivers/staging/media/meson/vdec/esparser.c | 34 +- >> drivers/staging/media/meson/vdec/vdec.c | 70 ++- >> drivers/staging/media/meson/vdec/vdec.h | 14 +- >> .../staging/media/meson/vdec/vdec_helpers.c | 85 ++- >> .../staging/media/meson/vdec/vdec_helpers.h | 6 +- >> .../staging/media/meson/vdec/vdec_platform.c | 43 ++ >> 9 files changed, 654 insertions(+), 96 deletions(-) >> create mode 100644 drivers/staging/media/meson/vdec/codec_h264.c >> create mode 100644 drivers/staging/media/meson/vdec/codec_h264.h >> >
On Wed, Oct 9, 2019 at 2:01 PM Hans Verkuil <hverkuil@xs4all.nl> wrote: > > On 10/8/19 3:40 PM, Maxime Jourdan wrote: > > On 07/10/2019 18:39, Hans Verkuil wrote: > >> On 10/7/19 6:24 PM, Maxime Jourdan wrote: > >>> On 07/10/2019 17:12, Hans Verkuil wrote: > >>>> On 10/7/19 4:59 PM, Maxime Jourdan wrote: > >>>>> Hello, > >>>>> > >>>>> This patch series aims to bring H.264 support as well as compliance update > >>>>> to the amlogic stateful video decoder driver. > >>>>> > >>>>> There is 1 issue that remains currently: > >>>>> > >>>>> - The following codepath had to be commented out from v4l2-compliance as > >>>>> it led to stalling: > >>>>> > >>>>> if (node->codec_mask & STATEFUL_DECODER) { > >>>>> struct v4l2_decoder_cmd cmd; > >>>>> buffer buf_cap(m2m_q); > >>>>> > >>>>> memset(&cmd, 0, sizeof(cmd)); > >>>>> cmd.cmd = V4L2_DEC_CMD_STOP; > >>>>> > >>>>> /* No buffers are queued, call STREAMON, then STOP */ > >>>>> fail_on_test(node->streamon(q.g_type())); > >>>>> fail_on_test(node->streamon(m2m_q.g_type())); > >>>>> fail_on_test(doioctl(node, VIDIOC_DECODER_CMD, &cmd)); > >>>>> > >>>>> fail_on_test(buf_cap.querybuf(node, 0)); > >>>>> fail_on_test(buf_cap.qbuf(node)); > >>>>> fail_on_test(buf_cap.dqbuf(node)); > >>>>> fail_on_test(!(buf_cap.g_flags() & V4L2_BUF_FLAG_LAST)); > >>>>> for (unsigned p = 0; p < buf_cap.g_num_planes(); p++) > >>>>> fail_on_test(buf_cap.g_bytesused(p)); > >>>>> fail_on_test(node->streamoff(q.g_type())); > >>>>> fail_on_test(node->streamoff(m2m_q.g_type())); > >>>>> > >>>>> /* Call STREAMON, queue one CAPTURE buffer, then STOP */ > >>>>> fail_on_test(node->streamon(q.g_type())); > >>>>> fail_on_test(node->streamon(m2m_q.g_type())); > >>>>> fail_on_test(buf_cap.querybuf(node, 0)); > >>>>> fail_on_test(buf_cap.qbuf(node)); > >>>>> fail_on_test(doioctl(node, VIDIOC_DECODER_CMD, &cmd)); > >>>>> > >>>>> fail_on_test(buf_cap.dqbuf(node)); > >>>>> fail_on_test(!(buf_cap.g_flags() & V4L2_BUF_FLAG_LAST)); > >>>>> for (unsigned p = 0; p < buf_cap.g_num_planes(); p++) > >>>>> fail_on_test(buf_cap.g_bytesused(p)); > >>>>> fail_on_test(node->streamoff(q.g_type())); > >>>>> fail_on_test(node->streamoff(m2m_q.g_type())); > >>>>> } > >>>>> > >>>>> The reason for this is because the driver has a limitation where all > >>>>> capturebuffers must be queued to the driver before STREAMON is effective. > >>>>> The firmware needs to know in advance what all the buffers are before > >>>>> starting to decode. > >>>>> This limitation is enforced via q->min_buffers_needed. > >>>>> As such, in this compliance codepath, STREAMON is never actually called > >>>>> driver-side and there is a stall on fail_on_test(buf_cap.dqbuf(node)); > >>>> > >>>> That's interesting. I will have to look more closely at this. > > This requires a helper function in videobuf2-v4l2.c. > > In vdec_decoder_cmd you would need code like this: > > if (!vb2_start_streaming_called(&capture_queue)) { > vb2_dequeue_empty_last_buf(&capture_queue); > return 0; > } > > The vb2_dequeue_empty_last_buf (function name can probably be improved upon!) > does nothing if no capture buffers were queued, otherwise it takes the first > buffer, sets the LAST flag and sets bytesused to 0 and marks it as DONE. > > The driver cannot do this directly, since the buffers were never queued to the > driver and are owned by vb2. > > This is something that needs to be done for any codec driver that sets > min_buffers_needed to a value > 1. > > The vb2 function would look something like this: > > void vb2_dqbuf_empty_last_buf(struct vb2_queue *q) > { > struct vb2_buffer *vb; > struct vb2_v4l2_buffer *vbuf; > unsigned int i; > > if (WARN_ON(q->is_output)) > return; > if (list_empty(&q->queued_list)) > return; > vb = list_first_entry(&q->queued_list, struct vb2_buffer, queued_entry); > list_del(&vb->queued_entry); > for (i = 0; i < vb->num_planes; i++) > vb2_set_plane_payload(vb, i, 0) > vbuf = to_vb2_v4l2_buffer(vb); > vbuf->flags |= V4L2_BUF_FLAG_LAST; > vb2_buffer_done(vb, VB2_BUF_STATE_DONE); > } > EXPORT_SYMBOL_GPL(vb2_dqbuf_empty_last_buf); > > Neither compiled, nor tested, and I think this should be in v4l2-mem2mem.c instead of > in videobuf2-v4l2.c since this is very m2m specific. > > So see this as a suggestion :-) > > Anyway, the key take-away from this is that userspace does not know if your driver > behaves the way it does, so STOP should still produce a sane expected result. > > Which in this is just a single empty capture buffer marked LAST. Thanks, this makes sense. It doesn't quite fit the current usage unfortunately as the test in v4l2-compliance goes like this: fail_on_test(doioctl(node, VIDIOC_DECODER_CMD, &cmd)); fail_on_test(buf_cap.querybuf(node, 0)); fail_on_test(buf_cap.qbuf(node)); fail_on_test(buf_cap.dqbuf(node)); fail_on_test(!(buf_cap.g_flags() & V4L2_BUF_FLAG_LAST)); Since the buffer is queued after issuing the stop cmd, it is not possible to flag it as DONE in vdec_decoder_cmd. A solution would be to hijack vidioc_qbuf and flag the buffer if a stop has been issued previously and the capture queue is not streaming. Would that be okay ? Maxime > > Regards, > > Hans
On 18/10/2019 09:50, Maxime Jourdan wrote: > On Wed, Oct 9, 2019 at 2:01 PM Hans Verkuil <hverkuil@xs4all.nl> wrote: >> >> On 10/8/19 3:40 PM, Maxime Jourdan wrote: >>> On 07/10/2019 18:39, Hans Verkuil wrote: >>>> On 10/7/19 6:24 PM, Maxime Jourdan wrote: >>>>> On 07/10/2019 17:12, Hans Verkuil wrote: >>>>>> On 10/7/19 4:59 PM, Maxime Jourdan wrote: >>>>>>> Hello, >>>>>>> >>>>>>> This patch series aims to bring H.264 support as well as compliance update >>>>>>> to the amlogic stateful video decoder driver. >>>>>>> >>>>>>> There is 1 issue that remains currently: >>>>>>> >>>>>>> - The following codepath had to be commented out from v4l2-compliance as >>>>>>> it led to stalling: >>>>>>> >>>>>>> if (node->codec_mask & STATEFUL_DECODER) { >>>>>>> struct v4l2_decoder_cmd cmd; >>>>>>> buffer buf_cap(m2m_q); >>>>>>> >>>>>>> memset(&cmd, 0, sizeof(cmd)); >>>>>>> cmd.cmd = V4L2_DEC_CMD_STOP; >>>>>>> >>>>>>> /* No buffers are queued, call STREAMON, then STOP */ >>>>>>> fail_on_test(node->streamon(q.g_type())); >>>>>>> fail_on_test(node->streamon(m2m_q.g_type())); >>>>>>> fail_on_test(doioctl(node, VIDIOC_DECODER_CMD, &cmd)); >>>>>>> >>>>>>> fail_on_test(buf_cap.querybuf(node, 0)); >>>>>>> fail_on_test(buf_cap.qbuf(node)); >>>>>>> fail_on_test(buf_cap.dqbuf(node)); >>>>>>> fail_on_test(!(buf_cap.g_flags() & V4L2_BUF_FLAG_LAST)); >>>>>>> for (unsigned p = 0; p < buf_cap.g_num_planes(); p++) >>>>>>> fail_on_test(buf_cap.g_bytesused(p)); >>>>>>> fail_on_test(node->streamoff(q.g_type())); >>>>>>> fail_on_test(node->streamoff(m2m_q.g_type())); >>>>>>> >>>>>>> /* Call STREAMON, queue one CAPTURE buffer, then STOP */ >>>>>>> fail_on_test(node->streamon(q.g_type())); >>>>>>> fail_on_test(node->streamon(m2m_q.g_type())); >>>>>>> fail_on_test(buf_cap.querybuf(node, 0)); >>>>>>> fail_on_test(buf_cap.qbuf(node)); >>>>>>> fail_on_test(doioctl(node, VIDIOC_DECODER_CMD, &cmd)); >>>>>>> >>>>>>> fail_on_test(buf_cap.dqbuf(node)); >>>>>>> fail_on_test(!(buf_cap.g_flags() & V4L2_BUF_FLAG_LAST)); >>>>>>> for (unsigned p = 0; p < buf_cap.g_num_planes(); p++) >>>>>>> fail_on_test(buf_cap.g_bytesused(p)); >>>>>>> fail_on_test(node->streamoff(q.g_type())); >>>>>>> fail_on_test(node->streamoff(m2m_q.g_type())); >>>>>>> } >>>>>>> >>>>>>> The reason for this is because the driver has a limitation where all >>>>>>> capturebuffers must be queued to the driver before STREAMON is effective. >>>>>>> The firmware needs to know in advance what all the buffers are before >>>>>>> starting to decode. >>>>>>> This limitation is enforced via q->min_buffers_needed. >>>>>>> As such, in this compliance codepath, STREAMON is never actually called >>>>>>> driver-side and there is a stall on fail_on_test(buf_cap.dqbuf(node)); >>>>>> >>>>>> That's interesting. I will have to look more closely at this. >> >> This requires a helper function in videobuf2-v4l2.c. >> >> In vdec_decoder_cmd you would need code like this: >> >> if (!vb2_start_streaming_called(&capture_queue)) { >> vb2_dequeue_empty_last_buf(&capture_queue); >> return 0; >> } >> >> The vb2_dequeue_empty_last_buf (function name can probably be improved upon!) >> does nothing if no capture buffers were queued, otherwise it takes the first >> buffer, sets the LAST flag and sets bytesused to 0 and marks it as DONE. >> >> The driver cannot do this directly, since the buffers were never queued to the >> driver and are owned by vb2. >> >> This is something that needs to be done for any codec driver that sets >> min_buffers_needed to a value > 1. >> >> The vb2 function would look something like this: >> >> void vb2_dqbuf_empty_last_buf(struct vb2_queue *q) >> { >> struct vb2_buffer *vb; >> struct vb2_v4l2_buffer *vbuf; >> unsigned int i; >> >> if (WARN_ON(q->is_output)) >> return; >> if (list_empty(&q->queued_list)) >> return; >> vb = list_first_entry(&q->queued_list, struct vb2_buffer, queued_entry); >> list_del(&vb->queued_entry); >> for (i = 0; i < vb->num_planes; i++) >> vb2_set_plane_payload(vb, i, 0) >> vbuf = to_vb2_v4l2_buffer(vb); >> vbuf->flags |= V4L2_BUF_FLAG_LAST; >> vb2_buffer_done(vb, VB2_BUF_STATE_DONE); >> } >> EXPORT_SYMBOL_GPL(vb2_dqbuf_empty_last_buf); >> >> Neither compiled, nor tested, and I think this should be in v4l2-mem2mem.c instead of >> in videobuf2-v4l2.c since this is very m2m specific. >> >> So see this as a suggestion :-) >> >> Anyway, the key take-away from this is that userspace does not know if your driver >> behaves the way it does, so STOP should still produce a sane expected result. >> >> Which in this is just a single empty capture buffer marked LAST. > > Thanks, this makes sense. It doesn't quite fit the current usage > unfortunately as the test in v4l2-compliance goes like this: > > fail_on_test(doioctl(node, VIDIOC_DECODER_CMD, &cmd)); > fail_on_test(buf_cap.querybuf(node, 0)); > fail_on_test(buf_cap.qbuf(node)); > fail_on_test(buf_cap.dqbuf(node)); > fail_on_test(!(buf_cap.g_flags() & V4L2_BUF_FLAG_LAST)); > > Since the buffer is queued after issuing the stop cmd, it is not > possible to flag it as DONE in vdec_decoder_cmd. > > A solution would be to hijack vidioc_qbuf and flag the buffer if a > stop has been issued previously and the capture queue is not > streaming. Would that be okay ? > I was able to pass the vanilla v4l2-compliance tests by hacking in the following 2 things: 1) Adding your function that I modified a bit (buffer had to be tagged as VB2_BUF_STATE_ACTIVE, it shouldn't be removed from the list, and q->owned_by_drv_count needs to be atomic_inc'd) void vb2_dqbuf_empty_last_buf(struct vb2_queue *q) { struct vb2_buffer *vb; struct vb2_v4l2_buffer *vbuf; unsigned int i; if (WARN_ON(q->is_output)) return; if (list_empty(&q->queued_list)) return; vb = list_first_entry(&q->queued_list, struct vb2_buffer, queued_entry); for (i = 0; i < vb->num_planes; i++) vb2_set_plane_payload(vb, i, 0); vb->state = VB2_BUF_STATE_ACTIVE; atomic_inc(&q->owned_by_drv_count); vbuf = to_vb2_v4l2_buffer(vb); vbuf->flags |= V4L2_BUF_FLAG_LAST; vb2_buffer_done(vb, VB2_BUF_STATE_DONE); } 2) Hijacking vidioc_qbuf to DONE the buffer if a stop was previously asked for, and if the capture queue isn't streaming: static int vdec_qbuf(struct file *file, void *priv, struct v4l2_buffer *buf) { struct v4l2_fh *fh = file->private_data; struct amvdec_session *sess; struct vb2_queue *vq; int ret; ret = v4l2_m2m_ioctl_qbuf(file, priv, buf); if (ret) return ret; sess = container_of(file->private_data, struct amvdec_session, fh); vq = v4l2_m2m_get_vq(fh->m2m_ctx, buf->type); /* * If the capture queue isn't streaming and we were asked to * stop, DONE the buffer instantly */ if (!V4L2_TYPE_IS_OUTPUT(vq->type) && !sess->streamon_cap && sess->should_stop) vb2_dqbuf_empty_last_buf(vq); return 0; } Overall it feels quite messy :( . It doesn't look like a buffer is supposed to be dequeued if streaming hasn't started (they are tagged as VB2_BUF_STATE_ACTIVE only when streaming starts, and this flag is a requirement for vb2_buffer_done). All this could be built in more properly into v4l2-mem2mem.c, though it would require the same hacks around VB2_BUF_STATE_ACTIVE and q->owned_by_drv_count. Or it would need a vb2 function specifically for this case, which would be very similar to vb2_buffer_done but allowing the state being VB2_BUF_STATE_QUEUED and not changing q->owned_by_drv_count. >> >> Regards, >> >> Hans
On 10/18/19 9:50 AM, Maxime Jourdan wrote: > On Wed, Oct 9, 2019 at 2:01 PM Hans Verkuil <hverkuil@xs4all.nl> wrote: >> >> On 10/8/19 3:40 PM, Maxime Jourdan wrote: >>> On 07/10/2019 18:39, Hans Verkuil wrote: >>>> On 10/7/19 6:24 PM, Maxime Jourdan wrote: >>>>> On 07/10/2019 17:12, Hans Verkuil wrote: >>>>>> On 10/7/19 4:59 PM, Maxime Jourdan wrote: >>>>>>> Hello, >>>>>>> >>>>>>> This patch series aims to bring H.264 support as well as compliance update >>>>>>> to the amlogic stateful video decoder driver. >>>>>>> >>>>>>> There is 1 issue that remains currently: >>>>>>> >>>>>>> - The following codepath had to be commented out from v4l2-compliance as >>>>>>> it led to stalling: >>>>>>> >>>>>>> if (node->codec_mask & STATEFUL_DECODER) { >>>>>>> struct v4l2_decoder_cmd cmd; >>>>>>> buffer buf_cap(m2m_q); >>>>>>> >>>>>>> memset(&cmd, 0, sizeof(cmd)); >>>>>>> cmd.cmd = V4L2_DEC_CMD_STOP; >>>>>>> >>>>>>> /* No buffers are queued, call STREAMON, then STOP */ >>>>>>> fail_on_test(node->streamon(q.g_type())); >>>>>>> fail_on_test(node->streamon(m2m_q.g_type())); >>>>>>> fail_on_test(doioctl(node, VIDIOC_DECODER_CMD, &cmd)); >>>>>>> >>>>>>> fail_on_test(buf_cap.querybuf(node, 0)); >>>>>>> fail_on_test(buf_cap.qbuf(node)); >>>>>>> fail_on_test(buf_cap.dqbuf(node)); >>>>>>> fail_on_test(!(buf_cap.g_flags() & V4L2_BUF_FLAG_LAST)); >>>>>>> for (unsigned p = 0; p < buf_cap.g_num_planes(); p++) >>>>>>> fail_on_test(buf_cap.g_bytesused(p)); >>>>>>> fail_on_test(node->streamoff(q.g_type())); >>>>>>> fail_on_test(node->streamoff(m2m_q.g_type())); >>>>>>> >>>>>>> /* Call STREAMON, queue one CAPTURE buffer, then STOP */ >>>>>>> fail_on_test(node->streamon(q.g_type())); >>>>>>> fail_on_test(node->streamon(m2m_q.g_type())); >>>>>>> fail_on_test(buf_cap.querybuf(node, 0)); >>>>>>> fail_on_test(buf_cap.qbuf(node)); >>>>>>> fail_on_test(doioctl(node, VIDIOC_DECODER_CMD, &cmd)); >>>>>>> >>>>>>> fail_on_test(buf_cap.dqbuf(node)); >>>>>>> fail_on_test(!(buf_cap.g_flags() & V4L2_BUF_FLAG_LAST)); >>>>>>> for (unsigned p = 0; p < buf_cap.g_num_planes(); p++) >>>>>>> fail_on_test(buf_cap.g_bytesused(p)); >>>>>>> fail_on_test(node->streamoff(q.g_type())); >>>>>>> fail_on_test(node->streamoff(m2m_q.g_type())); >>>>>>> } >>>>>>> >>>>>>> The reason for this is because the driver has a limitation where all >>>>>>> capturebuffers must be queued to the driver before STREAMON is effective. >>>>>>> The firmware needs to know in advance what all the buffers are before >>>>>>> starting to decode. >>>>>>> This limitation is enforced via q->min_buffers_needed. >>>>>>> As such, in this compliance codepath, STREAMON is never actually called >>>>>>> driver-side and there is a stall on fail_on_test(buf_cap.dqbuf(node)); >>>>>> >>>>>> That's interesting. I will have to look more closely at this. >> >> This requires a helper function in videobuf2-v4l2.c. >> >> In vdec_decoder_cmd you would need code like this: >> >> if (!vb2_start_streaming_called(&capture_queue)) { >> vb2_dequeue_empty_last_buf(&capture_queue); >> return 0; >> } >> >> The vb2_dequeue_empty_last_buf (function name can probably be improved upon!) >> does nothing if no capture buffers were queued, otherwise it takes the first >> buffer, sets the LAST flag and sets bytesused to 0 and marks it as DONE. >> >> The driver cannot do this directly, since the buffers were never queued to the >> driver and are owned by vb2. >> >> This is something that needs to be done for any codec driver that sets >> min_buffers_needed to a value > 1. >> >> The vb2 function would look something like this: >> >> void vb2_dqbuf_empty_last_buf(struct vb2_queue *q) >> { >> struct vb2_buffer *vb; >> struct vb2_v4l2_buffer *vbuf; >> unsigned int i; >> >> if (WARN_ON(q->is_output)) >> return; >> if (list_empty(&q->queued_list)) >> return; >> vb = list_first_entry(&q->queued_list, struct vb2_buffer, queued_entry); >> list_del(&vb->queued_entry); >> for (i = 0; i < vb->num_planes; i++) >> vb2_set_plane_payload(vb, i, 0) >> vbuf = to_vb2_v4l2_buffer(vb); >> vbuf->flags |= V4L2_BUF_FLAG_LAST; >> vb2_buffer_done(vb, VB2_BUF_STATE_DONE); >> } >> EXPORT_SYMBOL_GPL(vb2_dqbuf_empty_last_buf); >> >> Neither compiled, nor tested, and I think this should be in v4l2-mem2mem.c instead of >> in videobuf2-v4l2.c since this is very m2m specific. >> >> So see this as a suggestion :-) >> >> Anyway, the key take-away from this is that userspace does not know if your driver >> behaves the way it does, so STOP should still produce a sane expected result. >> >> Which in this is just a single empty capture buffer marked LAST. > > Thanks, this makes sense. It doesn't quite fit the current usage > unfortunately as the test in v4l2-compliance goes like this: > > fail_on_test(doioctl(node, VIDIOC_DECODER_CMD, &cmd)); > fail_on_test(buf_cap.querybuf(node, 0)); > fail_on_test(buf_cap.qbuf(node)); > fail_on_test(buf_cap.dqbuf(node)); > fail_on_test(!(buf_cap.g_flags() & V4L2_BUF_FLAG_LAST)); > > Since the buffer is queued after issuing the stop cmd, it is not > possible to flag it as DONE in vdec_decoder_cmd. > > A solution would be to hijack vidioc_qbuf and flag the buffer if a > stop has been issued previously and the capture queue is not > streaming. Would that be okay ? Actually, I am wondering if this shouldn't be integrated into v4l2-mem2mem.c. The corner case where you need to use an empty last buffer is really awkward for drivers. So perhaps this should be integrated into v4l2-mem2mem.c where you can mark that the next queued buffer shall be immediately returned as an empty buffer with the LAST flag set. Since v4l2-mem2mem already has its own vidioc_qbuf function it can easily be added there, and I think that's a much better place than having to touch vb2 itself. Regards, Hans > > Maxime > >> >> Regards, >> >> Hans
Hello, This patch series aims to bring H.264 support as well as compliance update to the amlogic stateful video decoder driver. There is 1 issue that remains currently: - The following codepath had to be commented out from v4l2-compliance as it led to stalling: if (node->codec_mask & STATEFUL_DECODER) { struct v4l2_decoder_cmd cmd; buffer buf_cap(m2m_q); memset(&cmd, 0, sizeof(cmd)); cmd.cmd = V4L2_DEC_CMD_STOP; /* No buffers are queued, call STREAMON, then STOP */ fail_on_test(node->streamon(q.g_type())); fail_on_test(node->streamon(m2m_q.g_type())); fail_on_test(doioctl(node, VIDIOC_DECODER_CMD, &cmd)); fail_on_test(buf_cap.querybuf(node, 0)); fail_on_test(buf_cap.qbuf(node)); fail_on_test(buf_cap.dqbuf(node)); fail_on_test(!(buf_cap.g_flags() & V4L2_BUF_FLAG_LAST)); for (unsigned p = 0; p < buf_cap.g_num_planes(); p++) fail_on_test(buf_cap.g_bytesused(p)); fail_on_test(node->streamoff(q.g_type())); fail_on_test(node->streamoff(m2m_q.g_type())); /* Call STREAMON, queue one CAPTURE buffer, then STOP */ fail_on_test(node->streamon(q.g_type())); fail_on_test(node->streamon(m2m_q.g_type())); fail_on_test(buf_cap.querybuf(node, 0)); fail_on_test(buf_cap.qbuf(node)); fail_on_test(doioctl(node, VIDIOC_DECODER_CMD, &cmd)); fail_on_test(buf_cap.dqbuf(node)); fail_on_test(!(buf_cap.g_flags() & V4L2_BUF_FLAG_LAST)); for (unsigned p = 0; p < buf_cap.g_num_planes(); p++) fail_on_test(buf_cap.g_bytesused(p)); fail_on_test(node->streamoff(q.g_type())); fail_on_test(node->streamoff(m2m_q.g_type())); } The reason for this is because the driver has a limitation where all capturebuffers must be queued to the driver before STREAMON is effective. The firmware needs to know in advance what all the buffers are before starting to decode. This limitation is enforced via q->min_buffers_needed. As such, in this compliance codepath, STREAMON is never actually called driver-side and there is a stall on fail_on_test(buf_cap.dqbuf(node)); One last detail: V4L2_FMT_FLAG_DYN_RESOLUTION is currently not recognized by v4l2-compliance, so it was left out for the test. However, it is present in the patch series. The second patch has 3 "Alignment should match open parenthesis" lines where I preferred to keep them that way. Thanks Stanimir for sharing your HDR file creation tools, this was very helpful :). Maxime # v4l2-compliance --stream-from-hdr test-25fps.h264.hdr -s250 v4l2-compliance SHA: a162244d47d4bb01d0692da879dce5a070f118e7, 64 bits Compliance test for meson-vdec device /dev/video0: Driver Info: Driver name : meson-vdec Card type : Amlogic Video Decoder Bus info : platform:meson-vdec Driver version : 5.4.0 Capabilities : 0x84204000 Video Memory-to-Memory Multiplanar Streaming Extended Pix Format Device Capabilities Device Caps : 0x04204000 Video Memory-to-Memory Multiplanar Streaming Extended Pix Format Detected Stateful Decoder Required ioctls: test VIDIOC_QUERYCAP: OK Allow for multiple opens: test second /dev/video0 open: OK test VIDIOC_QUERYCAP: OK test VIDIOC_G/S_PRIORITY: OK test for unlimited opens: OK Debug ioctls: test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported) test VIDIOC_LOG_STATUS: OK (Not Supported) Input ioctls: test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported) test VIDIOC_G/S_FREQUENCY: OK (Not Supported) test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported) test VIDIOC_ENUMAUDIO: OK (Not Supported) test VIDIOC_G/S/ENUMINPUT: OK (Not Supported) test VIDIOC_G/S_AUDIO: OK (Not Supported) Inputs: 0 Audio Inputs: 0 Tuners: 0 Output ioctls: test VIDIOC_G/S_MODULATOR: OK (Not Supported) test VIDIOC_G/S_FREQUENCY: OK (Not Supported) test VIDIOC_ENUMAUDOUT: OK (Not Supported) test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported) test VIDIOC_G/S_AUDOUT: OK (Not Supported) Outputs: 0 Audio Outputs: 0 Modulators: 0 Input/Output configuration ioctls: test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported) test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported) test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported) test VIDIOC_G/S_EDID: OK (Not Supported) Control ioctls: test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK test VIDIOC_QUERYCTRL: OK test VIDIOC_G/S_CTRL: OK test VIDIOC_G/S/TRY_EXT_CTRLS: OK test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK test VIDIOC_G/S_JPEGCOMP: OK (Not Supported) Standard Controls: 2 Private Controls: 0 Format ioctls: test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK test VIDIOC_G/S_PARM: OK (Not Supported) test VIDIOC_G_FBUF: OK (Not Supported) test VIDIOC_G_FMT: OK test VIDIOC_TRY_FMT: OK test VIDIOC_S_FMT: OK test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported) test Cropping: OK (Not Supported) test Composing: OK (Not Supported) test Scaling: OK Codec ioctls: test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported) test VIDIOC_G_ENC_INDEX: OK (Not Supported) test VIDIOC_(TRY_)DECODER_CMD: OK Buffer ioctls: test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK test VIDIOC_EXPBUF: OK test Requests: OK (Not Supported) Test input 0: Streaming ioctls: test read/write: OK (Not Supported) test blocking wait: OK Video Capture Multiplanar: Captured 250 buffers test MMAP (select): OK Video Capture Multiplanar: Captured 250 buffers test MMAP (epoll): OK test USERPTR (select): OK (Not Supported) test DMABUF: Cannot test, specify --expbuf-device Total for meson-vdec device /dev/video0: 49, Succeeded: 49, Failed: 0, Warnings: 0 Maxime Jourdan (2): media: meson: vdec: bring up to compliance media: meson: vdec: add H.264 decoding support drivers/staging/media/meson/vdec/Makefile | 2 +- drivers/staging/media/meson/vdec/codec_h264.c | 482 ++++++++++++++++++ drivers/staging/media/meson/vdec/codec_h264.h | 14 + drivers/staging/media/meson/vdec/esparser.c | 34 +- drivers/staging/media/meson/vdec/vdec.c | 70 ++- drivers/staging/media/meson/vdec/vdec.h | 14 +- .../staging/media/meson/vdec/vdec_helpers.c | 85 ++- .../staging/media/meson/vdec/vdec_helpers.h | 6 +- .../staging/media/meson/vdec/vdec_platform.c | 43 ++ 9 files changed, 654 insertions(+), 96 deletions(-) create mode 100644 drivers/staging/media/meson/vdec/codec_h264.c create mode 100644 drivers/staging/media/meson/vdec/codec_h264.h