From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.ffmpeg.org (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTPS id 021614EF60 for ; Fri, 13 Jun 2025 22:10:25 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.ffmpeg.org (Postfix) with ESMTP id 04DB168CC5D; Sat, 14 Jun 2025 01:10:22 +0300 (EEST) Received: from iq.passwd.hu (iq.passwd.hu [217.27.212.140]) by ffbox0-bg.ffmpeg.org (Postfix) with ESMTP id 274A468BD49 for ; Sat, 14 Jun 2025 01:10:16 +0300 (EEST) Received: from localhost (localhost [127.0.0.1]) by iq.passwd.hu (Postfix) with ESMTP id E0443E7E00 for ; Sat, 14 Jun 2025 00:07:19 +0200 (CEST) X-Virus-Scanned: amavisd-new at passwd.hu Received: from iq.passwd.hu ([127.0.0.1]) by localhost (iq.passwd.hu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id xhTwKWUC-sUk for ; Sat, 14 Jun 2025 00:07:17 +0200 (CEST) Received: from iq (iq [217.27.212.140]) by iq.passwd.hu (Postfix) with ESMTPS id 68D77E79A5 for ; Sat, 14 Jun 2025 00:07:17 +0200 (CEST) Date: Sat, 14 Jun 2025 00:07:17 +0200 (CEST) From: Marton Balint To: FFmpeg development discussions and patches In-Reply-To: Message-ID: <34253e5e-c1c1-75bc-1a0a-8b97da8361b9@passwd.hu> References: <0132f3f9332b2ee0c53cb8fbc34ad3e956d8b9b2.1749794066.git.ffmpegagent@gmail.com> <4c613514-e388-b859-4f33-c658051520d6@passwd.hu> MIME-Version: 1.0 Subject: Re: [FFmpeg-devel] [PATCH 01/10] avformat/segment: Add segment_write_temp option 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 Fri, 13 Jun 2025, softworkz . wrote: > Hi Marton, > > >> -----Original Message----- >> From: ffmpeg-devel On Behalf Of >> Marton Balint >> Sent: Freitag, 13. Juni 2025 22:38 >> To: FFmpeg development discussions and patches > devel@ffmpeg.org> >> Subject: Re: [FFmpeg-devel] [PATCH 01/10] avformat/segment: Add >> segment_write_temp option >> >> >> >> On Fri, 13 Jun 2025, softworkz wrote: >> >>> From: softworkz >>> >>> Allows to write segments as temp files (.tmp) which >>> are renamed on completion. >>> >>> Signed-off-by: softworkz >>> --- >>> libavformat/segment.c | 30 +++++++++++++++++++++++++++--- >>> 1 file changed, 27 insertions(+), 3 deletions(-) >>> >>> diff --git a/libavformat/segment.c b/libavformat/segment.c >>> index 65323ec678..04e973a198 100644 >>> --- a/libavformat/segment.c >>> +++ b/libavformat/segment.c >>> @@ -121,6 +121,7 @@ typedef struct SegmentContext { >>> int break_non_keyframes; >>> int write_empty; >>> >>> + int segment_write_temp; ///< write segments as temp files >> and rename on completion >>> int use_rename; >>> char temp_list_filename[1024]; >>> >>> @@ -226,6 +227,15 @@ static int >> set_segment_filename(AVFormatContext *s) >>> seg->entry_prefix ? seg->entry_prefix : "", >>> av_basename(oc->url)); >>> >>> + // Write segment as a temp file and rename on completion >>> + if(seg->segment_write_temp) { >>> + av_strlcatf(buf, sizeof(buf), ".tmp"); >>> + char *temp_name = av_strdup(buf); >>> + if (!temp_name) >>> + return AVERROR(ENOMEM); >> >> You should use av_asprintf() directly instead of strlcatf() + >> av_strdup() > > I could be wrong, but I was thinking that this way is more efficient, > because in the av_strdup case, the size of the needed allocation is > already known, while I would suppose that av_asprintf() needs to > allocate a larger size of memory because it cannot predict the eventual > size after applying all the formats. Am I on a wrong track? I think it does not matter, this is not performance critical code, so simplicity/readability is more imporant. Also we should avoid hard coded path length limits if we can, an by using a static buffer you can hit that. > > >>> + ff_format_set_url(oc, temp_name); >>> + } >>> + >>> return 0; >>> } >>> >>> @@ -372,7 +382,7 @@ static int segment_end(AVFormatContext *s, >> int write_trailer, int is_last) >>> SegmentListEntry *entry = av_mallocz(sizeof(*entry)); >>> if (!entry) { >>> ret = AVERROR(ENOMEM); >>> - goto end; >>> + goto fail; >>> } >>> >>> /* append new element */ >>> @@ -393,7 +403,7 @@ static int segment_end(AVFormatContext *s, >> int write_trailer, int is_last) >>> } >>> >>> if ((ret = segment_list_open(s)) < 0) >>> - goto end; >>> + goto fail; >>> for (entry = seg->segment_list_entries; entry; entry >> = entry->next) >>> segment_list_print_entry(seg->list_pb, seg- >>> list_type, entry, s); >>> if (seg->list_type == LIST_TYPE_M3U8 && is_last) >>> @@ -450,7 +460,20 @@ static int segment_end(AVFormatContext *s, >> int write_trailer, int is_last) >>> } >>> } >>> >>> -end: >>> + ff_format_io_close(oc, &oc->pb); >>> + >>> + // Now rename the .tmp file to its actual name. >>> + if (seg->segment_write_temp) { >>> + char *final_filename = av_strndup(oc->url, strlen(oc- >>> url) - 4); >>> + if (!final_filename) >>> + return AVERROR(ENOMEM); >> >> goto fail? > > At that point, ff_format_io_close(oc, &oc->pb) has > already been called. Ah, I missed that, OK then. Thanks, Marton > > Here's the full block (after the patch): > > > ff_format_io_close(oc, &oc->pb); > > // Now rename the .tmp file to its actual name. > if (seg->segment_write_temp) { > char *final_filename = av_strndup(oc->url, strlen(oc->url) - 4); > if (!final_filename) > return AVERROR(ENOMEM); > ret = ff_rename(oc->url, final_filename, s); > av_free(final_filename); > } > > return ret; > > fail: > ff_format_io_close(oc, &oc->pb); > > return ret; > } > > > Thanks, > sw > > _______________________________________________ > 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".