[FFmpeg-devel] [PATCH 2/2] lavc/videotoolboxenc: Get the encoder supported properties

[mypopy at gmail.com]

On Tue, Sep 5, 2023 at 3:52 PM Tomas Härdin <git at haerdin.se> wrote:
>
> mån 2023-09-04 klockan 11:53 +0800 skrev Jun Zhao:
> > Get the encoder supported properties list, it will be used for
> > feature support checks.
> >
> > Signed-off-by: Jun Zhao <barryjzhao at tencent.com>
> > ---
> >  libavcodec/videotoolboxenc.c | 27 +++++++++++++++++++++++++++
> >  1 file changed, 27 insertions(+)
> >
> > diff --git a/libavcodec/videotoolboxenc.c
> > b/libavcodec/videotoolboxenc.c
> > index 5633640a30..8e70915225 100644
> > --- a/libavcodec/videotoolboxenc.c
> > +++ b/libavcodec/videotoolboxenc.c
> > @@ -232,6 +232,7 @@ typedef struct VTEncContext {
> >      AVClass *class;
> >      enum AVCodecID codec_id;
> >      VTCompressionSessionRef session;
> > +    CFDictionaryRef supported_props;
> >      CFStringRef ycbcr_matrix;
> >      CFStringRef color_primaries;
> >      CFStringRef transfer_function;
> > @@ -1110,6 +1111,18 @@ static int
> > vtenc_create_encoder(AVCodecContext   *avctx,
> >          return AVERROR_EXTERNAL;
> >      }
> >
> > +    status = VTCopySupportedPropertyDictionaryForEncoder(avctx-
> > >width,
> > +                                                         avctx-
> > >height,
> > +                                                         codec_type,
> > +                                                         enc_info,
> > +                                                         NULL,
> > +                                                         &vtctx-
> > >supported_props);
> > +
> > +    if (status != noErr) {
> > +        av_log(avctx, AV_LOG_ERROR,"Error retrieving the supported
> > property dictionary err=%"PRId64"", (int64_t)status);
>
> Missing \n
Good catch
>
> > +        return AVERROR_EXTERNAL;
> > +    }
> > +
> >      // Dump the init encoder
> >      {
> >          CFStringRef encoderID = NULL;
> > @@ -1656,6 +1669,7 @@ static av_cold int vtenc_init(AVCodecContext
> > *avctx)
> >      if (vtctx->profile == FF_PROFILE_UNKNOWN)
> >          vtctx->profile = avctx->profile;
> >      vtctx->session = NULL;
> > +    vtctx->supported_props = NULL;
>
> NULLing isn't necessary since priv_data is calloc'd
Yes，I know this, just following the old code behaviour，maybe need to
refine the old code
>
> >      status = vtenc_configure_encoder(avctx);
> >      if (status) return status;
> >
> > @@ -2426,6 +2440,11 @@ static int
> > create_cv_pixel_buffer(AVCodecContext   *avctx,
> >          if (vtstatus == kVTInvalidSessionErr) {
> >              CFRelease(vtctx->session);
> >              vtctx->session = NULL;
> > +            if (vtctx->supported_props) {
> > +                CFRelease(vtctx->supported_props);
> > +                vtctx->supported_props = NULL;
> > +            }
> > +
> >              status = vtenc_configure_encoder(avctx);
> >              if (status == 0)
> >                  pix_buf_pool =
> > VTCompressionSessionGetPixelBufferPool(vtctx->session);
> > @@ -2685,6 +2704,10 @@ pe_cleanup:
> >          CFRelease(vtctx->session);
> >
> >      vtctx->session = NULL;
> > +    if (vtctx->supported_props) {
> > +        CFRelease(vtctx->supported_props);
> > +        vtctx->supported_props = NULL;
> > +    }
> >      vtctx->frame_ct_out = 0;
> >
> >      av_assert0(status != 0 || (avctx->extradata && avctx-
> > >extradata_size > 0));
> > @@ -2709,6 +2732,10 @@ static av_cold int vtenc_close(AVCodecContext
> > *avctx)
> >      pthread_mutex_destroy(&vtctx->lock);
> >      CFRelease(vtctx->session);
> >      vtctx->session = NULL;
> > +    if (vtctx->supported_props) {
> > +        CFRelease(vtctx->supported_props);
> > +        vtctx->supported_props = NULL;
> > +    }
>
> Copy-pasting this in three places looks ugly. Moving cleanup of session
> and supported_props to a small function would reduce the number of
> lines and look prettier.
>
It makes more sense.