[FFmpeg-devel] avcodec/zmbv: Simplify assigning decode_intra function pointer

Submitted by Andreas Rheinhardt on Oct. 7, 2019, 12:57 a.m.

Details

Message ID 20191007005725.13605-1-andreas.rheinhardt@gmail.com
State New
Headers show

Commit Message

Andreas Rheinhardt Oct. 7, 2019, 12:57 a.m.
zmbv has only one function for decoding intra frames, namely
decode_intra. This can be used to simplify the process of choosing the
right function pointer.

This also removes spec-incompliant conversions between function pointers
and pointers of type void * and thereby fixes the warning "ISO C forbids
assignment between function pointer and ‘void *’" that GCC emits with
the -pedantic option.

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

Comments

Paul B Mahol Oct. 7, 2019, 8:35 a.m.
lgtm

On 10/7/19, Andreas Rheinhardt <andreas.rheinhardt@gmail.com> wrote:
> zmbv has only one function for decoding intra frames, namely
> decode_intra. This can be used to simplify the process of choosing the
> right function pointer.
>
> This also removes spec-incompliant conversions between function pointers
> and pointers of type void * and thereby fixes the warning "ISO C forbids
> assignment between function pointer and ‘void *’" that GCC emits with
> the -pedantic option.
>
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---
>  libavcodec/zmbv.c | 8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)
>
> diff --git a/libavcodec/zmbv.c b/libavcodec/zmbv.c
> index 99e735cfd9..8e9e13c936 100644
> --- a/libavcodec/zmbv.c
> +++ b/libavcodec/zmbv.c
> @@ -425,7 +425,6 @@ static int decode_frame(AVCodecContext *avctx, void
> *data, int *got_frame, AVPac
>      c->flags = buf[0];
>      buf++; len--;
>      if (c->flags & ZMBV_KEYFRAME) {
> -        void *decode_intra = NULL;
>          c->decode_intra= NULL;
>
>          if (len < 6)
> @@ -436,7 +435,6 @@ static int decode_frame(AVCodecContext *avctx, void
> *data, int *got_frame, AVPac
>          c->fmt = buf[3];
>          c->bw = buf[4];
>          c->bh = buf[5];
> -        c->decode_intra = NULL;
>          c->decode_xor = NULL;
>
>          buf += 6;
> @@ -460,7 +458,6 @@ static int decode_frame(AVCodecContext *avctx, void
> *data, int *got_frame, AVPac
>          switch (c->fmt) {
>          case ZMBV_FMT_8BPP:
>              c->bpp = 8;
> -            decode_intra = zmbv_decode_intra;
>              c->decode_xor = zmbv_decode_xor_8;
>              avctx->pix_fmt = AV_PIX_FMT_PAL8;
>              c->stride = c->width;
> @@ -468,7 +465,6 @@ static int decode_frame(AVCodecContext *avctx, void
> *data, int *got_frame, AVPac
>          case ZMBV_FMT_15BPP:
>          case ZMBV_FMT_16BPP:
>              c->bpp = 16;
> -            decode_intra = zmbv_decode_intra;
>              c->decode_xor = zmbv_decode_xor_16;
>              if (c->fmt == ZMBV_FMT_15BPP)
>                  avctx->pix_fmt = AV_PIX_FMT_RGB555LE;
> @@ -479,7 +475,6 @@ static int decode_frame(AVCodecContext *avctx, void
> *data, int *got_frame, AVPac
>  #ifdef ZMBV_ENABLE_24BPP
>          case ZMBV_FMT_24BPP:
>              c->bpp = 24;
> -            decode_intra = zmbv_decode_intra;
>              c->decode_xor = zmbv_decode_xor_24;
>              avctx->pix_fmt = AV_PIX_FMT_BGR24;
>              c->stride = c->width * 3;
> @@ -487,7 +482,6 @@ static int decode_frame(AVCodecContext *avctx, void
> *data, int *got_frame, AVPac
>  #endif //ZMBV_ENABLE_24BPP
>          case ZMBV_FMT_32BPP:
>              c->bpp = 32;
> -            decode_intra = zmbv_decode_intra;
>              c->decode_xor = zmbv_decode_xor_32;
>              avctx->pix_fmt = AV_PIX_FMT_BGR0;
>              c->stride = c->width * 4;
> @@ -517,7 +511,7 @@ static int decode_frame(AVCodecContext *avctx, void
> *data, int *got_frame, AVPac
>          }
>          memset(c->cur, 0, avctx->width * avctx->height * (c->bpp / 8));
>          memset(c->prev, 0, avctx->width * avctx->height * (c->bpp / 8));
> -        c->decode_intra= decode_intra;
> +        c->decode_intra = zmbv_decode_intra;
>      }
>      if (c->flags & ZMBV_KEYFRAME) {
>          expected_size = avctx->width * avctx->height * (c->bpp / 8);
> --
> 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".
Tomas Härdin Oct. 7, 2019, 2:56 p.m.
mån 2019-10-07 klockan 02:57 +0200 skrev Andreas Rheinhardt:
> zmbv has only one function for decoding intra frames, namely
> decode_intra. This can be used to simplify the process of choosing the
> right function pointer.
> 
> This also removes spec-incompliant conversions between function pointers
> and pointers of type void * and thereby fixes the warning "ISO C forbids
> assignment between function pointer and ‘void *’" that GCC emits with
> the -pedantic option.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---
>  libavcodec/zmbv.c | 8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)

Why not just call zmbv_decode_intra() directly?

/Tomas
Andreas Rheinhardt Oct. 7, 2019, 4:42 p.m.
Tomas Härdin:
> mån 2019-10-07 klockan 02:57 +0200 skrev Andreas Rheinhardt:
>> zmbv has only one function for decoding intra frames, namely
>> decode_intra. This can be used to simplify the process of choosing the
>> right function pointer.
>>
>> This also removes spec-incompliant conversions between function pointers
>> and pointers of type void * and thereby fixes the warning "ISO C forbids
>> assignment between function pointer and ‘void *’" that GCC emits with
>> the -pedantic option.
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
>> ---
>>  libavcodec/zmbv.c | 8 +-------
>>  1 file changed, 1 insertion(+), 7 deletions(-)
> 
> Why not just call zmbv_decode_intra() directly?
> 
> /Tomas
> 
This is a few lines below:

    if (!c->decode_intra) {
        av_log(avctx, AV_LOG_ERROR, "Error! Got no format or no
keyframe!\n");
        return AVERROR_INVALIDDATA;
    }

So whether this function pointer is set or not is used as a test for
initialisation or so. Not being familiar with this code I therefore
opted to not change the observable outcome of it at all.

- Andreas
Tomas Härdin Oct. 8, 2019, 11:44 a.m.
mån 2019-10-07 klockan 16:42 +0000 skrev Andreas Rheinhardt:
> Tomas Härdin:
> > mån 2019-10-07 klockan 02:57 +0200 skrev Andreas Rheinhardt:
> > > zmbv has only one function for decoding intra frames, namely
> > > decode_intra. This can be used to simplify the process of choosing the
> > > right function pointer.
> > > 
> > > This also removes spec-incompliant conversions between function pointers
> > > and pointers of type void * and thereby fixes the warning "ISO C forbids
> > > assignment between function pointer and ‘void *’" that GCC emits with
> > > the -pedantic option.
> > > 
> > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> > > ---
> > >  libavcodec/zmbv.c | 8 +-------
> > >  1 file changed, 1 insertion(+), 7 deletions(-)
> > 
> > Why not just call zmbv_decode_intra() directly?
> > 
> > /Tomas
> > 
> This is a few lines below:
> 
>     if (!c->decode_intra) {
>         av_log(avctx, AV_LOG_ERROR, "Error! Got no format or no
> keyframe!\n");
>         return AVERROR_INVALIDDATA;
>     }
> 
> So whether this function pointer is set or not is used as a test for
> initialisation or so. Not being familiar with this code I therefore
> opted to not change the observable outcome of it at all.

You could just replace that with a simple flag :)

/Tomas

Patch hide | download patch | download mbox

diff --git a/libavcodec/zmbv.c b/libavcodec/zmbv.c
index 99e735cfd9..8e9e13c936 100644
--- a/libavcodec/zmbv.c
+++ b/libavcodec/zmbv.c
@@ -425,7 +425,6 @@  static int decode_frame(AVCodecContext *avctx, void *data, int *got_frame, AVPac
     c->flags = buf[0];
     buf++; len--;
     if (c->flags & ZMBV_KEYFRAME) {
-        void *decode_intra = NULL;
         c->decode_intra= NULL;
 
         if (len < 6)
@@ -436,7 +435,6 @@  static int decode_frame(AVCodecContext *avctx, void *data, int *got_frame, AVPac
         c->fmt = buf[3];
         c->bw = buf[4];
         c->bh = buf[5];
-        c->decode_intra = NULL;
         c->decode_xor = NULL;
 
         buf += 6;
@@ -460,7 +458,6 @@  static int decode_frame(AVCodecContext *avctx, void *data, int *got_frame, AVPac
         switch (c->fmt) {
         case ZMBV_FMT_8BPP:
             c->bpp = 8;
-            decode_intra = zmbv_decode_intra;
             c->decode_xor = zmbv_decode_xor_8;
             avctx->pix_fmt = AV_PIX_FMT_PAL8;
             c->stride = c->width;
@@ -468,7 +465,6 @@  static int decode_frame(AVCodecContext *avctx, void *data, int *got_frame, AVPac
         case ZMBV_FMT_15BPP:
         case ZMBV_FMT_16BPP:
             c->bpp = 16;
-            decode_intra = zmbv_decode_intra;
             c->decode_xor = zmbv_decode_xor_16;
             if (c->fmt == ZMBV_FMT_15BPP)
                 avctx->pix_fmt = AV_PIX_FMT_RGB555LE;
@@ -479,7 +475,6 @@  static int decode_frame(AVCodecContext *avctx, void *data, int *got_frame, AVPac
 #ifdef ZMBV_ENABLE_24BPP
         case ZMBV_FMT_24BPP:
             c->bpp = 24;
-            decode_intra = zmbv_decode_intra;
             c->decode_xor = zmbv_decode_xor_24;
             avctx->pix_fmt = AV_PIX_FMT_BGR24;
             c->stride = c->width * 3;
@@ -487,7 +482,6 @@  static int decode_frame(AVCodecContext *avctx, void *data, int *got_frame, AVPac
 #endif //ZMBV_ENABLE_24BPP
         case ZMBV_FMT_32BPP:
             c->bpp = 32;
-            decode_intra = zmbv_decode_intra;
             c->decode_xor = zmbv_decode_xor_32;
             avctx->pix_fmt = AV_PIX_FMT_BGR0;
             c->stride = c->width * 4;
@@ -517,7 +511,7 @@  static int decode_frame(AVCodecContext *avctx, void *data, int *got_frame, AVPac
         }
         memset(c->cur, 0, avctx->width * avctx->height * (c->bpp / 8));
         memset(c->prev, 0, avctx->width * avctx->height * (c->bpp / 8));
-        c->decode_intra= decode_intra;
+        c->decode_intra = zmbv_decode_intra;
     }
     if (c->flags & ZMBV_KEYFRAME) {
         expected_size = avctx->width * avctx->height * (c->bpp / 8);