diff mbox series

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

Message ID 20230904035340.1819580-2-mypopydev@gmail.com
State New
Headers show
Series [FFmpeg-devel,1/2] lavc/videotoolboxenc: Dump the encoder | expand

Commit Message

Jun Zhao Sept. 4, 2023, 3:53 a.m. UTC 
Get the encoder supported properties list, it will be used for
feature support checks.

Signed-off-by: Jun Zhao <barryjzhao@tencent.com>
---
 libavcodec/videotoolboxenc.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

Comments

Tomas Härdin Sept. 5, 2023, 7:52 a.m. UTC | #1 
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@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

> +        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

>      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.

/Tomas
mypopy@gmail.com Sept. 5, 2023, 9:36 a.m. UTC | #2 
On Tue, Sep 5, 2023 at 3:52 PM Tomas Härdin <git@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@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.
Zhao Zhili Sept. 5, 2023, 9:49 a.m. UTC | #3 
> On Sep 5, 2023, at 17:36, mypopy@gmail.com wrote:
> 
> On Tue, Sep 5, 2023 at 3:52 PM Tomas Härdin <git@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@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

On one hand, calloc() isn’t the same as set these pointers to NULL,
since NULL isn’t always all-zero-bits.

On the other hand, NULL is assumed all-zero-bits everywhere.

Both works for me. I don’t want to enforce any style.

>> 
>>>     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.
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff mbox series

Patch

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);
+        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;
     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;
+    }
 
     if (vtctx->color_primaries) {
         CFRelease(vtctx->color_primaries);