[FFmpeg-devel] [PATCHv6 4/4] libavcodec: v4l2: add support for v4l2 mem2mem codecs

wm4 nfxjfg at googlemail.com
Fri Aug 25 18:35:20 EEST 2017


That looks generally OK. Is there any chance a hwaccel approach would
be possible instead? If I've learned anything about hardware decoding,
then that hwaccel is vastly superior to vendor-implemented full stream
decoders.

I don't think I like the attempt of sharing the v4l helper functions
between libavdevice and libavcodec, but I can't tell how much it helps.

On Fri, 25 Aug 2017 13:22:48 +0200
Jorge Ramirez-Ortiz <jorge.ramirez-ortiz at linaro.org> wrote:


> +#define WIDTH(__ctx, __fmt) \
> +    (V4L2_TYPE_IS_MULTIPLANAR((__ctx)->type) ? __fmt.fmt.pix_mp.width : __fmt.fmt.pix.width)
> +
> +#define HEIGHT(__ctx, __fmt) \
> +    (V4L2_TYPE_IS_MULTIPLANAR((__ctx)->type) ? __fmt.fmt.pix_mp.height : __fmt.fmt.pix.height)

These names are a bit generic. Also, identifiers starting with __ are
always implementation reserved (i.e. using them undefined behavior).
You're forgetting to quote the __fmt macro parameter too.

> +static inline void set_pts(V4L2Buffer *out, int64_t pts)
> +{
> +    if (pts == AV_NOPTS_VALUE) {
> +        /* invalid timestamp: not sure how to handle this case */
> +        out->timestamp.tv_sec  = 0;
> +        out->timestamp.tv_usec = 0;
> +    } else {
> +        AVRational v4l2_timebase = { 1, 1000000 };
> +        int64_t v4l2_pts = av_rescale_q(pts, out->context->time_base, v4l2_timebase);
> +        out->timestamp.tv_sec  = v4l2_pts / INT64_C(1000000);
> +        out->timestamp.tv_usec = v4l2_pts % INT64_C(1000000);
> +    }
> +}

Why does it require a fixed timebase? A decoder shouldn't even look at
the timestamps, it should only pass them though. Also, not using DTS
will make it a nightmare to support containers like avi.

I suspect the decoder tries to "fix" timestamps, or maybe even does
something particularly bad like reordering frames by timestamps. This
is NOT something that should be in a kernel API.

(FFmpeg native decoders _and_ hwaccels pass through both PTS and DTS,
and don't touch their values.)

> +static void free_v4l2buf_cb(void *opaque, uint8_t *unused)
> +{
> +    V4L2Buffer* avbuf = opaque;
> +
> +    if (V4L2BUF_IN_DRIVER == avbuf->status)
> +        return;
> +
> +    if (V4L2_TYPE_IS_OUTPUT(avbuf->context->type))
> +        avbuf->status = V4L2BUF_AVAILABLE;
> +    else
> +       avbuf->context->ops.enqueue(avbuf);
> +}

> +static inline int buffer_ops_v4l2buf_to_bufref(V4L2Buffer *in, int plane, AVBufferRef **buf)
> +{
> +    if (plane >= in->num_planes)
> +        return AVERROR(EINVAL);
> +
> +    /* even though most encoders return 0 in data_offset encoding vp8 does require this value*/
> +    *buf = av_buffer_create((char *)in->plane_info[plane].mm_addr + in->planes[plane].data_offset,
> +                            in->plane_info[plane].lengths, free_v4l2buf_cb, in, 0);
> +    if (!*buf)
> +        return AVERROR(ENOMEM);
> +
> +    in->status = V4L2BUF_RET_USER;
> +
> +    return 0;
> +}

This looks like it would trigger massive UB if you keep a frame after
the decoder is closed.  This should not happen, an AVBufferRef must
stay valid forever. At least it looks like it assumes that the decoder
is somehow still around, without unreffing it, which hints towards that
this is done incorrectly.

> +static int buffer_ops_v4l2buf_to_avframe(AVFrame *frame, V4L2Buffer *avbuf)
> +{
> +    int i, ret;
> +
> +    av_frame_unref(frame);
> +
> +    /* 1. get references to the actual data */
> +    for (i = 0; i < avbuf->num_planes; i++) {
> +        ret = avbuf->ops.buf_to_bufref(avbuf, i, &frame->buf[i]);
> +        if (ret)
> +            return ret;
> +
> +        frame->linesize[i] = avbuf->bytesperline[i];
> +        frame->data[i] = frame->buf[i]->data;
> +    }
> +
> +    /* 1.1 fixup special cases */
> +    switch (avbuf->context->av_pix_fmt) {
> +    case AV_PIX_FMT_NV12:
> +        if (avbuf->num_planes > 1)
> +            break;
> +        frame->linesize[1] = avbuf->bytesperline[0];
> +        frame->data[1] = frame->buf[0]->data + avbuf->bytesperline[0] * avbuf->context->format.fmt.pix_mp.height;
> +        break;
> +    default:
> +        break;
> +    }
> +
> +    /* 2. get frame information */
> +    frame->key_frame = !!(avbuf->buf.flags & V4L2_BUF_FLAG_KEYFRAME);
> +    frame->format = avbuf->context->av_pix_fmt;
> +
> +    /* these values are updated also during re-init in process_video_event */
> +    frame->height = avbuf->context->height;
> +    frame->width = avbuf->context->width;
> +    frame->pts = get_pts(avbuf);
> +
> +    /* 3. report errors upstream */
> +    if (avbuf->buf.flags & V4L2_BUF_FLAG_ERROR) {
> +        av_log(avbuf->context->log_ctx, AV_LOG_ERROR, "%s: driver decode error\n", avbuf->context->name);
> +        frame->decode_error_flags |= FF_DECODE_ERROR_INVALID_BITSTREAM;
> +    }
> +
> +    return 0;
> +}

This function seems to lack setting typically required metadata like
colorspace.

> +static int buffer_ops_v4l2buf_to_avpkt(AVPacket *pkt, V4L2Buffer *avbuf)
> +{
> +    int ret;
> +
> +    av_packet_unref(pkt);
> +    ret = avbuf->ops.buf_to_bufref(avbuf, 0, &pkt->buf);
> +    if (ret)
> +        return ret;
> +
> +    pkt->size = V4L2_TYPE_IS_MULTIPLANAR(avbuf->context->type) ? avbuf->buf.m.planes[0].bytesused : avbuf->buf.bytesused;
> +    pkt->data = pkt->buf->data;
> +
> +    if (avbuf->buf.flags & V4L2_BUF_FLAG_KEYFRAME)
> +        pkt->flags |= AV_PKT_FLAG_KEY;
> +
> +    if (avbuf->buf.flags & V4L2_BUF_FLAG_ERROR) {
> +        av_log(avbuf->context->log_ctx, AV_LOG_ERROR, "%s driver encode error\n", avbuf->context->name);
> +        pkt->flags |= AV_PKT_FLAG_CORRUPT;
> +    }
> +
> +    pkt->pts = get_pts(avbuf);
> +
> +    return 0;
> +}

(Didn't check whether it's done correctly here, but again, AVPackets
must stay valid even if the encoder is closed.)

Why does this not set the DTS? The DTS is required for muxing with some
important containers.

> +static V4L2Buffer* context_ops_dequeue_v4l2buf(V4L2Context *ctx, unsigned int timeout)
> +{
> +    struct v4l2_plane planes[VIDEO_MAX_PLANES];
> +    struct v4l2_buffer buf = { 0 };
> +    V4L2Buffer* avbuf = NULL;
> +    struct pollfd pfd = {
> +        .events =  POLLIN | POLLRDNORM | POLLPRI, /* default capture context */
> +        .fd = ctx->fd,
> +    };
> +    int ret;
> +
> +    if (ctx->num_queued < ctx->min_queued_buffers)
> +        return NULL;
> +
> +    if (V4L2_TYPE_IS_OUTPUT(ctx->type))
> +        pfd.events =  POLLOUT | POLLWRNORM;
> +
> +    for (;;) {
> +        ret = poll(&pfd, 1, timeout);

Why does this have a timeout? This makes no sense: either a frame will
be decoded/packet can be encoded, or you need to give it more input
frames or packets, or an error happened.

> +/**
> + * Initializes a V4L2Context.
> + *
> + * @param[in] ctx A pointer to a V4L2Context. See V4L2Context description for required variables.
> + * @return 0 in case of success, a negative value representing the error otherwise.
> + */
> +int avpriv_v4l2_context_init(V4L2Context* ctx, int lazy_init);
> +
> +/**
> + * Releases a V4L2Context.
> + *
> + * @param[in] ctx A pointer to a V4L2Context.
> + *               The caller is reponsible for freeing it.
> + *               It must not be used after calling this function.
> + */
> +void avpriv_v4l2_context_release(V4L2Context* ctx);

We shouldn't add new avpriv functions. The situation is bad enough as
it is.

> +int avpriv_v4l2m2m_init(V4L2m2mContext* s, void* log_ctx)
> +{
> +    char *devname_save = s->devname;
> +    int ret = AVERROR(EINVAL);
> +    char node[PATH_MAX];
> +    struct dirent *entry;
> +    DIR *dirp;
> +
> +    if (s->devname && *s->devname)
> +        return configure_contexts(s, log_ctx);
> +
> +    dirp = opendir("/dev");
> +    if (!dirp)
> +        return AVERROR(errno);
> +
> +    for (entry = readdir(dirp); entry; entry = readdir(dirp)) {
> +
> +        if (strncmp(entry->d_name, "video", 5))
> +            continue;
> +
> +        snprintf(node, sizeof(node), "/dev/%s", entry->d_name);
> +
> +        av_log(log_ctx, AV_LOG_DEBUG, "probing device %s\n", node);
> +
> +        s->devname = node;
> +        ret = probe_v4l2_driver(s, log_ctx);
> +        if (!ret)
> +                break;
> +    }

This doesn't really look like a good idea. Even somehow trying to
enumerate stuff in /sys/class/ sounds better. Just because device
filename starts with "video" it doesn't need to be relevant, and poking
random ioctl()s at arbitrary devices doesn't sound very reliable.

> +    closedir(dirp);
> +
> +    if (!ret) {
> +        av_log(log_ctx, AV_LOG_INFO, "Using device %s\n", node);
> +        ret = configure_contexts(s, log_ctx);
> +    } else {
> +        av_log(log_ctx, AV_LOG_ERROR, "Could not find a valid device\n");
> +    }
> +    s->devname = devname_save;
> +
> +    return ret;
> +}
> +
> +int avpriv_v4l2m2m_reinit(V4L2Context* ctx)
> +{
> +    V4L2m2mContext *s = container_of(ctx, V4L2m2mContext, capture);
> +    int ret;
> +
> +    av_log(ctx->log_ctx, AV_LOG_DEBUG, "%s reinit context\n", ctx->name);
> +
> +    /* 1. streamoff */
> +    ret = avpriv_v4l2_context_set_status(ctx, VIDIOC_STREAMOFF);
> +    if (ret)
> +        av_log(ctx->log_ctx, AV_LOG_ERROR, "VIDIOC_STREAMOFF %s\n", ctx->name);
> +
> +    /* 2. unmap the buffers (v4l2 and ffmpeg) */
> +    avpriv_v4l2_context_release(ctx);

What happens to AVFrames or AVPackets that are still referenced?

> +
> +    /* 3. query the new format */
> +    ret = avpriv_v4l2m2m_format(ctx, 1);
> +    if (ret) {
> +        av_log(ctx->log_ctx, AV_LOG_ERROR, "setting %s format\n", ctx->name);
> +        return ret;
> +    }
> +
> +    /* 4. do lazy initialization */
> +    ret = avpriv_v4l2_context_init(ctx, ctx->lazy_init);
> +    if (ret) {
> +        av_log(ctx->log_ctx, AV_LOG_ERROR, "%s buffers lazy init\n", ctx->name);
> +        return ret;
> +    }
> +
> +    /* 5. update AVCodecContext after re-init */
> +    ret = ff_set_dimensions(s->avctx, ctx->width, ctx->height);
> +    if (ret < 0)
> +        av_log(ctx->log_ctx, AV_LOG_WARNING, "update avcodec height and width\n");
> +
> +    return 0;
> +}
> +
> +int ff_v4l2m2m_codec_end(AVCodecContext *avctx)
> +{
> +    V4L2m2mContext *s = avctx->priv_data;
> +
> +    av_log(avctx, AV_LOG_DEBUG, "Closing context\n");
> +
> +    return avpriv_v4l2m2m_end(s);
> +}
> +
> +int ff_v4l2m2m_codec_init(AVCodecContext *avctx)
> +{
> +    V4L2m2mContext *s = avctx->priv_data;
> +    s->avctx = avctx;
> +
> +    return avpriv_v4l2m2m_init(s, avctx);
> +}
> +
> diff --git a/libavcodec/v4l2_m2m.h b/libavcodec/v4l2_m2m.h
> new file mode 100644
> index 0000000..41cd6e9
> --- /dev/null
> +++ b/libavcodec/v4l2_m2m.h
> @@ -0,0 +1,59 @@
> +/*
> + * V4L2 mem2mem helper functions
> + *
> + * Copyright (C) 2017 Alexis Ballier <aballier at gentoo.org>
> + * Copyright (C) 2017 Jorge Ramirez <jorge.ramirez-ortiz at linaro.org>
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#ifndef AVCODEC_V4L2_M2M_H
> +#define AVCODEC_V4L2_M2M_H
> +
> +#include "v4l2_buffers.h"
> +#include "v4l2_fmt.h"
> +
> +#define container_of(ptr, type, member) ({ \
> +        const __typeof__(((type *)0)->member ) *__mptr = (ptr); \
> +        (type *)((char *)__mptr - offsetof(type,member) );})
> +
> +#define V4L_M2M_DEFAULT_OPTS \
> +    { "device",\
> +      "Path to the device to use",\
> +        OFFSET(devname), AV_OPT_TYPE_STRING, {.str = NULL }, 0, 0, FLAGS },\
> +    { "num_output_buffers",\
> +      "Number of buffers in the output context",\
> +        OFFSET(output.num_buffers), AV_OPT_TYPE_INT, { .i64 = 16 }, 4, INT_MAX, FLAGS }
> +
> +typedef struct V4L2m2mContext
> +{
> +    AVClass *class;
> +    int fd;
> +    char *devname;
> +    struct v4l2_capability cap;
> +    V4L2Context capture;
> +    V4L2Context output;
> +    /* update codec while dynamic stream reconfig */
> +    AVCodecContext *avctx;
> +} V4L2m2mContext;
> +
> +int avpriv_v4l2m2m_init(V4L2m2mContext *ctx, void* log_ctx);
> +int avpriv_v4l2m2m_reinit(V4L2Context *ctx);
> +int avpriv_v4l2m2m_format(V4L2Context *ctx, int set);
> +int avpriv_v4l2m2m_end(V4L2m2mContext *ctx);
> +
> +#endif /* AVCODEC_V4L2_M2M_H */
> diff --git a/libavcodec/v4l2_m2m_avcodec.h b/libavcodec/v4l2_m2m_avcodec.h
> new file mode 100644
> index 0000000..3e17ae9
> --- /dev/null
> +++ b/libavcodec/v4l2_m2m_avcodec.h
> @@ -0,0 +1,32 @@
> +/*
> + * V4L2 mem2mem avcodec helper functions
> + *
> + * Copyright (C) 2017 Alexis Ballier <aballier at gentoo.org>
> + * Copyright (C) 2017 Jorge Ramirez <jorge.ramirez-ortiz at linaro.org>
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#ifndef AVCODEC_V4L2_M2M_AVCODEC_H
> +#define AVCODEC_V4L2_M2M_AVCODEC_H
> +
> +#include "avcodec.h"
> +
> +int ff_v4l2m2m_codec_init(AVCodecContext *avctx);
> +int ff_v4l2m2m_codec_end(AVCodecContext *avctx);
> +
> +#endif
> diff --git a/libavcodec/v4l2_m2m_dec.c b/libavcodec/v4l2_m2m_dec.c
> new file mode 100644
> index 0000000..19fef72
> --- /dev/null
> +++ b/libavcodec/v4l2_m2m_dec.c
> @@ -0,0 +1,252 @@
> +/*
> + * V4L2 mem2mem decoders
> + *
> + * Copyright (C) 2017 Alexis Ballier <aballier at gentoo.org>
> + * Copyright (C) 2017 Jorge Ramirez <jorge.ramirez-ortiz at linaro.org>
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#include <sys/ioctl.h>
> +#include "libavutil/pixfmt.h"
> +#include "libavutil/pixdesc.h"
> +#include "libavutil/opt.h"
> +#include "v4l2_m2m_avcodec.h"
> +#include "v4l2_fmt.h"
> +#include "v4l2_buffers.h"
> +#include "v4l2_m2m.h"
> +#include "decode.h"
> +#include "avcodec.h"
> +
> +static int try_start(AVCodecContext *avctx)
> +{
> +    V4L2m2mContext *s = avctx->priv_data;
> +    V4L2Context *const capture = &s->capture;
> +    V4L2Context *const output = &s->output;
> +    struct v4l2_event_subscription sub;
> +    struct v4l2_selection selection;
> +    struct v4l2_control ctrl;
> +    int ret;
> +
> +    if (output->streamon && capture->streamon)
> +        return 0;
> +
> +    /* 0. subscribe to source change event */
> +    memset(&sub, 0, sizeof(sub));
> +    sub.type = V4L2_EVENT_SOURCE_CHANGE;
> +    ret = ioctl(s->fd, VIDIOC_SUBSCRIBE_EVENT, &sub);
> +    if ( ret < 0)
> +        av_log(avctx, AV_LOG_WARNING, "decoding does not support resolution change\n");
> +
> +    /* 1. start the output process */
> +    if (!output->streamon) {
> +        ret = avpriv_v4l2_context_set_status(output, VIDIOC_STREAMON);
> +        if (ret < 0) {
> +            av_log(avctx, AV_LOG_DEBUG, "VIDIOC_STREAMON on output context\n");
> +            return ret;
> +        }
> +    }
> +
> +    /* 2. get the capture format */
> +    capture->format.type = capture->type;
> +    ret = ioctl(capture->fd, VIDIOC_G_FMT, &capture->format);
> +    if (ret) {
> +        av_log(avctx, AV_LOG_ERROR, "VIDIOC_G_FMT ioctl\n");
> +        return ret;
> +    }
> +
> +    /* 2.1 update the AVCodecContext */
> +    avctx->pix_fmt = avpriv_v4l_fmt_v4l2ff(capture->format.fmt.pix_mp.pixelformat, AV_CODEC_ID_RAWVIDEO);
> +    capture->av_pix_fmt = avctx->pix_fmt;
> +
> +    /* 3. set the crop parameters */
> +    selection.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
> +    selection.r.height = avctx->coded_height;
> +    selection.r.width = avctx->coded_width;
> +    ret = ioctl(s->fd, VIDIOC_S_SELECTION, &selection);
> +    if (!ret) {
> +        ret = ioctl(s->fd, VIDIOC_G_SELECTION, &selection);
> +        if (ret) {
> +            av_log(avctx, AV_LOG_ERROR, "VIDIOC_G_SELECTION ioctl\n");
> +        } else {
> +            av_log(avctx, AV_LOG_DEBUG, "crop output %dx%d\n", selection.r.width, selection.r.height);
> +            /* update the size of the resulting frame */
> +            capture->height = selection.r.height;
> +            capture->width  = selection.r.width;
> +        }
> +    }
> +
> +    /* 4. get the minimum number of buffers required by capture (or default) */
> +    memset(&ctrl, 0, sizeof(ctrl));
> +    ctrl.id = V4L2_CID_MIN_BUFFERS_FOR_CAPTURE;
> +    ret = ioctl(s->fd, VIDIOC_G_CTRL, &ctrl);
> +    if (!ret) 
> +        capture->min_queued_buffers = ctrl.value;
> +
> +    /* 5. init the capture context now that we have the capture format */
> +    if (!capture->buffers) {
> +
> +        av_log(capture->log_ctx, AV_LOG_DEBUG, "%s requested (%dx%d)\n",
> +            capture->name, capture->format.fmt.pix_mp.width, capture->format.fmt.pix_mp.height);
> +
> +        ret = avpriv_v4l2_context_init(capture, 0);
> +        if (ret) {
> +            av_log(avctx, AV_LOG_DEBUG, "can't request output buffers\n");
> +            return ret;
> +        }
> +    }
> +
> +    /* 6. start the capture process */
> +    ret = avpriv_v4l2_context_set_status(capture, VIDIOC_STREAMON);
> +    if (ret) {
> +        av_log(avctx, AV_LOG_DEBUG, "VIDIOC_STREAMON, on capture context\n");
> +        return ret;
> +    }
> +
> +    return 0;
> +}
> +
> +static av_cold int v4l2m2m_decode_init(AVCodecContext *avctx)
> +{
> +    V4L2m2mContext *s = avctx->priv_data;
> +    V4L2Context *capture = &s->capture;
> +    V4L2Context *output = &s->output;
> +
> +    output->default_flags = capture->default_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> +    output->time_base = capture->time_base = avctx->framerate;
> +    output->height = capture->height = avctx->coded_height;
> +    output->width = capture->width =avctx->coded_width;
> +
> +    output->av_codec_id = avctx->codec_id;
> +    output->av_pix_fmt  = AV_PIX_FMT_NONE;
> +    output->min_queued_buffers = 6;
> +
> +    /*
> +     * increase the number of buffers to support h.264/h.265
> +     * default (configurable via option) is 16
> +     */
> +    switch (avctx->codec_id) {
> +    case AV_CODEC_ID_H264:
> +    case AV_CODEC_ID_HEVC:
> +        output->num_buffers += 4;
> +        break;
> +     }

That sounds questionable. The maximum DPB for h264 and HEVC are usually
put at 16. So it should add 14 buffers, assuming traditional MPEG
codecs use 2. VP9 uses something between.

> +
> +    /*
> +     * the buffers associated to this context cant be initialized without
> +     * additional information available in the kernel driver, so do let's postpone it
> +     */
> +    capture->lazy_init = 1;
> +
> +    capture->av_codec_id = AV_CODEC_ID_RAWVIDEO;
> +    capture->av_pix_fmt = avctx->pix_fmt;
> +    capture->min_queued_buffers = 6;
> +
> +    return ff_v4l2m2m_codec_init(avctx);
> +}
> +
> +static int v4l2m2m_receive_frame(AVCodecContext *avctx, AVFrame *frame)
> +{
> +    V4L2m2mContext *s = avctx->priv_data;
> +    V4L2Context *const capture = &s->capture;
> +    V4L2Context *const output = &s->output;
> +    unsigned int timeout = 50;
> +    AVPacket avpkt = {0};

I think we've been arguing that we should avoid allocating AVPackets on
the stack because we want to remove sizeof(AVPacket) from the ABI, but
I guess it doesn't actually matter for now.

> +    int ret;
> +
> +    ret = ff_decode_get_packet(avctx, &avpkt);
> +    if (ret < 0 && ret != AVERROR_EOF)
> +        return ret;
> +
> +    ret = avpriv_v4l2_enqueue_packet(output, &avpkt);
> +    if (ret < 0)
> +        return ret;
> +
> +    if (avpkt.size) {
> +        ret = try_start(avctx);
> +        if (ret)
> +            return 0;
> +    }
> +
> +    return avpriv_v4l2_dequeue_frame(capture, frame, timeout);
> +}
> +
> +#define OFFSET(x) offsetof(V4L2m2mContext, x)
> +#define FLAGS AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_DECODING_PARAM
> +
> +        static const AVOption options[] = {
> +        V4L_M2M_DEFAULT_OPTS,{ "num_capture_extra_buffers","Number of extra buffers in the capture context",
> +        OFFSET(capture.num_buffers), AV_OPT_TYPE_INT,{.i64 = 6}, 6, INT_MAX, FLAGS},
> +        { NULL},
> +        };
> +
> +#define M2MDEC(NAME, LONGNAME, CODEC, bsf_name) \
> +static const AVClass v4l2_m2m_ ## NAME ## _dec_class = {\
> +    .class_name = #NAME "_v4l2_m2m_decoder",\
> +    .item_name  = av_default_item_name,\
> +    .option     = options,\
> +    .version    = LIBAVUTIL_VERSION_INT,\
> +};\
> +\
> +AVCodec ff_ ## NAME ## _v4l2m2m_decoder = { \
> +    .name           = #NAME "_v4l2m2m" ,\
> +    .long_name      = NULL_IF_CONFIG_SMALL("V4L2 mem2mem " LONGNAME " decoder wrapper"),\
> +    .type           = AVMEDIA_TYPE_VIDEO,\
> +    .id             = CODEC ,\
> +    .priv_data_size = sizeof(V4L2m2mContext),\
> +    .priv_class     = &v4l2_m2m_ ## NAME ## _dec_class,\
> +    .init           = v4l2m2m_decode_init,\
> +    .receive_frame  = v4l2m2m_receive_frame,\
> +    .close          = ff_v4l2m2m_codec_end,\
> +    .bsfs           = bsf_name, \
> +};
> +
> +#if CONFIG_H263_V4L2M2M_DECODER
> +        M2MDEC(h263, "H.263", AV_CODEC_ID_H263, NULL);
> +#endif
> +
> +#if CONFIG_H264_V4L2M2M_DECODER
> +        M2MDEC(h264, "H.264", AV_CODEC_ID_H264, "h264_mp4toannexb");
> +#endif
> +
> +#if CONFIG_MPEG1_V4L2M2M_DECODER
> +        M2MDEC(mpeg1, "MPEG1", AV_CODEC_ID_MPEG1VIDEO, NULL);
> +#endif
> +
> +#if CONFIG_MPEG2_V4L2M2M_DECODER
> +        M2MDEC(mpeg2, "MPEG2", AV_CODEC_ID_MPEG2VIDEO, NULL);
> +#endif
> +
> +#if CONFIG_MPEG4_V4L2M2M_DECODER
> +        M2MDEC(mpeg4, "MPEG4", AV_CODEC_ID_MPEG4, NULL);
> +#endif
> +
> +#if CONFIG_VC1_V4L2M2M_DECODER
> +        M2MDEC(vc1 , "VC1", AV_CODEC_ID_VC1, NULL);
> +#endif
> +
> +#if CONFIG_VP8_V4L2M2M_DECODER
> +        M2MDEC(vp8, "VP8", AV_CODEC_ID_VP8, NULL);
> +#endif
> +
> +#if CONFIG_VP9_V4L2M2M_DECODER
> +        M2MDEC(vp9, "VP9", AV_CODEC_ID_VP9, NULL);
> +#endif
> +
> +#if CONFIG_HEVC_V4L2M2M_DECODER
> +        M2MDEC(hevc, "HEVC", AV_CODEC_ID_HEVC, "h264_mp4toannexb");
> +#endif

You don't need all this ifdeffery. A codec entry doesn't pull in
any additional functions, so a disabled codec entry will simply not
reference the AVCodec.

> diff --git a/libavcodec/v4l2_m2m_enc.c b/libavcodec/v4l2_m2m_enc.c
> new file mode 100644
> index 0000000..db9c090
> --- /dev/null
> +++ b/libavcodec/v4l2_m2m_enc.c
> @@ -0,0 +1,288 @@
> +/*
> + * V4L2 mem2mem encoders
> + *
> + * Copyright (C) 2017 Alexis Ballier <aballier at gentoo.org>
> + * Copyright (C) 2017 Jorge Ramirez <jorge.ramirez-ortiz at linaro.org>
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#include <sys/ioctl.h>
> +
> +#include "libavutil/pixfmt.h"
> +#include "libavutil/pixdesc.h"
> +#include "libavutil/opt.h"
> +#include "v4l2_m2m_avcodec.h"
> +#include "v4l2_buffers.h"
> +#include "v4l2_fmt.h"
> +#include "v4l2_m2m.h"
> +#include "avcodec.h"
> +
> +#define STR(s) AV_TOSTRING(s)
> +#define MPEG_CID(x) V4L2_CID_MPEG_VIDEO_##x
> +#define MPEG_VIDEO(x) V4L2_MPEG_VIDEO_##x
> +
> +#define SET_V4L2_EXT_CTRL(TYPE, ID, VALUE, NAME)                    \
> +{                                                                   \
> +    struct v4l2_ext_control ctrl = { 0 };                           \
> +    struct v4l2_ext_controls ctrls = { 0 };                         \
> +    ctrls.ctrl_class = V4L2_CTRL_CLASS_MPEG;                        \
> +    ctrls.controls = &ctrl;                                         \
> +    ctrl.TYPE = VALUE ;                                             \
> +    ctrl.id = ID ;                                                  \
> +    ctrls.count = 1;                                                \
> +                                                                    \
> +    if ((ret = ioctl(s->fd, VIDIOC_S_EXT_CTRLS, &ctrls)) < 0)       \
> +        av_log(avctx, AV_LOG_WARNING, "Failed to set " NAME "%s\n", STR(ID));  \
> +}
> +
> +#define SET_V4L2_TIME_PER_FRAME(NUM, DEN)                           \
> +{                                                                   \
> +    struct v4l2_streamparm parm = { 0 };                            \
> +    parm.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;                  \
> +    parm.parm.output.timeperframe.numerator = NUM;                  \
> +    parm.parm.output.timeperframe.denominator = DEN;                \
> +                                                                    \
> +    if ((ret = ioctl(s->fd, VIDIOC_S_PARM, &parm)) < 0)             \
> +        av_log(avctx, AV_LOG_WARNING, "Failed to set  timeperframe");  \
> +}

I think those should be functions. ctrl has only 3 fields, so it's ok
to have the caller set it up. (If you use C99 struct literals it won't
be more code than before, maybe.)

> +
> +static inline int v4l2_h264_profile_from_ff(int p)
> +{
> +    switch(p) {
> +    case FF_PROFILE_H264_CONSTRAINED_BASELINE:
> +        return MPEG_VIDEO(H264_PROFILE_CONSTRAINED_BASELINE);
> +    case FF_PROFILE_H264_HIGH_444_PREDICTIVE:
> +        return MPEG_VIDEO(H264_PROFILE_HIGH_444_PREDICTIVE);
> +    case FF_PROFILE_H264_HIGH_422_INTRA:
> +        return MPEG_VIDEO(H264_PROFILE_HIGH_422_INTRA);
> +    case FF_PROFILE_H264_HIGH_444_INTRA:
> +        return MPEG_VIDEO(H264_PROFILE_HIGH_444_INTRA);
> +    case FF_PROFILE_H264_HIGH_10_INTRA:
> +        return MPEG_VIDEO(H264_PROFILE_HIGH_10_INTRA);
> +    case FF_PROFILE_H264_HIGH_422:
> +        return MPEG_VIDEO(H264_PROFILE_HIGH_422);
> +    case FF_PROFILE_H264_BASELINE:
> +        return MPEG_VIDEO(H264_PROFILE_BASELINE);
> +    case FF_PROFILE_H264_EXTENDED:
> +        return MPEG_VIDEO(H264_PROFILE_EXTENDED);
> +    case FF_PROFILE_H264_HIGH_10:
> +        return MPEG_VIDEO(H264_PROFILE_HIGH_10);
> +    case FF_PROFILE_H264_MAIN:
> +        return MPEG_VIDEO(H264_PROFILE_MAIN);
> +    case FF_PROFILE_H264_HIGH:
> +        return MPEG_VIDEO(H264_PROFILE_HIGH);
> +    }
> +
> +    return -1;
> +}
> +
> +static inline int v4l2_mpeg4_profile_from_ff(int p)
> +{
> +    switch(p) {
> +    case FF_PROFILE_MPEG4_ADVANCED_CODING:
> +        return MPEG_VIDEO(MPEG4_PROFILE_ADVANCED_CODING_EFFICIENCY);
> +    case FF_PROFILE_MPEG4_ADVANCED_SIMPLE:
> +        return MPEG_VIDEO(MPEG4_PROFILE_ADVANCED_SIMPLE);
> +    case FF_PROFILE_MPEG4_SIMPLE_SCALABLE:
> +
> +        return MPEG_VIDEO(MPEG4_PROFILE_SIMPLE_SCALABLE);
> +    case FF_PROFILE_MPEG4_SIMPLE:
> +        return MPEG_VIDEO(MPEG4_PROFILE_SIMPLE);
> +    case FF_PROFILE_MPEG4_CORE:
> +        return MPEG_VIDEO(MPEG4_PROFILE_CORE);
> +    }
> +
> +    return -1;
> +}

Would a table be better maybe?

> +
> +static av_cold int v4l2m2m_encode_init(AVCodecContext *avctx)
> +{
> +    V4L2m2mContext *s = avctx->priv_data;
> +    V4L2Context *capture = &s->capture;
> +    V4L2Context *output = &s->output;
> +    int qmin_cid, qmax_cid, ret, val;
> +    int qmin, qmax;
> +
> +    output->default_flags = capture->default_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> +    output->time_base = capture->time_base = avctx->time_base;
> +    output->height = capture->height = avctx->height;
> +    output->width = capture->width = avctx->width;
> +
> +    /* output context */
> +    output->av_codec_id = AV_CODEC_ID_RAWVIDEO;
> +    output->av_pix_fmt = avctx->pix_fmt;
> +
> +    /* capture context */
> +    capture->av_codec_id = avctx->codec_id;
> +    capture->av_pix_fmt = AV_PIX_FMT_NONE;
> +    capture->min_queued_buffers = 1;
> +
> +    if (ret = ff_v4l2m2m_codec_init(avctx))
> +        return ret;
> +
> +    SET_V4L2_TIME_PER_FRAME(avctx->framerate.num, avctx->framerate.den);
> +    SET_V4L2_EXT_CTRL(value, MPEG_CID(HEADER_MODE), MPEG_VIDEO(HEADER_MODE_SEPARATE), "header mode");
> +    SET_V4L2_EXT_CTRL(value, MPEG_CID(B_FRAMES), avctx->max_b_frames,  "number of B-frames");
> +    SET_V4L2_EXT_CTRL(value, MPEG_CID(GOP_SIZE), avctx->gop_size,"gop size");
> +    SET_V4L2_EXT_CTRL(value, MPEG_CID(BITRATE) , avctx->bit_rate, "bit rate");
> +
> +    av_log(avctx, AV_LOG_DEBUG, "Encoder Context: frame rate(%d/%d), number b-frames (%d),"
> +          "gop size (%d), bit rate (%ld), qmin (%d), qmax (%d)\n",
> +        avctx->framerate.num, avctx->framerate.den, avctx->max_b_frames, avctx->gop_size, avctx->bit_rate, avctx->qmin, avctx->qmax);
> +
> +    switch(avctx->codec_id) {
> +    case AV_CODEC_ID_H264:
> +        val = v4l2_h264_profile_from_ff(avctx->profile);
> +        if (val >= 0) {
> +            SET_V4L2_EXT_CTRL(value, MPEG_CID(H264_PROFILE), val, "h264 profile");
> +        }
> +        qmin_cid = MPEG_CID(H264_MIN_QP);
> +        qmax_cid = MPEG_CID(H264_MAX_QP);
> +        qmin = 0;
> +        qmax = 51;
> +        break;
> +    case AV_CODEC_ID_MPEG4:
> +        val = v4l2_mpeg4_profile_from_ff(avctx->profile);
> +        if (val >= 0) {
> +            SET_V4L2_EXT_CTRL(value, MPEG_CID(MPEG4_PROFILE), val, "mpeg4 profile");
> +        }
> +        qmin_cid = MPEG_CID(MPEG4_MIN_QP);
> +        qmax_cid = MPEG_CID(MPEG4_MAX_QP);
> +        if (avctx->flags & CODEC_FLAG_QPEL) {
> +            SET_V4L2_EXT_CTRL(value, MPEG_CID(MPEG4_QPEL), 1, "qpel");
> +        }
> +        qmax = 51;
> +        qmin = 0;
> +        break;
> +    case AV_CODEC_ID_H263:
> +        qmin_cid = MPEG_CID(H263_MIN_QP);
> +        qmax_cid = MPEG_CID(H263_MAX_QP);
> +        qmin = 1;
> +        qmax = 31;
> +        break;
> +    case AV_CODEC_ID_VP8:
> +        qmin_cid = MPEG_CID(VPX_MIN_QP);
> +        qmax_cid = MPEG_CID(VPX_MAX_QP);
> +        qmin = 0;
> +        qmax = 127;
> +        break;
> +    case AV_CODEC_ID_VP9:
> +        qmin_cid = MPEG_CID(VPX_MIN_QP);
> +        qmax_cid = MPEG_CID(VPX_MAX_QP);
> +        qmin = 0;
> +        qmax = 255;
> +        break;
> +    default:
> +        return 0;
> +    }
> +
> +    if (qmin != avctx->qmin || qmax != avctx->qmax)
> +        av_log(avctx, AV_LOG_WARNING, "Encoder adjusted: qmin (%d), qmax (%d)\n", qmin, qmax);
> +
> +    SET_V4L2_EXT_CTRL(value, qmin_cid, qmin, "minimum video quantizer scale");
> +    SET_V4L2_EXT_CTRL(value, qmax_cid, qmax, "maximum video quantizer scale");
> +
> +    return 0;
> +}
> +
> +static int v4l2m2m_send_frame(AVCodecContext *avctx, const AVFrame *frame)
> +{
> +    V4L2m2mContext *s = avctx->priv_data;
> +    V4L2Context *const output = &s->output;
> +
> +    return avpriv_v4l2_enqueue_frame(output, frame);
> +}
> +
> +static int v4l2m2m_receive_packet(AVCodecContext *avctx, AVPacket *avpkt)
> +{
> +    V4L2m2mContext *s = avctx->priv_data;
> +    V4L2Context *const capture = &s->capture;
> +    V4L2Context *const output = &s->output;
> +    unsigned int timeout = 50;
> +    int ret;
> +
> +    if (!output->streamon) {
> +        ret = avpriv_v4l2_context_set_status(output, VIDIOC_STREAMON);
> +        if (ret) {
> +            av_log(avctx, AV_LOG_ERROR, "VIDIOC_STREAMOFF failed on output context\n");
> +            return ret;
> +        }
> +    }
> +
> +    if (!capture->streamon) {
> +        ret = avpriv_v4l2_context_set_status(capture, VIDIOC_STREAMON);
> +        if (ret) {
> +            av_log(avctx, AV_LOG_ERROR, "VIDIOC_STREAMON failed on capture context\n");
> +            return ret;
> +        }
> +    }
> +
> +    return avpriv_v4l2_dequeue_packet(capture, avpkt, timeout);
> +}
> +
> +#define OFFSET(x) offsetof(V4L2m2mContext, x)
> +#define FLAGS AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_ENCODING_PARAM
> +
> +static const AVOption options[] = {
> +    V4L_M2M_DEFAULT_OPTS,
> +    { "num_capture_buffers", "Number of buffers in the capture context",
> +        OFFSET(capture.num_buffers), AV_OPT_TYPE_INT, {.i64 = 4 }, 4, INT_MAX, FLAGS },
> +    { NULL },
> +};
> +
> +#define M2MENC(NAME, LONGNAME, CODEC) \
> +static const AVClass v4l2_m2m_ ## NAME ## _enc_class = {\
> +    .class_name = #NAME "_v4l2_m2m_encoder",\
> +    .item_name  = av_default_item_name,\
> +    .option     = options,\
> +    .version    = LIBAVUTIL_VERSION_INT,\
> +};\
> +\
> +AVCodec ff_ ## NAME ## _v4l2m2m_encoder = { \
> +    .name           = #NAME "_v4l2m2m" ,\
> +    .long_name      = NULL_IF_CONFIG_SMALL("V4L2 mem2mem " LONGNAME " encoder wrapper"),\
> +    .type           = AVMEDIA_TYPE_VIDEO,\
> +    .id             = CODEC ,\
> +    .priv_data_size = sizeof(V4L2m2mContext),\
> +    .priv_class     = &v4l2_m2m_ ## NAME ##_enc_class,\
> +    .init           = v4l2m2m_encode_init,\
> +    .send_frame     = v4l2m2m_send_frame,\
> +    .receive_packet = v4l2m2m_receive_packet,\
> +    .close          = ff_v4l2m2m_codec_end,\
> +};
> +
> +#if CONFIG_H263_V4L2M2M_ENCODER
> +M2MENC(h263, "H.263", AV_CODEC_ID_H263);
> +#endif
> +
> +#if CONFIG_H264_V4L2M2M_ENCODER
> +M2MENC(h264, "H.264", AV_CODEC_ID_H264);
> +#endif
> +
> +#if CONFIG_MPEG4_V4L2M2M_ENCODER
> +M2MENC(mpeg4, "MPEG4", AV_CODEC_ID_MPEG4);
> +#endif
> +
> +#if CONFIG_VP8_V4L2M2M_ENCODER
> +M2MENC(vp8, "VP8", AV_CODEC_ID_VP8);
> +#endif
> +
> +#if CONFIG_HEVC_V4L2M2M_ENCODER
> +M2MENC(hevc, "HEVC", AV_CODEC_ID_HEVC);
> +#endif
> +

Same comment about the ifdeffery as on the decoder side.


More information about the ffmpeg-devel mailing list