On 02/08/17 06:53, Jun Zhao wrote: > V2: Change the slice/parameter buffers to dynamic alloc and split > the mutil-slice support for AVC/HEVC. > > From 39fd7852df0c96217310c525107da06a7ec0a062 Mon Sep 17 00:00:00 2001 > From: Jun Zhao <jun.z...@intel.com> > Date: Mon, 31 Jul 2017 22:46:23 -0400 > Subject: [PATCH V2 1/4] lavc/vaapi_encode: Change the slice/parameter buffers > to dynamic alloc. > > Change the slice/parameter buffers to be allocated dynamically. > > Signed-off-by: Wang, Yi A <yi.a.w...@intel.com> > Signed-off-by: Jun Zhao <jun.z...@intel.com> > --- > libavcodec/vaapi_encode.c | 27 ++++++++++++++++++++++++--- > libavcodec/vaapi_encode.h | 6 ++---- > 2 files changed, 26 insertions(+), 7 deletions(-) > > diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c > index 2de5f76cab..11d9803b5d 100644 > --- a/libavcodec/vaapi_encode.c > +++ b/libavcodec/vaapi_encode.c > @@ -36,13 +36,19 @@ static int vaapi_encode_make_packed_header(AVCodecContext > *avctx, > VAAPIEncodeContext *ctx = avctx->priv_data; > VAStatus vas; > VABufferID param_buffer, data_buffer; > + VABufferID *tmp; > VAEncPackedHeaderParameterBuffer params = { > .type = type, > .bit_length = bit_len, > .has_emulation_bytes = 1, > }; > > - av_assert0(pic->nb_param_buffers + 2 <= MAX_PARAM_BUFFERS); > + tmp = av_realloc_array(pic->param_buffers, sizeof(*tmp), > (pic->nb_param_buffers + 2));
Redundant parentheses? > + if (!tmp) { > + av_freep(&pic->param_buffers); This leaks the actual buffers which were allocated (pic->param_buffers[0..pic->nb_param_buffers-1]). > + return AVERROR(ENOMEM); > + } > + pic->param_buffers = tmp; > > vas = vaCreateBuffer(ctx->hwctx->display, ctx->va_context, > VAEncPackedHeaderParameterBufferType, > @@ -77,9 +83,15 @@ static int vaapi_encode_make_param_buffer(AVCodecContext > *avctx, > { > VAAPIEncodeContext *ctx = avctx->priv_data; > VAStatus vas; > + VABufferID *tmp; > VABufferID buffer; > > - av_assert0(pic->nb_param_buffers + 1 <= MAX_PARAM_BUFFERS); > + tmp = av_realloc_array(pic->param_buffers, sizeof(*tmp), > (pic->nb_param_buffers + 1)); > + if (!tmp) { > + av_freep(&pic->param_buffers); Same here. > + return AVERROR(ENOMEM); > + } > + pic->param_buffers = tmp; > > vas = vaCreateBuffer(ctx->hwctx->display, ctx->va_context, > type, len, 1, data, &buffer); > @@ -122,6 +134,8 @@ static int vaapi_encode_wait(AVCodecContext *avctx, > // Input is definitely finished with now. > av_frame_free(&pic->input_image); > > + av_freep(&pic->param_buffers); Maybe it would be cleaner to free param_buffers at the end of vaapi_encode_issue() - the buffers themselves are no longer usable at that point. > + > pic->encode_complete = 1; > return 0; > } > @@ -313,7 +327,10 @@ static int vaapi_encode_issue(AVCodecContext *avctx, > } > } > > - av_assert0(pic->nb_slices <= MAX_PICTURE_SLICES); > + pic->slices = (VAAPIEncodeSlice **)av_malloc(sizeof(VAAPIEncodeSlice *) > * pic->nb_slices); > + if (pic->slices == NULL) > + goto fail; > + > for (i = 0; i < pic->nb_slices; i++) { > slice = av_mallocz(sizeof(*slice)); How about just making pic->slices be VAAPIEncodeSlice*, pointing to an array of the structures? This code is now allocating both an array of pointers and then each element of that array individually. > if (!slice) { > @@ -427,6 +444,8 @@ fail: > vaDestroyBuffer(ctx->hwctx->display, pic->param_buffers[i]); > fail_at_end: > av_freep(&pic->codec_picture_params); > + av_freep(&pic->param_buffers); > + av_freep(&pic->slices); > av_frame_free(&pic->recon_image); > av_buffer_unref(&pic->output_buffer_ref); > pic->output_buffer = VA_INVALID_ID; > @@ -544,6 +563,8 @@ static int vaapi_encode_free(AVCodecContext *avctx, > av_frame_free(&pic->input_image); > av_frame_free(&pic->recon_image); > > + av_freep(&pic->param_buffers); > + av_freep(&pic->slices); > // Output buffer should already be destroyed. > av_assert0(pic->output_buffer == VA_INVALID_ID); > > diff --git a/libavcodec/vaapi_encode.h b/libavcodec/vaapi_encode.h > index 0edf27e4cb..b542772aed 100644 > --- a/libavcodec/vaapi_encode.h > +++ b/libavcodec/vaapi_encode.h > @@ -35,8 +35,6 @@ enum { > MAX_CONFIG_ATTRIBUTES = 4, > MAX_GLOBAL_PARAMS = 4, > MAX_PICTURE_REFERENCES = 2, > - MAX_PICTURE_SLICES = 112, > - MAX_PARAM_BUFFERS = 128, > MAX_REORDER_DELAY = 16, > MAX_PARAM_BUFFER_SIZE = 1024, > }; > @@ -73,7 +71,7 @@ typedef struct VAAPIEncodePicture { > VASurfaceID recon_surface; > > int nb_param_buffers; > - VABufferID param_buffers[MAX_PARAM_BUFFERS]; > + VABufferID *param_buffers; > > AVBufferRef *output_buffer_ref; > VABufferID output_buffer; > @@ -85,7 +83,7 @@ typedef struct VAAPIEncodePicture { > struct VAAPIEncodePicture *refs[MAX_PICTURE_REFERENCES]; > > int nb_slices; > - VAAPIEncodeSlice *slices[MAX_PICTURE_SLICES]; > + VAAPIEncodeSlice **slices; > } VAAPIEncodePicture; > > typedef struct VAAPIEncodeContext { > -- > 2.11.0 > Actual aim of patch looks fine to me. Thanks, - Mark _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel