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 5288840297 for ; Sat, 16 Dec 2023 08:13:21 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 1226368D041; Sat, 16 Dec 2023 10:13:18 +0200 (EET) Received: from shout02.mail.de (shout02.mail.de [62.201.172.25]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 625FE68CF9B for ; Sat, 16 Dec 2023 10:13:11 +0200 (EET) Received: from postfix03.mail.de (postfix03.bt.mail.de [10.0.121.127]) by shout02.mail.de (Postfix) with ESMTP id DB927240B79 for ; Sat, 16 Dec 2023 09:13:10 +0100 (CET) Received: from smtp01.mail.de (smtp04.bt.mail.de [10.0.121.214]) by postfix03.mail.de (Postfix) with ESMTP id BE39980297 for ; Sat, 16 Dec 2023 09:13:10 +0100 (CET) Received: from [127.0.0.1] (localhost [127.0.0.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by smtp01.mail.de (Postfix) with ESMTPSA id 8E718240076 for ; Sat, 16 Dec 2023 09:13:09 +0100 (CET) Message-ID: Date: Sat, 16 Dec 2023 09:13:09 +0100 MIME-Version: 1.0 Content-Language: en-US To: ffmpeg-devel@ffmpeg.org References: <20231215072447.31761-1-thilo.borgmann@mail.de> <20231215072447.31761-2-thilo.borgmann@mail.de> In-Reply-To: X-purgate: clean X-purgate: This mail is considered clean (visit http://www.eleven.de for further information) X-purgate-type: clean X-purgate-Ad: Categorized by eleven eXpurgate (R) http://www.eleven.de X-purgate: This mail is considered clean (visit http://www.eleven.de for further information) X-purgate: clean X-purgate-size: 3998 X-purgate-ID: 154282::1702714390-7DFF8338-C3AED631/0/0 Subject: Re: [FFmpeg-devel] [PATCH 1/2] avfilter: Add fsync filter 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: , From: Thilo Borgmann via ffmpeg-devel Reply-To: FFmpeg development discussions and patches Cc: Thilo Borgmann 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: Am 15.12.23 um 15:17 schrieb Andreas Rheinhardt: > Thilo Borgmann via ffmpeg-devel: >> --- >> Changelog | 1 + >> MAINTAINERS | 1 + >> doc/filters.texi | 33 +++++ >> libavfilter/Makefile | 1 + >> libavfilter/allfilters.c | 1 + >> libavfilter/version.h | 2 +- >> libavfilter/vf_fsync.c | 304 +++++++++++++++++++++++++++++++++++++++ >> 7 files changed, 342 insertions(+), 1 deletion(-) >> create mode 100644 libavfilter/vf_fsync.c >> >> [...] >> +// fills the buffer from cur to end, add \0 at EOF >> +static int buf_fill(FsyncContext *ctx) >> +{ >> + int ret; >> + int num = ctx->end - ctx->cur; >> + >> + ret = avio_read(ctx->avio_ctx, ctx->cur, num); >> + if (ret < 0) >> + return ret; >> + if (ret < num) { >> + *(ctx->cur + ret) = '\0'; >> + } >> + >> + return ret; >> +} >> + >> +// copies cur to end to the beginning and fills the rest >> +static int buf_reload(FsyncContext *ctx) >> +{ >> + int i, ret; >> + int num = ctx->end - ctx->cur; >> + >> + for (i = 0; i < num; i++) { >> + ctx->buf[i] = *ctx->cur++; >> + } >> + >> + ctx->cur = ctx->buf + i; >> + ret = buf_fill(ctx); >> + if (ret < 0) >> + return ret; >> + ctx->cur = ctx->buf; > > I wonder whether you should not just use avio_read_to_bprint() for all > of this. I tested a bit. It appears good for filling the buffer with one function call, getting the stuff out for scanf appears not as well as when and how to refill the buffer. Comparing a bit to f_metadata where this is used, the necessary function get bigger and more complex than this easily. Maybe its just I didn't use it often enough, but this char* handling appears easier and less complex to me. So if you don't insist I'd leave it that way. >> + >> + return ret; >> +} >> + >> +// skip from cur over eol >> +static void buf_skip_eol(FsyncContext *ctx) >> +{ >> + char *i; >> + for (i = ctx->cur; i < ctx->end; i++) { >> + if (*i != '\n')// && *i != '\r') >> + break; >> + } >> + ctx->cur = i; >> +} >> + >> +// get number of bytes from cur until eol >> +static int buf_get_line_count(FsyncContext *ctx) >> +{ >> + int ret = 0; >> + char *i; >> + for (i = ctx->cur; i < ctx->end; i++, ret++) { >> + if (*i == '\0' || *i == '\n') >> + return ret; > > If you unconditionally added a single \0 to the end of the buffer, you > could use strchr() here. I'd need two strchr() calls, for \0 and \n, plus the interpretation of the resulting pointers. Where \0 would always be found at the end of the buffer which needs another if to see if its what I need or do need to load more data. >> [...] >> + >> +static av_cold void fsync_uninit(AVFilterContext *ctx) >> +{ >> + FsyncContext *s = ctx->priv; >> + >> + avio_close(s->avio_ctx); > > avio_closep() > >> + av_freep(&s->buf); >> + av_frame_unref(s->last_frame); > > I expect that this needs to be changed to av_frame_free(). Anyway, you > should run your tests via valgrind/asan. Asan is fine both ways, so I assume it wouldn't catch it? Changed to av_frame_free anyways. There appears no Valgrind nor msan to be available for OSX. 'leaks' also even complains it cannot really do its job: "Can't examine target process's malloc zone asan_0x10c134950, so memory analysis will be incomplete or incorrect. Reason: for security, cannot load non-system library /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/clang/14.0.0/lib/darwin/libclang_rt.asan_osx_dynamic.dylib Process 49784 is not debuggable. Due to security restrictions, leaks can only show or save contents of readonly memory of restricted processes." Am I out of options on OSX? Seems like the first real drawback when on a mac... Fixed all the other things for v3. -Thilo _______________________________________________ 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".