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

[Jorge Ramirez]

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.


>
>> +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);
>> +}