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 9AED440E9E for ; Wed, 9 Feb 2022 09:17:29 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 4BE7A68B1CA; Wed, 9 Feb 2022 11:17:27 +0200 (EET) Received: from mail-yb1-f170.google.com (mail-yb1-f170.google.com [209.85.219.170]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 956EE68B0B6 for ; Wed, 9 Feb 2022 11:17:20 +0200 (EET) Received: by mail-yb1-f170.google.com with SMTP id x136so2316026ybe.11 for ; Wed, 09 Feb 2022 01:17:20 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to; bh=UsrrFEU26l25iMeV0irjbA2xsEfqawTXvfdnAqBD+1E=; b=NsLE1IGeUy7ZOPbBzPNvQU0thJAIg1L/eBZLV3mF2uo9wdfzKDbUh6yaZHLF00VeKE eCnjTxKujd36+GaKxSdBPt/3T69qdlqppSZS4GPK+hyCmlUUyLCJ8mOKE2y5K1iDVcfM cqPaWpcyQtlOSIdaIfJ0bLgjyYqq6VoIWeVqZaX/dKoFe5oMAE8g2qmJC4FXdFhl4EAS 4UuvR6++kFWdpLO+nVbSQabv8ZZufmVW4Ww258CGWXmfLlwjhLvjLnGHaO1a3iW3hMq/ yj1q6sk9rGZJnKU5oDJj/AnQLWmPtLAIUKvr2ZsvMgwCEwnFxuSBFA1+RWIoafq6p69A yOyQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to; bh=UsrrFEU26l25iMeV0irjbA2xsEfqawTXvfdnAqBD+1E=; b=ABukxLKPY35ErPEUvmbs5IRtVq9o39gKEc/9HyNoIGCH+NodWWvhQZ/rT7AWExV3JI 2Q2DrYerL3XUTbUIRVUV9leXUKk+sJ0o/m1xcUwGKHt06XMEZ/qXaZmyC/mvR1zouwAc qUXVN3HnckKLWDso6DDyz4lgMGWDcmpSLYD5Z0xCVsPU6J+nFczr1vniE4GIYUHaQDSv OV3Pfh9R9PBCV+4wkecrC9naIEmoVnx+C5euLy9aFVp8HVxOTypFvQu3vEQDIjuDXqG/ DNmQxIUTkpyShivvrgZLi/R3iNxAl0vPXgKqCk7s5Fz4dBKkEOhAar6LEO/c6u7jh6vY A3SQ== X-Gm-Message-State: AOAM5308qUbJ/6ODIea3b3/uW3xVf/Ki517oBha5D6+PUKy81cdJBpaS FCGK6BYHTMG027QW3gilRzUGYLZBY40BeogNqH2Z8rcvJIU= X-Google-Smtp-Source: ABdhPJz1HjFqQgps3MxJhQX5B3tbvN8HwSDK66AIKPPrx+LsIamDRA1CEfF/6NSaQ1ee29RDfhKXR2yO6LBzlmyk+hE= X-Received: by 2002:a81:e50b:: with SMTP id s11mr1142678ywl.262.1644398239062; Wed, 09 Feb 2022 01:17:19 -0800 (PST) MIME-Version: 1.0 References: <20220110145836.3449558-1-alankelly@google.com> <20220203141130.GP2829255@pb2> In-Reply-To: <20220203141130.GP2829255@pb2> From: Alan Kelly Date: Wed, 9 Feb 2022 10:17:08 +0100 Message-ID: To: FFmpeg development discussions and patches X-Content-Filtered-By: Mailman/MimeDel 2.1.29 Subject: Re: [FFmpeg-devel] [PATCH 1/4] libswscale: Re-factor ff_shuffle_filter_coefficients. 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 Michael, Thanks for your feedback. I have updated the patches and split this patch into two, one with cosmetic fixes and one propagating the errors. Since there is now an extra patch in the set and the commit messages have changed, new threads have been started. Alan On Thu, Feb 3, 2022 at 3:11 PM Michael Niedermayer wrote: > On Mon, Jan 10, 2022 at 03:58:33PM +0100, Alan Kelly wrote: > > Make the code more readable, follow the style guide and propagate memory > > allocation errors. > > Cosmetics and bugfixes should not be in the same patch > > > > --- > > libswscale/swscale_internal.h | 2 +- > > libswscale/utils.c | 68 ++++++++++++++++++++--------------- > > 2 files changed, 40 insertions(+), 30 deletions(-) > > > > diff --git a/libswscale/swscale_internal.h > b/libswscale/swscale_internal.h > > index 3a78d95ba6..26d28d42e6 100644 > > --- a/libswscale/swscale_internal.h > > +++ b/libswscale/swscale_internal.h > > @@ -1144,5 +1144,5 @@ void ff_sws_slice_worker(void *priv, int jobnr, > int threadnr, > > #define MAX_LINES_AHEAD 4 > > > > //shuffle filter and filterPos for hyScale and hcScale filters in avx2 > > -void ff_shuffle_filter_coefficients(SwsContext *c, int* filterPos, int > filterSize, int16_t *filter, int dstW); > > +int ff_shuffle_filter_coefficients(SwsContext *c, int* filterPos, int > filterSize, int16_t *filter, int dstW); > > #endif /* SWSCALE_SWSCALE_INTERNAL_H */ > > diff --git a/libswscale/utils.c b/libswscale/utils.c > > index c5ea8853d5..52f07e1661 100644 > > --- a/libswscale/utils.c > > +++ b/libswscale/utils.c > > @@ -278,39 +278,47 @@ static const FormatEntry format_entries[] = { > > [AV_PIX_FMT_P416LE] = { 1, 1 }, > > }; > > > > -void ff_shuffle_filter_coefficients(SwsContext *c, int *filterPos, int > filterSize, int16_t *filter, int dstW){ > > +int ff_shuffle_filter_coefficients(SwsContext *c, int *filterPos, > > + int filterSize, int16_t *filter, > > + int dstW) > > +{ > > #if ARCH_X86_64 > > > - int i, j, k, l; > > + int i = 0, j = 0, k = 0; > > why? > they are set when used if iam not mistaken > > > > int cpu_flags = av_get_cpu_flags(); > > > + if (!filter || dstW % 16 != 0) return 0; > > please add \n also a comment what the dstW & 16 case exactly does and why > > > [...] > > int sws_isSupportedInput(enum AVPixelFormat pix_fmt) > > @@ -1836,7 +1844,8 @@ av_cold int sws_init_context(SwsContext *c, > SwsFilter *srcFilter, > > get_local_pos(c, 0, 0, 0), > > get_local_pos(c, 0, 0, 0))) < 0) > > goto fail; > > - ff_shuffle_filter_coefficients(c, c->hLumFilterPos, > c->hLumFilterSize, c->hLumFilter, dstW); > > + if ((ret = ff_shuffle_filter_coefficients(c, > c->hLumFilterPos, c->hLumFilterSize, c->hLumFilter, dstW)) != 0) > > + goto nomem; > > This is confusing as ret is never used, also error codes are <0 > > thx > > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > Those who are best at talking, realize last or never when they are wrong. > _______________________________________________ > 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". > _______________________________________________ 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".