[FFmpeg-devel] [PATCH] avcodec/v4l2_m2m_enc: Adapt to the new internal encode API

[Andriy Gelman]

On Sat, 07. Mar 23:13, James Almer wrote:
> On 3/7/2020 9:23 PM, Andriy Gelman wrote:
> > From: Andriy Gelman <andriy.gelman at gmail.com>
> > 
> > Should be squashed with:
> > http://ffmpeg.org/pipermail/ffmpeg-devel/2020-February/257735.html
> 
> Thank you! Only three remain now :)
> 
> > 
> > Signed-off-by: Andriy Gelman <andriy.gelman at gmail.com>
> > ---
> >  libavcodec/v4l2_m2m.c     |  9 ++++++++-
> >  libavcodec/v4l2_m2m.h     |  6 +++++-
> >  libavcodec/v4l2_m2m_dec.c |  2 +-
> >  libavcodec/v4l2_m2m_enc.c | 17 +++++++++++++++--
> >  4 files changed, 29 insertions(+), 5 deletions(-)
> > 
> > diff --git a/libavcodec/v4l2_m2m.c b/libavcodec/v4l2_m2m.c
> > index 2d21f910bcc..1799837fb9f 100644
> > --- a/libavcodec/v4l2_m2m.c
> > +++ b/libavcodec/v4l2_m2m.c
> > @@ -329,6 +329,7 @@ static void v4l2_m2m_destroy_context(void *opaque, uint8_t *context)
> >      sem_destroy(&s->refsync);
> >  
> >      close(s->fd);
> > +    av_frame_free(&s->frame);
> >  
> >      av_free(s);
> >  }
> > @@ -394,7 +395,7 @@ int ff_v4l2_m2m_codec_init(V4L2m2mPriv *priv)
> >      return v4l2_configure_contexts(s);
> >  }
> >  
> > -int ff_v4l2_m2m_create_context(V4L2m2mPriv *priv, V4L2m2mContext **s)
> > +int ff_v4l2_m2m_create_context(V4L2m2mPriv *priv, int is_encoder, V4L2m2mContext **s)
> >  {
> >      *s = av_mallocz(sizeof(V4L2m2mContext));
> >      if (!*s)
> > @@ -417,5 +418,11 @@ int ff_v4l2_m2m_create_context(V4L2m2mPriv *priv, V4L2m2mContext **s)
> >      priv->context->self_ref = priv->context_ref;
> >      priv->context->fd = -1;
> >  
> > +    if (is_encoder) {
> > +        priv->context->frame = av_frame_alloc();
> > +        if (!priv->context->frame)
> > +            return AVERROR(ENOMEM);
> > +    }
> > +
> >      return 0;
> >  }
> > diff --git a/libavcodec/v4l2_m2m.h b/libavcodec/v4l2_m2m.h
> > index 456281f48c5..5d6106224dd 100644
> > --- a/libavcodec/v4l2_m2m.h
> > +++ b/libavcodec/v4l2_m2m.h
> > @@ -58,6 +58,9 @@ typedef struct V4L2m2mContext {
> >      int draining;
> >      AVPacket buf_pkt;
> >  
> > +    /* Reference to a frame. Only used during encoding */
> > +    AVFrame *frame;
> > +
> >      /* Reference to self; only valid while codec is active. */
> >      AVBufferRef *self_ref;
> >  
> > @@ -79,11 +82,12 @@ typedef struct V4L2m2mPriv {
> >   * Allocate a new context and references for a V4L2 M2M instance.
> >   *
> >   * @param[in] ctx The V4L2m2mPriv instantiated by the encoder/decoder.
> > + * @param[in] is_encoder Whether the context is for encoder/decoder
> >   * @param[out] ctx The V4L2m2mContext.
> >   *
> >   * @returns 0 in success, a negative error code otherwise.
> >   */
> > -int ff_v4l2_m2m_create_context(V4L2m2mPriv *priv, V4L2m2mContext **s);
> > +int ff_v4l2_m2m_create_context(V4L2m2mPriv *priv, int is_encoder, V4L2m2mContext **s);
> >  
> >  
> >  /**
> > diff --git a/libavcodec/v4l2_m2m_dec.c b/libavcodec/v4l2_m2m_dec.c
> > index d666edffe46..7c7ac2de8c5 100644
> > --- a/libavcodec/v4l2_m2m_dec.c
> > +++ b/libavcodec/v4l2_m2m_dec.c
> > @@ -181,7 +181,7 @@ static av_cold int v4l2_decode_init(AVCodecContext *avctx)
> >      V4L2m2mPriv *priv = avctx->priv_data;
> >      int ret;
> >  
> > -    ret = ff_v4l2_m2m_create_context(priv, &s);
> > +    ret = ff_v4l2_m2m_create_context(priv, !av_codec_is_decoder(avctx->codec), &s);

> 
> There's av_codec_is_encoder(). Also, You could pass avctx to
> ff_v4l2_m2m_create_context() instead of priv, and call
> av_codec_is_encoder() there, saving yourself the extra parameter.
> 
> Alternatively, just allocate the frame unconditionally and avoid all
> this extra code. ff_v4l2_m2m_create_context() is called once during
> init, so it's not an issue if the decoder allocates it as well.
> 

That's true, thanks.
I prefer your second option a bit more because it keeps same style in
v4l2_m2m.h.

I'll resend tomorrow in case there are more comments.

-- 
Andriy