[FFmpeg-devel] [PATCH 02/18] avcodec/vp8: Disable lf_delta for VP7
Andreas Rheinhardt
andreas.rheinhardt at outlook.com
Sun Sep 11 07:38:39 EEST 2022
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, because all the functions that use
is_vp7 in this decoder are marked as av_always_inline.
If it were otherwise, several of the patches 3-18 would make no sense
and would just add another runtime check.
- Andreas
More information about the ffmpeg-devel
mailing list