[FFmpeg-devel] [PATCH 2/3] libavcodec/cbs: Don't zero twice

[Mark Thompson]

On 10/02/2019 23:11, Andreas Rheinhardt wrote:
> Mark Thompson:
>> On 05/02/2019 20:08, Andreas Rheinhardt wrote:
>>>  int ff_cbs_read_extradata(CodedBitstreamContext *ctx,
>>>                            CodedBitstreamFragment *frag,
>>> -                          const AVCodecParameters *par)
>>> +                          const AVCodecParameters *par,
>>> +                          int reuse)
>>>  {
>>>      int err;
>>>  
>>> -    memset(frag, 0, sizeof(*frag));
>>> +    if (!reuse)
>>> +        memset(frag, 0, sizeof(*frag));
>>>  
>>>      err = cbs_fill_fragment_data(ctx, frag, par->extradata,
>>>                                   par->extradata_size);
>>> @@ -237,11 +239,12 @@ int ff_cbs_read_extradata(CodedBitstreamContext *ctx,
>>>  
>>>  int ff_cbs_read_packet(CodedBitstreamContext *ctx,
>>>                         CodedBitstreamFragment *frag,
>>> -                       const AVPacket *pkt)
>>> +                       const AVPacket *pkt, int reuse)
>>>  {
>>>      int err;
>>>  
>>> -    memset(frag, 0, sizeof(*frag));
>>> +    if (!reuse)
>>> +        memset(frag, 0, sizeof(*frag));
>>>  
>>>      if (pkt->buf) {
>>>          frag->data_ref = av_buffer_ref(pkt->buf);
>>> @@ -266,11 +269,13 @@ int ff_cbs_read_packet(CodedBitstreamContext *ctx,
>>>  
>>>  int ff_cbs_read(CodedBitstreamContext *ctx,
>>>                  CodedBitstreamFragment *frag,
>>> -                const uint8_t *data, size_t size)
>>> +                const uint8_t *data, size_t size,
>>> +                int reuse)
>>>  {
>>>      int err;
>>>  
>>> -    memset(frag, 0, sizeof(*frag));
>>> +    if (!reuse)
>>> +        memset(frag, 0, sizeof(*frag));
>>>  
>>>      err = cbs_fill_fragment_data(ctx, frag, data, size);
>>>      if (err < 0)
>>
>> I don't think this patch should be needed.  Can we just document that your fragment must either be zeroed (so, allocated by av_*allocz() or memset() to zero) or you've called the ff_cbs_fragment_reset() (whatever the name is) function before any read call?  It can even check that the user hasn't messed up by asserting that data, data_size and nb_units are all zero.
> 
> I agree with your suggestion regarding the documentation; but it is
> nevertheless needed to eliminate the memset in the ff_cbs_read
> functions. Otherwise the unit array is leaked.

Yes, sorry.  Indeed still remove the memset, but don't change any of the parameters or callers.

Thanks,

- Mark