lkml.org 
[lkml]   [2021]   [Sep]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v8 04/15] media:Add v4l2 event codec_error and skip
    From
    Date
    Hi Ming,

    more API only review.

    Le mardi 07 septembre 2021 à 17:49 +0800, Ming Qian a écrit :
    > The codec_error event can tell client that
    > there are some error occurs in the decoder engine.
    >
    > The skip event can tell the client that
    > there are a frame has been decoded,
    > but it won't be outputed.
    >
    > Signed-off-by: Ming Qian <ming.qian@nxp.com>
    > Signed-off-by: Shijie Qin <shijie.qin@nxp.com>
    > Signed-off-by: Zhou Peng <eagle.zhou@nxp.com>
    > ---
    > .../userspace-api/media/v4l/vidioc-dqevent.rst | 12 ++++++++++++
    > include/uapi/linux/videodev2.h | 2 ++
    > 2 files changed, 14 insertions(+)
    >
    > diff --git a/Documentation/userspace-api/media/v4l/vidioc-dqevent.rst b/Documentation/userspace-api/media/v4l/vidioc-dqevent.rst
    > index 6eb40073c906..87d40ad25604 100644
    > --- a/Documentation/userspace-api/media/v4l/vidioc-dqevent.rst
    > +++ b/Documentation/userspace-api/media/v4l/vidioc-dqevent.rst
    > @@ -182,6 +182,18 @@ call.
    > the regions changes. This event has a struct
    > :c:type:`v4l2_event_motion_det`
    > associated with it.
    > + * - ``V4L2_EVENT_CODEC_ERROR``
    > + - 7
    > + - This event is triggered when some error occurs inside the codec engine,
    > + usually it can be replaced by a POLLERR event, but in some cases, the POLLERR
    > + may cause the application to exit, but this event can allow the application to
    > + handle the codec error without exiting.

    Events are sent to userspace in a separate queue from the VB2 queue. Which means
    it's impossible for userspace to know where this error actually took place.
    Userspace may endup discarding valid frames from the VB queue, as it does not
    know which one are good, and which one are bad.

    There is likely a bit of spec work to be done here for non-fatal decode errors,
    but I think the right approach is to use V4L2_BUF_FLAG_ERROR. What we expect
    from decoders is that for each frame, a CAPTURE buffer is assigned. If decoding
    that frame was not possible but the error is recoverable (corrupted bitstream,
    missing reference, etc.), then the failing frame get marked with FLAG_ERROR and
    decoding continues as usual.

    What isn't documented is that you can set bytesused to 0, meaning there is
    nothing useful in that frame, or a valid bytesused when you know only some
    blocks are broken (e.g. missing 1 ref). Though, GStreamer might be the only
    implementation of that, and byteused 0 may confuse some existing userspace.

    > + * - ``V4L2_EVENT_SKIP``
    > + - 8
    > + - This event is triggered when one frame is decoded, but it won't be outputed
    > + to the display. So the application can't get this frame, and the input frame count
    > + is dismatch with the output frame count. And this evevt is telling the client to
    > + handle this case.

    Similar to my previous comment, this event is flawed, since userspace cannot
    know were the skip is located in the queued buffers. Currently, all decoders are
    mandated to support V4L2_BUF_FLAG_TIMESTAMP_COPY. The timestamp must NOT be
    interpreted by the driver and must be reproduce as-is in the associated CAPTURE
    buffer. It is possible to "garbage" collect skipped frames with this method,
    though tedious.

    An alternative, and I think it would be much nicer then this, would be to use
    the v4l2_buffer.sequence counter, and just make it skip 1 on skips. Though, the
    down side is that userspace must also know how to reorder frames (a driver job
    for stateless codecs) in order to identify which frame was skipped. So this is
    perhaps not that useful, other then knowing something was skipped in the past.

    A third option would be to introduce V4L2_BUF_FLAG_SKIPPED. This way the driver
    could return an empty payload (bytesused = 0) buffer with this flag set, and the
    proper timestamp properly copied. This would let the driver communicate skipped
    frames in real-time. Note that this could break with existing userspace, so it
    would need to be opted-in somehow (a control or some flags).

    > * - ``V4L2_EVENT_PRIVATE_START``
    > - 0x08000000
    > - Base event number for driver-private events.
    > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
    > index 5bb0682b4a23..c56640d42dc5 100644
    > --- a/include/uapi/linux/videodev2.h
    > +++ b/include/uapi/linux/videodev2.h
    > @@ -2369,6 +2369,8 @@ struct v4l2_streamparm {
    > #define V4L2_EVENT_FRAME_SYNC 4
    > #define V4L2_EVENT_SOURCE_CHANGE 5
    > #define V4L2_EVENT_MOTION_DET 6
    > +#define V4L2_EVENT_CODEC_ERROR 7
    > +#define V4L2_EVENT_SKIP 8
    > #define V4L2_EVENT_PRIVATE_START 0x08000000
    >
    > /* Payload for V4L2_EVENT_VSYNC */


    \
     
     \ /
      Last update: 2021-09-08 15:33    [W:4.064 / U:0.048 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site