[FFmpeg-devel] [PATCH 4/6] avcodec/trace_headers: move the reference in the bsf internal buffer

[James Almer]

On 3/11/2018 3:32 PM, Mark Thompson wrote:
> On 11/03/18 17:58, James Almer wrote:
>> There's no need to allocate a new packet for it.
>>
>> Signed-off-by: James Almer <jamrial at gmail.com>
>> ---
>>  libavcodec/trace_headers_bsf.c | 30 ++++++++++++++----------------
>>  1 file changed, 14 insertions(+), 16 deletions(-)
>>
>> diff --git a/libavcodec/trace_headers_bsf.c b/libavcodec/trace_headers_bsf.c
>> index 93d04cb509..d59bc828a9 100644
>> --- a/libavcodec/trace_headers_bsf.c
>> +++ b/libavcodec/trace_headers_bsf.c
>> @@ -71,41 +71,39 @@ static int trace_headers(AVBSFContext *bsf, AVPacket *out)
>>  {
>>      TraceHeadersContext *ctx = bsf->priv_data;
>>      CodedBitstreamFragment au;
>> -    AVPacket *in;
>>      char tmp[256] = { 0 };
>>      int err;
>>  
>> -    err = ff_bsf_get_packet(bsf, &in);
>> +    err = ff_bsf_get_packet_ref(bsf, out);
>>      if (err < 0)
>>          return err;
>>  
>> -    if (in->flags & AV_PKT_FLAG_KEY)
>> +    if (out->flags & AV_PKT_FLAG_KEY)
>>          av_strlcat(tmp, ", key frame", sizeof(tmp));
>> -    if (in->flags & AV_PKT_FLAG_CORRUPT)
>> +    if (out->flags & AV_PKT_FLAG_CORRUPT)
>>          av_strlcat(tmp, ", corrupt", sizeof(tmp));
>>  
>> -    if (in->pts != AV_NOPTS_VALUE)
>> -        av_strlcatf(tmp, sizeof(tmp), ", pts %"PRId64, in->pts);
>> +    if (out->pts != AV_NOPTS_VALUE)
>> +        av_strlcatf(tmp, sizeof(tmp), ", pts %"PRId64, out->pts);
>>      else
>>          av_strlcat(tmp, ", no pts", sizeof(tmp));
>> -    if (in->dts != AV_NOPTS_VALUE)
>> -        av_strlcatf(tmp, sizeof(tmp), ", dts %"PRId64, in->dts);
>> +    if (out->dts != AV_NOPTS_VALUE)
>> +        av_strlcatf(tmp, sizeof(tmp), ", dts %"PRId64, out->dts);
>>      else
>>          av_strlcat(tmp, ", no dts", sizeof(tmp));
>> -    if (in->duration > 0)
>> -        av_strlcatf(tmp, sizeof(tmp), ", duration %"PRId64, in->duration);
>> +    if (out->duration > 0)
>> +        av_strlcatf(tmp, sizeof(tmp), ", duration %"PRId64, out->duration);
>>  
>> -    av_log(bsf, AV_LOG_INFO, "Packet: %d bytes%s.\n", in->size, tmp);
>> +    av_log(bsf, AV_LOG_INFO, "Packet: %d bytes%s.\n", out->size, tmp);
>>  
>> -    err = ff_cbs_read_packet(ctx->cbc, &au, in);
>> -    if (err < 0)
>> +    err = ff_cbs_read_packet(ctx->cbc, &au, out);
>> +    if (err < 0) {
>> +        av_packet_unref(out);
>>          return err;
>> +    }
>>  
>>      ff_cbs_fragment_uninit(ctx->cbc, &au);
>>  
>> -    av_packet_move_ref(out, in);
>> -    av_packet_free(&in);
>> -
>>      return 0;
>>  }
>>  
>>
> 
> Rename the packet to something like "pkt" throughout?  The "out" name looks kindof weird after this change.
> 
> (That could probably apply to all of the patches, but this one makes the most use of the packet.)
> 
> Whole series looks fine to me.

Changed in all patches and pushed. Thanks!

> 
> - Mark