[FFmpeg-devel] [PATCH 02/18] avcodec/vp8: Disable lf_delta for VP7

[Ronald S. Bultje]

Hi,

On Sun, Sep 11, 2022 at 12:38 AM Andreas Rheinhardt <
andreas.rheinhardt at outlook.com> wrote:

> Peter Ross:
> > On Sat, Sep 10, 2022 at 03:07:13AM +0200, Andreas Rheinhardt wrote:
> >> It is a VP8-only feature.
> >>
> >> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at outlook.com>
> >> ---
> >>  libavcodec/vp8.c | 3 +--
> >>  1 file changed, 1 insertion(+), 2 deletions(-)
> >>
> >> diff --git a/libavcodec/vp8.c b/libavcodec/vp8.c
> >> index c00c5c975e..04a2113cc8 100644
> >> --- a/libavcodec/vp8.c
> >> +++ b/libavcodec/vp8.c
> >> @@ -637,7 +637,6 @@ static int vp7_decode_frame_header(VP8Context *s,
> const uint8_t *buf, int buf_si
> >>          for (i = 0; i < 2; i++)
> >>              memcpy(s->prob->mvc[i], vp7_mv_default_prob[i],
> >>                     sizeof(vp7_mv_default_prob[i]));
> >> -        memset(&s->lf_delta, 0, sizeof(s->lf_delta));
> >>          memcpy(s->prob[0].scan, ff_zigzag_scan,
> sizeof(s->prob[0].scan));
> >>      }
> >>
> >> @@ -2171,7 +2170,7 @@ void filter_level_for_mb(VP8Context *s,
> VP8Macroblock *mb,
> >>      } else
> >>          filter_level = s->filter.level;
> >>
> >> -    if (s->lf_delta.enabled) {
> >> +    if (!is_vp7 && s->lf_delta.enabled) {
> >>          filter_level += s->lf_delta.ref[mb->ref_frame];
> >>          filter_level += s->lf_delta.mode[mb->mode];
> >>      }
> >
> > what's the motivation for patches 01 and 02?
> > are you not just adding another condition (is_vp7) to evaluate at decode
> time?
> > if its improved readability, then suggest renaming 'lf_delta' to
> 'lf_delta_vp8'
> >
> > pathces 3-18 look fine to me.
> >
>
> is_vp7 is evaluated at compile-time
>

This should probably be changed to be uppercase then. Lowercase suggests
it's a variable.

Ronald