Re: [FFmpeg-devel] [PATCH] vulkan_hevc: switch from a buffer pool to a simple malloc

[Andreas Rheinhardt <andreas.rheinhardt@outlook.com>]

Lynne:
> -static int get_data_set_buf(FFVulkanDecodeContext *s, AVBufferRef **data_buf,
> -                            int nb_vps, AVBufferRef * const vps_list[HEVC_MAX_VPS_COUNT])
> +static int alloc_hevc_header_structs(FFVulkanDecodeContext *s,
> +                                     int nb_vps,
> +                                     AVBufferRef * const vps_list[HEVC_MAX_VPS_COUNT])
>  {
> -    uint8_t *data_ptr;
> +    uintptr_t data_ptr;
>      HEVCHeaderSet *hdr;
>  
>      size_t base_size = sizeof(StdVideoH265SequenceParameterSet)*HEVC_MAX_SPS_COUNT +
> @@ -93,22 +94,18 @@ static int get_data_set_buf(FFVulkanDecodeContext *s, AVBufferRef **data_buf,
>          buf_size += sizeof(HEVCHeaderVPSSet)*vps->vps_num_hrd_parameters;
>      }
>  
> -    if (buf_size > s->tmp_pool_ele_size) {
> -        av_buffer_pool_uninit(&s->tmp_pool);
> -        s->tmp_pool_ele_size = 0;
> -        s->tmp_pool = av_buffer_pool_init(buf_size, NULL);
> -        if (!s->tmp_pool)
> +    if (buf_size > s->hevc_headers_size) {
> +        av_freep(&s->hevc_headers);
> +        s->hevc_headers_size = 0;
> +        s->hevc_headers = av_mallocz(buf_size);

The earlier code did not zero-initialize the buffer. I thought the
set_[vsp]ps functions were enough to initialize them.

> +        if (!s->hevc_headers)
>              return AVERROR(ENOMEM);
> -        s->tmp_pool_ele_size = buf_size;
> +        s->hevc_headers_size = buf_size;
>      }
>  
> -    *data_buf = av_buffer_pool_get(s->tmp_pool);
> -    if (!(*data_buf))
> -        return AVERROR(ENOMEM);
> -
>      /* Setup pointers */
> -    data_ptr = (*data_buf)->data;
> -    hdr = (HEVCHeaderSet *)data_ptr;
> +    hdr = s->hevc_headers;
> +    data_ptr = (uintptr_t)hdr;
>      hdr->hvps = (HEVCHeaderVPS *)(data_ptr + base_size);

The standard way to get pointer that is at a certain byte offset from a
given pointer is by casting said pointer to char* and applying the
offset; casting to uintptr_t and back will work, but is actually not
guaranteed. In fact, the whole data_ptr variable seems unnecessary.
You may use FF_FIELD_AT from lavu/internal.h if you think that it is
helpful for this. (I don't think so.)

>      data_ptr += base_size + vps_size*nb_vps;
>      for (int i = 0; i < nb_vps; i++) {

LGTM apart from these nits.

- Andreas

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