From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id B990643461 for ; Sun, 11 Sep 2022 15:09:55 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 2503068B9ED; Sun, 11 Sep 2022 18:09:52 +0300 (EEST) Received: from mail-ej1-f53.google.com (mail-ej1-f53.google.com [209.85.218.53]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id BCCEF68B8EC for ; Sun, 11 Sep 2022 18:09:45 +0300 (EEST) Received: by mail-ej1-f53.google.com with SMTP id go34so14786344ejc.2 for ; Sun, 11 Sep 2022 08:09:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=to:subject:message-id:date:from:in-reply-to:references:mime-version :from:to:cc:subject:date; bh=KknCCetQVBa7dBfz4jl0RX/CvKgOKVJe23EgEjJVhl4=; b=P+tQIDAVu2obdPwjU9T5YHGWz+bNwVFopB7J+H0cIHI/toBPAtwGRmNfLGk9YzUarR 6IOLzgMZkZxCCowhubVQ+22mnHtHTHJ2S0Ee/XykwL7n/YTFFl7w8Tq0E+jZuZGb7JkL OqyRZO6/RJnbBOl0AnxQiZfGBpVUrOBVN5Q4kkiN3x9+Qnm5WipIGIdK9eZIcDV46H+o Pk6Ex28dLP8JC6D4emImZHSQWDL9kjRymotGa84/2Z34TlgsLRbjs75lv2cyu2lbPfPP 9AG8fe3QQQm13zXQuoZf9PB0RYDCrL+aRF0rQ5Qvmc7VTVs1tUQ8vSG0CJj7xliriJLg 9DVQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=to:subject:message-id:date:from:in-reply-to:references:mime-version :x-gm-message-state:from:to:cc:subject:date; bh=KknCCetQVBa7dBfz4jl0RX/CvKgOKVJe23EgEjJVhl4=; b=PIDkWueutss2P+Ayhfpuf2xANIIgwnYtbN0cruaRYcFce1rJaclyaMV5eivxsW/P3m 2fp8GB8REO9Z4w7kIXCqhfwhnjioGw3imBJtAEGxZr06tTaNG1oxP6k3h0eCgR6chfXx KPVmCmGm3eCYVziujFJNTPM6qCacc6ABu4JGnXNOPpC8A/Ma2ZyJ+l0Tpz/x2mddiByw lA8F0SgCBata5kmL0Fj06sGfc8i0hLICvmQ9gWALMeNDXKB2uS+7m7o5EhT8F3VyhEPX ZZctV5t92MNoA8axArySx63RVgxp9vL61VVllfsZD227vWvkTXUnvw1ypl8j3tzGgLe1 J40Q== X-Gm-Message-State: ACgBeo1SpPRNXiQQhxPdUNDy20SF23SgIIJtRQyJeT2mBFaaC1PAIWeo f0rbewt/u30pUQHEtSJE3tC12lCzeevLg/Lxiu18BHVZ X-Google-Smtp-Source: AA6agR5AdSRm6d4Bl6b2eW2Wl6yhm6mdWZ9w6enpDUKCTbww3kGpSwzJ2warWJ8n4KPpu2ixMlEXKbyIepdTC2QFdwY= X-Received: by 2002:a17:906:cc4e:b0:77c:b7a:9de6 with SMTP id mm14-20020a170906cc4e00b0077c0b7a9de6mr3100921ejb.531.1662908984617; Sun, 11 Sep 2022 08:09:44 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: "Ronald S. Bultje" Date: Sun, 11 Sep 2022 11:09:32 -0400 Message-ID: To: FFmpeg development discussions and patches X-Content-Filtered-By: Mailman/MimeDel 2.1.29 Subject: Re: [FFmpeg-devel] [PATCH 02/18] avcodec/vp8: Disable lf_delta for VP7 X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: Hi, On Sun, Sep 11, 2022 at 10:41 AM Ronald S. Bultje wrote: > Hi, > > On Sun, Sep 11, 2022 at 12:38 AM Andreas Rheinhardt < > andreas.rheinhardt@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 >> >> --- >> >> 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. > It's inline, not macro, apparently. I don't like the impact on readability here. Part of me wonders whether it doesn't make more sense to split this file in vp7.c, vp8.c and vp78.c, and have a proper design of two decoders and separate/shared parent/child contexts definitions... A classic FFmpeg dilemma, I guess. The problem is basically that this complicates people who have to debug this code when issues occur (me) at the benefit of losing some dead code in vp7. I'm not super-excited about that... Ronald _______________________________________________ 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".