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

[Andreas Rheinhardt]

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.

Andreas