diff mbox series

[FFmpeg-devel,11/21] avcodec/smacker: Don't zero-initialize unnecessarily

Message ID 20200801134704.3647-3-andreas.rheinhardt@gmail.com
State New
Headers show
Series [FFmpeg-devel,1/8] avcodec/smacker: Remove write-only and unused variables
Related show

Checks

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

Commit Message

Andreas Rheinhardt Aug. 1, 2020, 1:46 p.m. UTC
With the possible exception of the "last" values when decoding video,
only the part that is actually initialized with values derived from the
bitstream is used afterwards, so it is unnecessary to zero everything at
the beginning. This is also no problem for the "last" values at all,
because they are reset for every frame anyway.

While at it, use sizeof(variable) instead of sizeof(type).

Performance increased slightly: For GCC, from 2068389 decicycles per call
to smka_decode_frame() when decoding the sample from ticket #2425 to 2053758
decicycles; for Clang, from 1534188 to 1523153 decicycles.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
 libavcodec/smacker.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

Comments

Paul B Mahol Aug. 1, 2020, 2:03 p.m. UTC | #1
I prefer in this case av_calloc or av_malloc_array, if memset hurts.

Anyway LGTM

On 8/1/20, Andreas Rheinhardt <andreas.rheinhardt@gmail.com> wrote:
> With the possible exception of the "last" values when decoding video,
> only the part that is actually initialized with values derived from the
> bitstream is used afterwards, so it is unnecessary to zero everything at
> the beginning. This is also no problem for the "last" values at all,
> because they are reset for every frame anyway.
>
> While at it, use sizeof(variable) instead of sizeof(type).
>
> Performance increased slightly: For GCC, from 2068389 decicycles per call
> to smka_decode_frame() when decoding the sample from ticket #2425 to 2053758
> decicycles; for Clang, from 1534188 to 1523153 decicycles.
>
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---
>  libavcodec/smacker.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/libavcodec/smacker.c b/libavcodec/smacker.c
> index 27de30f0e1..c249ce7514 100644
> --- a/libavcodec/smacker.c
> +++ b/libavcodec/smacker.c
> @@ -197,14 +197,15 @@ static int smacker_decode_header_tree(SmackVContext
> *smk, GetBitContext *gb, int
>      for (int i = 0; i < 2; i++) {
>          h[i].length  = 256;
>          h[i].current = 0;
> -        h[i].bits    = av_mallocz(256 * sizeof(h[i].bits[0]));
> -        h[i].lengths = av_mallocz(256 * sizeof(h[i].lengths[0]));
> -        h[i].values  = av_mallocz(256 * sizeof(h[i].values[0]));
> +        h[i].bits    = av_malloc(256 * sizeof(h[i].bits[0]));
> +        h[i].lengths = av_malloc(256 * sizeof(h[i].lengths[0]));
> +        h[i].values  = av_malloc(256 * sizeof(h[i].values[0]));
>          if (!h[i].bits || !h[i].lengths || !h[i].values) {
>              err = AVERROR(ENOMEM);
>              goto error;
>          }
>          if (!get_bits1(gb)) {
> +            h[i].values[0] = 0;
>              av_log(smk->avctx, AV_LOG_ERROR, "Skipping %s bytes tree\n",
>                     i ? "high" : "low");
>              continue;
> @@ -242,7 +243,7 @@ static int smacker_decode_header_tree(SmackVContext
> *smk, GetBitContext *gb, int
>
>      huff.length = (size + 3) >> 2;
>      huff.current = 0;
> -    huff.values = av_mallocz_array(huff.length + 3,
> sizeof(huff.values[0]));
> +    huff.values = av_malloc_array(huff.length + 3, sizeof(huff.values[0]));
>      if (!huff.values) {
>          err = AVERROR(ENOMEM);
>          goto error;
> @@ -645,9 +646,9 @@ static int smka_decode_frame(AVCodecContext *avctx, void
> *data,
>      for(i = 0; i < (1 << (bits + stereo)); i++) {
>          h[i].length = 256;
>          h[i].current = 0;
> -        h[i].bits = av_mallocz(256 * 4);
> -        h[i].lengths = av_mallocz(256 * sizeof(int));
> -        h[i].values = av_mallocz(256 * sizeof(int));
> +        h[i].bits    = av_malloc(256 * sizeof(h[i].bits));
> +        h[i].lengths = av_malloc(256 * sizeof(h[i].lengths));
> +        h[i].values  = av_malloc(256 * sizeof(h[i].values));
>          if (!h[i].bits || !h[i].lengths || !h[i].values) {
>              ret = AVERROR(ENOMEM);
>              goto error;
> --
> 2.20.1
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff mbox series

Patch

diff --git a/libavcodec/smacker.c b/libavcodec/smacker.c
index 27de30f0e1..c249ce7514 100644
--- a/libavcodec/smacker.c
+++ b/libavcodec/smacker.c
@@ -197,14 +197,15 @@  static int smacker_decode_header_tree(SmackVContext *smk, GetBitContext *gb, int
     for (int i = 0; i < 2; i++) {
         h[i].length  = 256;
         h[i].current = 0;
-        h[i].bits    = av_mallocz(256 * sizeof(h[i].bits[0]));
-        h[i].lengths = av_mallocz(256 * sizeof(h[i].lengths[0]));
-        h[i].values  = av_mallocz(256 * sizeof(h[i].values[0]));
+        h[i].bits    = av_malloc(256 * sizeof(h[i].bits[0]));
+        h[i].lengths = av_malloc(256 * sizeof(h[i].lengths[0]));
+        h[i].values  = av_malloc(256 * sizeof(h[i].values[0]));
         if (!h[i].bits || !h[i].lengths || !h[i].values) {
             err = AVERROR(ENOMEM);
             goto error;
         }
         if (!get_bits1(gb)) {
+            h[i].values[0] = 0;
             av_log(smk->avctx, AV_LOG_ERROR, "Skipping %s bytes tree\n",
                    i ? "high" : "low");
             continue;
@@ -242,7 +243,7 @@  static int smacker_decode_header_tree(SmackVContext *smk, GetBitContext *gb, int
 
     huff.length = (size + 3) >> 2;
     huff.current = 0;
-    huff.values = av_mallocz_array(huff.length + 3, sizeof(huff.values[0]));
+    huff.values = av_malloc_array(huff.length + 3, sizeof(huff.values[0]));
     if (!huff.values) {
         err = AVERROR(ENOMEM);
         goto error;
@@ -645,9 +646,9 @@  static int smka_decode_frame(AVCodecContext *avctx, void *data,
     for(i = 0; i < (1 << (bits + stereo)); i++) {
         h[i].length = 256;
         h[i].current = 0;
-        h[i].bits = av_mallocz(256 * 4);
-        h[i].lengths = av_mallocz(256 * sizeof(int));
-        h[i].values = av_mallocz(256 * sizeof(int));
+        h[i].bits    = av_malloc(256 * sizeof(h[i].bits));
+        h[i].lengths = av_malloc(256 * sizeof(h[i].lengths));
+        h[i].values  = av_malloc(256 * sizeof(h[i].values));
         if (!h[i].bits || !h[i].lengths || !h[i].values) {
             ret = AVERROR(ENOMEM);
             goto error;