From: "Jernej Škrabec" <jernej.skrabec@siol.net>
To: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Cc: devel@driverdev.osuosl.org, linux-media@vger.kernel.org,
pawel@osciak.com, jonas@kwiboo.se, gregkh@linuxfoundation.org,
wens@csie.org, mripard@kernel.org, tfiga@chromium.org,
paul.kocialkowski@bootlin.com, kyungmin.park@samsung.com,
boris.brezillon@collabora.com,
linux-arm-kernel@lists.infradead.org, mchehab@kernel.org,
ezequiel@collabora.com, linux-kernel@vger.kernel.org,
m.szyprowski@samsung.com
Subject: Re: [PATCH v2 0/6] media: cedrus: h264: Support multi-slice frames
Date: Tue, 01 Oct 2019 00:58:35 +0200 [thread overview]
Message-ID: <11665334.4LxpHo8hza@jernej-laptop> (raw)
In-Reply-To: <eb127b9a-e226-d230-67b5-069795bd76fb@xs4all.nl>
Dne torek, 01. oktober 2019 ob 00:43:34 CEST je Hans Verkuil napisal(a):
> On 10/1/19 12:27 AM, Jernej Škrabec wrote:
> > Dne ponedeljek, 30. september 2019 ob 10:10:48 CEST je Hans Verkuil
> >
> > napisal(a):
> >> On 9/29/19 10:00 PM, Jernej Skrabec wrote:
> >>> This series adds support for decoding multi-slice H264 frames along with
> >>> support for V4L2_DEC_CMD_FLUSH and V4L2_BUF_FLAG_M2M_HOLD_CAPTURE_BUF.
> >>>
> >>> Code was tested by modified ffmpeg, which can be found here:
> >>> https://github.com/jernejsk/FFmpeg, branch mainline-test
> >>> It has to be configured with at least following options:
> >>> --enable-v4l2-request --enable-libudev --enable-libdrm
> >>>
> >>> Samples used for testing:
> >>> http://jernej.libreelec.tv/videos/h264/BA1_FT_C.mp4
> >>> http://jernej.libreelec.tv/videos/h264/h264.mp4
> >>>
> >>> Command line used for testing:
> >>> ffmpeg -hwaccel drm -hwaccel_device /dev/dri/card0 -i h264.mp4 -pix_fmt
> >>> bgra -f fbdev /dev/fb0
> >>>
> >>> Please note that V4L2_DEC_CMD_FLUSH was not tested because I'm
> >>> not sure how. ffmpeg follows exactly which slice is last in frame
> >>> and sets hold flag accordingly. Improper usage of hold flag would
> >>> corrupt ffmpeg assumptions and it would probably crash. Any ideas
> >>> how to test this are welcome!
> >>
> >> It can be tested partially at least: if ffmpeg tells you it is the last
> >> slice, then queue the slice with the HOLD flag set, then call FLUSH
> >> afterwards. This should clear the HOLD flag again. In this case the
> >> queued
> >> buffer is probably not yet processed, so the flag is cleared before the
> >> decode job starts.
> >>
> >> You can also try to add a sleep before calling FLUSH to see what happens
> >> if the last queued buffer is already decoded.
> >
> > Ok, I tried following code:
> > https://github.com/jernejsk/FFmpeg/blob/flush_test/libavcodec/
> > v4l2_request.c#L220-L233
> >
> > But results are not good. It seems that out_vb in flush command is always
> > NULL and so it always marks capture buffer as done, which leads to kernel
> > warnings.
> >
> > dmesg output with debugging messages is here: http://ix.io/1Ks8
> >
> > Currently I'm not sure what is going on. Shouldn't be output buffer queued
> > and waiting to MEDIA_REQUEST_IOC_QUEUE which is executed after flush
> > command as it can be seen from ffmpeg code linked above?
>
> Argh, I forgot about the fact that this uses requests.
>
> The FLUSH should happen *after* the MEDIA_REQUEST_IOC_QUEUE ioctl. Otherwise
> it has no effect. As long as the request hasn't been queued, the buffer is
> also not queued to the driver, so out_vb will indeed be NULL.
Well, flush cmd has effect if it is called before MEDIA_REQUEST_IOC_QUEUE ioctl
as it can be seen from linked dmesg output. I guess there is nothing that we
can do to prevent wrong usage?
BTW, if capture buffer is marked as done, shouldn't be also removed from the
queue?
Best regards,
Jernej
>
> Sorry for the confusion.
>
> Regards,
>
> Hans
>
> > Best regards,
> > Jernej
> >
> >> Regards,
> >>
> >> Hans
> >>
> >>> Thanks to Jonas for adjusting ffmpeg.
> >>>
> >>> Please let me know what you think.
> >>>
> >>> Best regards,
> >>> Jernej
> >>>
> >>> Changes from v1:
> >>> - added Rb tags
> >>> - updated V4L2_DEC_CMD_FLUSH documentation
> >>> - updated first slice detection in Cedrus
> >>> - hold capture buffer flag is set according to source format
> >>> - added v4l m2m stateless_(try_)decoder_cmd ioctl helpers
> >>>
> >>> Hans Verkuil (2):
> >>> vb2: add V4L2_BUF_FLAG_M2M_HOLD_CAPTURE_BUF
> >>> videodev2.h: add V4L2_DEC_CMD_FLUSH
> >>>
> >>> Jernej Skrabec (4):
> >>> media: v4l2-mem2mem: add stateless_(try_)decoder_cmd ioctl helpers
> >>> media: cedrus: Detect first slice of a frame
> >>> media: cedrus: h264: Support multiple slices per frame
> >>> media: cedrus: Add support for holding capture buffer
> >>>
> >>> Documentation/media/uapi/v4l/buffer.rst | 13 ++++++
> >>> .../media/uapi/v4l/vidioc-decoder-cmd.rst | 10 +++-
> >>> .../media/uapi/v4l/vidioc-reqbufs.rst | 6 +++
> >>> .../media/videodev2.h.rst.exceptions | 1 +
> >>> .../media/common/videobuf2/videobuf2-v4l2.c | 8 +++-
> >>> drivers/media/v4l2-core/v4l2-mem2mem.c | 35 ++++++++++++++
> >>> drivers/staging/media/sunxi/cedrus/cedrus.h | 1 +
> >>> .../staging/media/sunxi/cedrus/cedrus_dec.c | 11 +++++
> >>> .../staging/media/sunxi/cedrus/cedrus_h264.c | 11 ++++-
> >>> .../staging/media/sunxi/cedrus/cedrus_hw.c | 8 ++--
> >>> .../staging/media/sunxi/cedrus/cedrus_video.c | 14 ++++++
> >>> include/media/v4l2-mem2mem.h | 46 +++++++++++++++++++
> >>> include/media/videobuf2-core.h | 3 ++
> >>> include/media/videobuf2-v4l2.h | 5 ++
> >>> include/uapi/linux/videodev2.h | 14 ++++--
> >>> 15 files changed, 175 insertions(+), 11 deletions(-)
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2019-09-30 22:58 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-29 20:00 [PATCH v2 0/6] media: cedrus: h264: Support multi-slice frames Jernej Skrabec
2019-09-29 20:00 ` [PATCH v2 1/6] vb2: add V4L2_BUF_FLAG_M2M_HOLD_CAPTURE_BUF Jernej Skrabec
2019-09-29 20:00 ` [PATCH v2 2/6] videodev2.h: add V4L2_DEC_CMD_FLUSH Jernej Skrabec
2019-09-29 20:00 ` [PATCH v2 3/6] media: v4l2-mem2mem: add stateless_(try_)decoder_cmd ioctl helpers Jernej Skrabec
2019-10-04 9:21 ` Hans Verkuil
2019-10-07 6:02 ` Jernej Škrabec
2019-10-07 8:32 ` Hans Verkuil
2019-10-07 9:44 ` Hans Verkuil
2019-09-29 20:00 ` [PATCH v2 4/6] media: cedrus: Detect first slice of a frame Jernej Skrabec
2019-09-29 20:00 ` [PATCH v2 5/6] media: cedrus: h264: Support multiple slices per frame Jernej Skrabec
2019-09-29 20:00 ` [PATCH v2 6/6] media: cedrus: Add support for holding capture buffer Jernej Skrabec
2019-09-30 8:14 ` Hans Verkuil
2019-09-30 16:44 ` Jernej Škrabec
2019-09-30 8:10 ` [PATCH v2 0/6] media: cedrus: h264: Support multi-slice frames Hans Verkuil
2019-09-30 22:27 ` Jernej Škrabec
2019-09-30 22:43 ` Hans Verkuil
2019-09-30 22:58 ` Jernej Škrabec [this message]
2019-10-01 5:33 ` Jernej Škrabec
2019-10-07 10:44 ` Hans Verkuil
2019-10-07 19:01 ` Jernej Škrabec
2019-10-09 10:18 ` Hans Verkuil
2019-10-09 14:44 ` Jernej Škrabec
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=11665334.4LxpHo8hza@jernej-laptop \
--to=jernej.skrabec@siol.net \
--cc=boris.brezillon@collabora.com \
--cc=devel@driverdev.osuosl.org \
--cc=ezequiel@collabora.com \
--cc=gregkh@linuxfoundation.org \
--cc=hverkuil-cisco@xs4all.nl \
--cc=jonas@kwiboo.se \
--cc=kyungmin.park@samsung.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=m.szyprowski@samsung.com \
--cc=mchehab@kernel.org \
--cc=mripard@kernel.org \
--cc=paul.kocialkowski@bootlin.com \
--cc=pawel@osciak.com \
--cc=tfiga@chromium.org \
--cc=wens@csie.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).