[FFmpeg-devel] [PATCHv8] libavcodec: v4l2: add support for v4l2 mem2mem codecs

[Mark Thompson]

On 03/09/17 14:10, Jorge Ramirez wrote:
> On 09/03/2017 02:27 AM, Mark Thompson wrote:
>>> +        av_log(s->avctx, AV_LOG_ERROR, "setting capture format\n");
>>> +        return ret;
>>> +    }
>>> +
>>> +    /* 5. do lazy initialization */
>>> +    ret = ff_v4l2_context_init(&s->capture, s->capture.lazy_init);
>>> +    if (ret) {
>>> +        av_log(s->avctx, AV_LOG_ERROR, "capture buffers lazy init\n");
>>> +        return ret;
>>> +    }
>>> +
>>> +    /* 6. update AVCodecContext */
>>> +    ret = ff_set_dimensions(s->avctx, s->capture.width, s->capture.height);
>>> +    if (ret < 0)
>>> +        av_log(s->avctx, AV_LOG_WARNING, "update avcodec height and width\n");
>>> +
>>> +    /* 7. complete reinit */
>>> +    sem_destroy(&s->refsync);
>>> +    sem_init(&s->refsync, 0, 0);
>>> +    s->draining = 0;
>>> +    s->reinit = 0;
>>> +
>>> +    return 0;
>>> +}
>> Seems like it might be nicer if you just passed the AVCodecContext to all of these functions rather than separate V4L2m2mContext and log context.  (Feel free to ignore that.)
> 
> sure I accept your input.
> even though from where I am coming from (less ffmpeg experience) I think it increases the semantic distance in the header file maybe making it a bit harder to read  (now it will look as below)
> 
> 
> typedef struct V4L2m2mContext
> {
>     AVClass *class;
>     int fd;
>     char *devname;
> 
>     /* the codec context queues */
>     V4L2Context capture;
>     V4L2Context output;
> 
>     /* refcount of buffers held by the user */
>     atomic_uint refcount;
> 
>     /* dynamic stream reconfig */
>     AVCodecContext *avctx;
>     sem_t refsync;
>     int reinit;
> 
>     /* null frame or packet received */
>     int draining;
> } V4L2m2mContext;
> 
> int ff_v4l2_m2m_codec_init(AVCodecContext *);
> int ff_v4l2_m2m_codec_reinit(AVCodecContext *);
> int ff_v4l2_m2m_codec_end(AVCodecContext *);
> int ff_v4l2_m2m_codec_format_context(V4L2Context *ctx, int set);
> 
> while in the past all functions accepted the V4L2m2mContext only needing the AVCodecContext in the marginal case when the buffers need to be reinit while encoding/decoding due to new sizes/formats....
> 
> but sure will do.

It isn't just those functions - it's all of the ones of the form "foo(V4L2m2mContext* s, void* log_ctx)" as well.

Still, do ignore it if you prefer your way.

> 
> 
>>
>>> +int ff_v4l2m2m_codec_end(AVCodecContext *avctx)
>>> +{
>>> +    V4L2m2mContext *s = avctx->priv_data;
>>> +
>>> +    av_log(avctx, AV_LOG_DEBUG, "Closing context\n");
>>> +
>>> +    return ff_v4l2_m2m_codec_end(s);
>>> +}
>>> +
>>> +int ff_v4l2m2m_codec_init(AVCodecContext *avctx)
>>> +{
>>> +    V4L2m2mContext *s = avctx->priv_data;
>>> +    s->avctx = avctx;
>>> +
>>> +    return ff_v4l2_m2m_codec_init(s, avctx);
>>> +}
> 
>