diff mbox

[FFmpeg-devel,3/7] avformat/av1: Avoid allocation for small headers

Message ID 20191127122211.6352-3-andreas.rheinhardt@gmail.com
State Accepted
Commit a31f68fb449eaf6f030ce5633435663f154bb34d
Headers show

Commit Message

Andreas Rheinhardt Nov. 27, 2019, 12:22 p.m. UTC
By using avio_get_dyn_buf() + ffio_free_dyn_buf() instead of
avio_close_dyn_buf() + av_free() one can avoid an allocation + copy for
small headers. Furthermore, it simplifies freeing.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
 libavformat/av1.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

Comments

James Almer Nov. 27, 2019, 12:26 p.m. UTC | #1
On 11/27/2019 9:22 AM, Andreas Rheinhardt wrote:
> By using avio_get_dyn_buf() + ffio_free_dyn_buf() instead of
> avio_close_dyn_buf() + av_free() one can avoid an allocation + copy for
> small headers. Furthermore, it simplifies freeing.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---
>  libavformat/av1.c | 14 +++++---------
>  1 file changed, 5 insertions(+), 9 deletions(-)
> 
> diff --git a/libavformat/av1.c b/libavformat/av1.c
> index 03669dfd2a..132f4e987b 100644
> --- a/libavformat/av1.c
> +++ b/libavformat/av1.c
> @@ -326,7 +326,7 @@ int ff_isom_write_av1c(AVIOContext *pb, const uint8_t *buf, int size)
>      AV1SequenceParameters seq_params;
>      PutBitContext pbc;
>      uint8_t header[4];
> -    uint8_t *seq = NULL, *meta = NULL;
> +    uint8_t *seq, *meta;
>      int64_t obu_size;
>      int start_pos, type, temporal_id, spatial_id;
>      int ret, nb_seq = 0, seq_size, meta_size;
> @@ -376,7 +376,7 @@ int ff_isom_write_av1c(AVIOContext *pb, const uint8_t *buf, int size)
>          buf  += len;
>      }
>  
> -    seq_size  = avio_close_dyn_buf(seq_pb, &seq);
> +    seq_size  = avio_get_dyn_buf(seq_pb, &seq);
>      if (!seq_size) {
>          ret = AVERROR_INVALIDDATA;
>          goto fail;
> @@ -401,17 +401,13 @@ int ff_isom_write_av1c(AVIOContext *pb, const uint8_t *buf, int size)
>      avio_write(pb, header, sizeof(header));
>      avio_write(pb, seq, seq_size);
>  
> -    meta_size = avio_close_dyn_buf(meta_pb, &meta);
> +    meta_size = avio_get_dyn_buf(meta_pb, &meta);
>      if (meta_size)
>          avio_write(pb, meta, meta_size);
>  
>  fail:
> -    if (!seq)
> -        avio_close_dyn_buf(seq_pb, &seq);
> -    if (!meta)
> -        avio_close_dyn_buf(meta_pb, &meta);
> -    av_free(seq);
> -    av_free(meta);
> +    ffio_free_dyn_buf(&seq_pb);
> +    ffio_free_dyn_buf(&meta_pb);
>  
>      return ret;
>  }

Ah, I had this written and queued for when i pushed my avcC patch with
your suggested changes, but will apply yours instead since you actually
sent it first.
James Almer Nov. 27, 2019, 12:27 p.m. UTC | #2
On 11/27/2019 9:26 AM, James Almer wrote:
> On 11/27/2019 9:22 AM, Andreas Rheinhardt wrote:
>> By using avio_get_dyn_buf() + ffio_free_dyn_buf() instead of
>> avio_close_dyn_buf() + av_free() one can avoid an allocation + copy for
>> small headers. Furthermore, it simplifies freeing.
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
>> ---
>>  libavformat/av1.c | 14 +++++---------
>>  1 file changed, 5 insertions(+), 9 deletions(-)
>>
>> diff --git a/libavformat/av1.c b/libavformat/av1.c
>> index 03669dfd2a..132f4e987b 100644
>> --- a/libavformat/av1.c
>> +++ b/libavformat/av1.c
>> @@ -326,7 +326,7 @@ int ff_isom_write_av1c(AVIOContext *pb, const uint8_t *buf, int size)
>>      AV1SequenceParameters seq_params;
>>      PutBitContext pbc;
>>      uint8_t header[4];
>> -    uint8_t *seq = NULL, *meta = NULL;
>> +    uint8_t *seq, *meta;
>>      int64_t obu_size;
>>      int start_pos, type, temporal_id, spatial_id;
>>      int ret, nb_seq = 0, seq_size, meta_size;
>> @@ -376,7 +376,7 @@ int ff_isom_write_av1c(AVIOContext *pb, const uint8_t *buf, int size)
>>          buf  += len;
>>      }
>>  
>> -    seq_size  = avio_close_dyn_buf(seq_pb, &seq);
>> +    seq_size  = avio_get_dyn_buf(seq_pb, &seq);
>>      if (!seq_size) {
>>          ret = AVERROR_INVALIDDATA;
>>          goto fail;
>> @@ -401,17 +401,13 @@ int ff_isom_write_av1c(AVIOContext *pb, const uint8_t *buf, int size)
>>      avio_write(pb, header, sizeof(header));
>>      avio_write(pb, seq, seq_size);
>>  
>> -    meta_size = avio_close_dyn_buf(meta_pb, &meta);
>> +    meta_size = avio_get_dyn_buf(meta_pb, &meta);
>>      if (meta_size)
>>          avio_write(pb, meta, meta_size);
>>  
>>  fail:
>> -    if (!seq)
>> -        avio_close_dyn_buf(seq_pb, &seq);
>> -    if (!meta)
>> -        avio_close_dyn_buf(meta_pb, &meta);
>> -    av_free(seq);
>> -    av_free(meta);
>> +    ffio_free_dyn_buf(&seq_pb);
>> +    ffio_free_dyn_buf(&meta_pb);
>>  
>>      return ret;
>>  }
> 
> Ah, I had this written and queued for when i pushed my avcC patch with
> your suggested changes, but will apply yours instead since you actually
> sent it first.

And of course i reply to the wrong patch... Was talking about Patch 4/7.
diff mbox

Patch

diff --git a/libavformat/av1.c b/libavformat/av1.c
index 03669dfd2a..132f4e987b 100644
--- a/libavformat/av1.c
+++ b/libavformat/av1.c
@@ -326,7 +326,7 @@  int ff_isom_write_av1c(AVIOContext *pb, const uint8_t *buf, int size)
     AV1SequenceParameters seq_params;
     PutBitContext pbc;
     uint8_t header[4];
-    uint8_t *seq = NULL, *meta = NULL;
+    uint8_t *seq, *meta;
     int64_t obu_size;
     int start_pos, type, temporal_id, spatial_id;
     int ret, nb_seq = 0, seq_size, meta_size;
@@ -376,7 +376,7 @@  int ff_isom_write_av1c(AVIOContext *pb, const uint8_t *buf, int size)
         buf  += len;
     }
 
-    seq_size  = avio_close_dyn_buf(seq_pb, &seq);
+    seq_size  = avio_get_dyn_buf(seq_pb, &seq);
     if (!seq_size) {
         ret = AVERROR_INVALIDDATA;
         goto fail;
@@ -401,17 +401,13 @@  int ff_isom_write_av1c(AVIOContext *pb, const uint8_t *buf, int size)
     avio_write(pb, header, sizeof(header));
     avio_write(pb, seq, seq_size);
 
-    meta_size = avio_close_dyn_buf(meta_pb, &meta);
+    meta_size = avio_get_dyn_buf(meta_pb, &meta);
     if (meta_size)
         avio_write(pb, meta, meta_size);
 
 fail:
-    if (!seq)
-        avio_close_dyn_buf(seq_pb, &seq);
-    if (!meta)
-        avio_close_dyn_buf(meta_pb, &meta);
-    av_free(seq);
-    av_free(meta);
+    ffio_free_dyn_buf(&seq_pb);
+    ffio_free_dyn_buf(&meta_pb);
 
     return ret;
 }