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

["\"zhilizhao(赵志立)\"" <quinkblack@foxmail.com>]

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

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