[FFmpeg-devel] [PATCH v15 1/2] lavc/svt_hevc: add libsvt hevc encoder wrapper

[Sun, Jing A]

-----Original Message-----
From: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] On Behalf Of Limin Wang
Sent: Wednesday, July 31, 2019 7:05 AM
To: FFmpeg development discussions and patches <ffmpeg-devel at ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH v15 1/2] lavc/svt_hevc: add libsvt hevc encoder wrapper

On Tue, Jul 30, 2019 at 07:48:43PM +0200, Carl Eugen Hoyos wrote:
> Am Di., 30. Juli 2019 um 15:20 Uhr schrieb James Almer <jamrial at gmail.com>:
> >
> > On 7/30/2019 2:59 AM, Limin Wang wrote:
> > >> +    if (avctx->flags & AV_CODEC_FLAG_GLOBAL_HEADER) {
> > >> +        EB_BUFFERHEADERTYPE *header_ptr = NULL;
> > >> +
> > >> +        svt_ret = EbH265EncStreamHeader(svt_enc->svt_handle, &header_ptr);
> > >> +        if (svt_ret != EB_ErrorNone) {
> > >> +            av_log(avctx, AV_LOG_ERROR, "Failed to build stream header\n");
> > >> +            goto failed_init_encoder;
> > >> +        }
> > >> +
> > >> +        avctx->extradata_size = header_ptr->nFilledLen;
> > >> +        avctx->extradata = av_malloc(avctx->extradata_size + 
> > >> + AV_INPUT_BUFFER_PADDING_SIZE);
> > > It's preferalbe to use av_mallocz
> >
> > He was asked to do it this way as it's faster. No need to zero the 
> > whole buffer if it's going to be written to immediately afterwards. 
> > Only the trailing padding bytes needs to be zeroed.
> 
> In this case I suspect there is an unneeded cast in the next line.

> It's very confusing to allocate with extra padding size, then memcpy the actual data size, then zero the padding data.

> IMO, the padding size is used for the bitstream optimized buffer read(32 or 64 bit for minimal at least). If it's memcpy, do we need malloc with extra AV_INPUT_BUFFER_PADDING_SIZE?

Thanks for everyone's advices and I am considering on how to make this not confusing. Will update the patch later.
BTW, it's a "she" :-).

Regards,
Sun, Jing