diff mbox series

[FFmpeg-devel,03/10] avformat/argo_brp: remove an allocation

Message ID 20200920080528.26200-4-zane@zanevaniperen.com
State Accepted
Headers show
Series argo_brp cleanups and fixes
Related show

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Zane van Iperen Sept. 20, 2020, 8:06 a.m. UTC
Signed-off-by: Zane van Iperen <zane@zanevaniperen.com>
---
 libavformat/argo_brp.c | 20 ++------------------
 1 file changed, 2 insertions(+), 18 deletions(-)

Comments

Andreas Rheinhardt Sept. 20, 2020, 8:13 a.m. UTC | #1
Zane van Iperen:
> Signed-off-by: Zane van Iperen <zane@zanevaniperen.com>
> ---
>  libavformat/argo_brp.c | 20 ++------------------
>  1 file changed, 2 insertions(+), 18 deletions(-)
> 
> diff --git a/libavformat/argo_brp.c b/libavformat/argo_brp.c
> index 27029d07b1..e57c20eb96 100644
> --- a/libavformat/argo_brp.c
> +++ b/libavformat/argo_brp.c
> @@ -77,7 +77,7 @@ typedef struct ArgoBRPStreamHeader {
>  
>  typedef struct ArgoBRPDemuxContext {
>      ArgoBRPFileHeader   fhdr;
> -    ArgoBRPStreamHeader *streams;
> +    ArgoBRPStreamHeader streams[BRP_MAX_STREAMS];
>      /* To know how much of a BASF to give. */
>      int64_t             lastpts;
>      int                 hit_eof;
> @@ -101,16 +101,6 @@ static int argo_brp_probe(const AVProbeData *p)
>      return AVPROBE_SCORE_EXTENSION + 1;
>  }
>  
> -static int argo_brp_read_close(AVFormatContext *s)
> -{
> -    ArgoBRPDemuxContext *brp = s->priv_data;
> -
> -    if (brp->streams != NULL)
> -        av_freep(&brp->streams);
> -
> -    return 0;
> -}
> -
>  static int read_extradata(AVFormatContext *s, const ArgoBRPStreamHeader *hdr,
>                            void *buf, size_t bufsz)
>  {
> @@ -174,9 +164,6 @@ static int argo_brp_read_header(AVFormatContext *s)
>          return AVERROR_PATCHWELCOME;
>      }
>  
> -    if ((brp->streams = av_mallocz_array(brp->fhdr.num_streams, sizeof(ArgoBRPStreamHeader))) == NULL)
> -        return AVERROR(ENOMEM);
> -
>      /* Build the stream info. */
>      brp->basf.index = -1;
>      for (uint32_t i = 0; i < brp->fhdr.num_streams; i++) {
> @@ -331,8 +318,6 @@ static int argo_brp_read_header(AVFormatContext *s)
>      return 0;
>  
>  fail:
> -    /* TODO: Remove once AVFMT_HEADER_CLEANUP lands. */
> -    argo_brp_read_close(s);
>      return ret;
>  }
>  
> @@ -444,6 +429,5 @@ AVInputFormat ff_argo_brp_demuxer = {
>      .priv_data_size = sizeof(ArgoBRPDemuxContext),
>      .read_probe     = argo_brp_probe,
>      .read_header    = argo_brp_read_header,
> -    .read_packet    = argo_brp_read_packet,
> -    .read_close     = argo_brp_read_close
> +    .read_packet    = argo_brp_read_packet

Unless you absolutely know that no entry will ever be added afterwards
(e.g. if you have a sentinel), you should add a trailing comma as this
means that if you add/remove a line later, you will leave the other
lines as they are. If not, the diff will be unnecessarily bigger and
more complicated and the usefulness of git blame will suffer (if you
applied the above, git blame would show that this commit added the
read_pack line and one would have to go further into the history to see
the commit that really did it).
So just remove the offending read_close, but leave the other stuff
untouched.

>  };
>
Zane van Iperen Sept. 20, 2020, 9:03 a.m. UTC | #2
On Sun, 20 Sep 2020 10:13:48 +0200
"Andreas Rheinhardt" <andreas.rheinhardt@gmail.com> wrote:

> > @@ -444,6 +429,5 @@ AVInputFormat ff_argo_brp_demuxer = {
> >      .priv_data_size = sizeof(ArgoBRPDemuxContext),
> >      .read_probe     = argo_brp_probe,
> >      .read_header    = argo_brp_read_header,
> > -    .read_packet    = argo_brp_read_packet,
> > -    .read_close     = argo_brp_read_close
> > +    .read_packet    = argo_brp_read_packet
> 
> Unless you absolutely know that no entry will ever be added afterwards
> (e.g. if you have a sentinel), you should add a trailing comma as this
> means that if you add/remove a line later, you will leave the other
> lines as they are. If not, the diff will be unnecessarily bigger and
> more complicated and the usefulness of git blame will suffer (if you
> applied the above, git blame would show that this commit added the
> read_pack line and one would have to go further into the history to see
> the commit that really did it).
> So just remove the offending read_close, but leave the other stuff
> untouched.

Whoops, never meant to remove it. Have fixed locally.
diff mbox series

Patch

diff --git a/libavformat/argo_brp.c b/libavformat/argo_brp.c
index 27029d07b1..e57c20eb96 100644
--- a/libavformat/argo_brp.c
+++ b/libavformat/argo_brp.c
@@ -77,7 +77,7 @@  typedef struct ArgoBRPStreamHeader {
 
 typedef struct ArgoBRPDemuxContext {
     ArgoBRPFileHeader   fhdr;
-    ArgoBRPStreamHeader *streams;
+    ArgoBRPStreamHeader streams[BRP_MAX_STREAMS];
     /* To know how much of a BASF to give. */
     int64_t             lastpts;
     int                 hit_eof;
@@ -101,16 +101,6 @@  static int argo_brp_probe(const AVProbeData *p)
     return AVPROBE_SCORE_EXTENSION + 1;
 }
 
-static int argo_brp_read_close(AVFormatContext *s)
-{
-    ArgoBRPDemuxContext *brp = s->priv_data;
-
-    if (brp->streams != NULL)
-        av_freep(&brp->streams);
-
-    return 0;
-}
-
 static int read_extradata(AVFormatContext *s, const ArgoBRPStreamHeader *hdr,
                           void *buf, size_t bufsz)
 {
@@ -174,9 +164,6 @@  static int argo_brp_read_header(AVFormatContext *s)
         return AVERROR_PATCHWELCOME;
     }
 
-    if ((brp->streams = av_mallocz_array(brp->fhdr.num_streams, sizeof(ArgoBRPStreamHeader))) == NULL)
-        return AVERROR(ENOMEM);
-
     /* Build the stream info. */
     brp->basf.index = -1;
     for (uint32_t i = 0; i < brp->fhdr.num_streams; i++) {
@@ -331,8 +318,6 @@  static int argo_brp_read_header(AVFormatContext *s)
     return 0;
 
 fail:
-    /* TODO: Remove once AVFMT_HEADER_CLEANUP lands. */
-    argo_brp_read_close(s);
     return ret;
 }
 
@@ -444,6 +429,5 @@  AVInputFormat ff_argo_brp_demuxer = {
     .priv_data_size = sizeof(ArgoBRPDemuxContext),
     .read_probe     = argo_brp_probe,
     .read_header    = argo_brp_read_header,
-    .read_packet    = argo_brp_read_packet,
-    .read_close     = argo_brp_read_close
+    .read_packet    = argo_brp_read_packet
 };