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 ESMTPS id 3A1E54DE2C for ; Tue, 4 Mar 2025 16:30:57 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 8A00468E9E2; Tue, 4 Mar 2025 18:30:51 +0200 (EET) Received: from mail-wm1-f44.google.com (mail-wm1-f44.google.com [209.85.128.44]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 5A0F368E6D1 for ; Tue, 4 Mar 2025 18:30:44 +0200 (EET) Received: by mail-wm1-f44.google.com with SMTP id 5b1f17b1804b1-43bbc8b7c65so27847335e9.0 for ; Tue, 04 Mar 2025 08:30:44 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1741105843; x=1741710643; darn=ffmpeg.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:to:subject:user-agent:mime-version:date:message-id:from :to:cc:subject:date:message-id:reply-to; bh=1zSJ6X3JZy49WZJJKwsdjT982DG0Wy3n1KfoZnNW5Cg=; b=SBpVU7HjhnB/fgrFkqstBcnqpwenidaaDIvn4p5kSZQQEtNKs17DK++/VyscGU2igY bpDEP6fubhrtS5gJsv2/UMuwKIvvmjYkg1HZqo2O1pzlCs7Oq535biNSNT76bJA48kT5 AfJVwO+Worq24YlLbYfgGf6cmViLn9PMRwwU3mhabVaEDl8IWkUJKdcuksh+XJYFVNA+ yfv7EvQbI4yBrzMlOTuhKUxtf9hqyIQ9ic/7d1o3ligcJ3XM3Kzu/w5t7uCII+mnPXcn 3ak6jvWrbcaerRXo3U0++/wTfghU5CaR/JOSI+JR5EhlTklPcSL57IVzAJVs5WRr8D5J eqIQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1741105843; x=1741710643; h=content-transfer-encoding:in-reply-to:from:content-language :references:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=1zSJ6X3JZy49WZJJKwsdjT982DG0Wy3n1KfoZnNW5Cg=; b=MF94ZVGCuFA0op9bUkfpiIlrCdd82FX0ugUTi/ta15pUgOdpVdValCyO4j8dhdiP8C OsSpoACneVp/wBuj5/NBQmnPfwfvkP1eLFivV2QcEhkSxAidDJEpc6OuVyqDNadjVlQQ 6w2KrOwZ2NmcX/wFfqsOAN6uxGePaToRD9AN6vRAEZ3Cacs7MhhueED2FeMn1f5atXpe u8sJxGMzNsg2POVXEM50tiHnAHOTap+o65D/ze9zX7TQCehud3yh8GYwqmSzIU/BfPEa Jq2VL281acX/oMfRO35thNLunLlxjSQSW1JmNmCB2/RyPOVXjvZv0BUY5ClKyGtZwXBx FCbA== X-Gm-Message-State: AOJu0YyRhV8hiegNBI4fy6sY0z0Bsoj9Y4AswLazGsRRVyuTLaGGzGl3 /LhZzFmC/zPw+yhe3Qfd17JtHhoTmiSgB6ArbTYwc84IwdX6Agbbxa95Oo6N X-Gm-Gg: ASbGnctE1LS6yCrzhx8Wr70WhJQog3AwLxqWimWTuPAnokV5EEflDD1RAg89Ksrx88l SJW+vIGrsUDvKfcrXt91HHka3LSHqlcfCBQvotpzFhURBRoLtneD0NbBFJsl2Rl3DpVHrVctKNE ZeRjhn0AxhGXn950hjs56eHbfPFDj4IObMKr+bwbJZ9VfVjDz8bQ1kV5xI5USYV071hXsH+zWnH lKcjS4jGuvnrRWm0YIAkFcrQIawetHgByeTGSvzwKFbsMpLkCT1OjTDGwAiAjjdzQmr5W9n1uNX Y7CuoICzhR7npE2ZzxXCQAtsqFvTB4DgCKDkjrifDbRy6krv+AJracz5hBHHGEad9LQwV3gNzcR y/HEFbE+rtUHWB0Kkzw== X-Google-Smtp-Source: AGHT+IHRq5Ek1KNIZXvQeM4ydUPQD2taXPa0V4kBrXqqM8eRK4RRAuq+9wb5HmeA+hg/Bhzo9sIOPA== X-Received: by 2002:a05:600c:470e:b0:43b:cf9c:700c with SMTP id 5b1f17b1804b1-43bcf9c7a4dmr18336625e9.16.1741105843008; Tue, 04 Mar 2025 08:30:43 -0800 (PST) Received: from [192.168.1.137] (20.238-74-193.fia-dyn.isp.proximus.be. [193.74.238.20]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-43bc63bcaafsm61448525e9.28.2025.03.04.08.30.42 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 04 Mar 2025 08:30:42 -0800 (PST) Message-ID: <0ef6683d-688a-4e09-ab65-bd2131253a0d@gmail.com> Date: Tue, 4 Mar 2025 17:30:41 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird To: ffmpeg-devel@ffmpeg.org References: Content-Language: en-US From: Ramiro Polla In-Reply-To: Subject: Re: [FFmpeg-devel] Misc mpegvideo patches 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-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: On 3/4/25 14:42, Andreas Rheinhardt wrote: > (Mostly trivial) patches attached. A branch is at > https://github.com/mkver/FFmpeg/tree/mpegvideo_misc [PATCH 10/40] avcodec/mpegvideo_enc: Move default_mv_penalty to h261enc.c > diff --git a/libavcodec/h261enc.c b/libavcodec/h261enc.c > index dabab9d80a..e33bf35a8a 100644 > --- a/libavcodec/h261enc.c > +++ b/libavcodec/h261enc.c > @@ -46,6 +46,7 @@ static struct VLCLUT { > uint16_t code; > } vlc_lut[H261_MAX_RUN + 1][32 /* 0..2 * H261_MAX_LEN are used */]; > > +static uint8_t mv_penalty[MAX_FCODE + 1][MAX_DMV * 2 + 1]; > static uint8_t uni_h261_rl_len [64 * 128]; > static uint8_t uni_h261_rl_len_last[64 * 128]; > static uint8_t h261_mv_codes[64][2]; > @@ -370,6 +371,8 @@ av_cold int ff_h261_encode_init(MpegEncContext *s) > s->max_qcoeff = 127; > s->ac_esc_length = H261_ESC_LEN; > > + s->me.mv_penalty = mv_penalty; > + > s->intra_ac_vlc_length = s->inter_ac_vlc_length = uni_h261_rl_len; > s->intra_ac_vlc_last_length = s->inter_ac_vlc_last_length = uni_h261_rl_len_last; > ff_thread_once(&init_static_once, h261_encode_init_static); This global mv_penalty doesn't seem to be ever initialized; it could be declared const. But it also makes me think that whatever code is using this mv_penalty, which is always set to zero, might be calculating things wrong. [PATCH 15/40] avcodec/ituh263enc: Make SVQ1+Snowenc stop calling ff_h263_encode_init() > diff --git a/libavcodec/ituh263enc.c b/libavcodec/ituh263enc.c > index 02da090ba4..8313b2c2c1 100644 > --- a/libavcodec/ituh263enc.c > +++ b/libavcodec/ituh263enc.c > @@ -65,6 +65,127 @@ static uint8_t uni_h263_inter_rl_len [64*64*2*2]; [...] > +static av_cold void h263_encode_init_static(void) > +{ > + static uint8_t rl_intra_table[2][2 * MAX_RUN + MAX_LEVEL + 3]; > + > + ff_rl_init(&ff_rl_intra_aic, rl_intra_table); > + ff_h263_init_rl_inter(); > + > + init_uni_h263_rl_tab(&ff_rl_intra_aic, uni_h263_intra_aic_rl_len); > + init_uni_h263_rl_tab(&ff_h263_rl_inter, uni_h263_inter_rl_len); > + > + init_mv_penalty_and_fcode(); > +} > + > +av_cold const uint8_t (*ff_h263_get_mv_penalty(void))[MAX_DMV*2+1] > +{ > + static AVOnce init_static_once = AV_ONCE_INIT; > + > + ff_thread_once(&init_static_once, h263_encode_init_static); > + > + return mv_penalty; > +} > + This approach kind of hides the rest of h263_encode_init_static() inside ff_h263_get_mv_penalty(), so the name is a bit misleading. I'd expect h263 to still call some init function and ff_h263_get_mv_penalty(), and SVQ1 and Snow to only call ff_h263_get_mv_penalty(), which would only take care of the mv_penalty table. [PATCH 18/40] avcodec/ituh263dec: Reorder branches > diff --git a/libavcodec/ituh263dec.c b/libavcodec/ituh263dec.c > index e0f3034e57..93349a3b0e 100644 > --- a/libavcodec/ituh263dec.c > +++ b/libavcodec/ituh263dec.c > @@ -543,6 +543,8 @@ static int h263_decode_block(MpegEncContext * s, int16_t * block, > if (s->h263_aic && s->mb_intra) { > rl = &ff_rl_intra_aic; > i = 0; > + if (!coded) > + goto not_coded; Why not move the if before setting rl (doesn't seem to be used) and i (will be overwritten anyways)? > if (s->ac_pred) { > if (s->h263_aic_dir) > scan_table = s->permutated_intra_v_scantable; /* left */ > @@ -587,8 +589,6 @@ static int h263_decode_block(MpegEncContext * s, int16_t * block, > i = 0; > } > if (!coded) { > - if (s->mb_intra && s->h263_aic) > - goto not_coded; > s->block_last_index[n] = i - 1; > return 0; > } [PATCH 20/40] avcodec/mpeg4video: Split ff_mpeg4_pred_dc() > diff --git a/libavcodec/mpeg4videoenc.c b/libavcodec/mpeg4videoenc.c > index 64fb96a0cf..26f9b40ff7 100644 > --- a/libavcodec/mpeg4videoenc.c > +++ b/libavcodec/mpeg4videoenc.c > @@ -806,8 +806,14 @@ void ff_mpeg4_encode_mb(MpegEncContext *s, int16_t block[6][64], > const uint8_t *scan_table[6]; > int i; > > - for (i = 0; i < 6; i++) > - dc_diff[i] = ff_mpeg4_pred_dc(s, i, block[i][0], &dir[i], 1); > + for (int i = 0; i < 6; i++) { Redeclaring i inside for. > + int pred = ff_mpeg4_pred_dc(s, i, &dir[i]); > + int scale = i < 4 ? s->y_dc_scale : s->c_dc_scale; > + > + pred = FASTDIV((pred + (scale >> 1)), scale); > + dc_diff[i] = block[i][0] - pred; > + s->dc_val[0][s->block_index[i]] = av_clip_uintp2(block[i][0] * scale, 11); > + } > > if (s->avctx->flags & AV_CODEC_FLAG_AC_PRED) { > s->ac_pred = decide_ac_pred(s, block, dir, scan_table, zigzag_last_index); [PATCH 35/40] avcodec/speedhqenc: Inline ff_speedhq_mb_y_order_to_mb() Why? I'm not against this, but it doesn't seem to have a greater reason. Ramiro _______________________________________________ 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".