On Fri, 13 Jun 2025, softworkz . wrote:
Hi Marton,
-----Original Message-----
From: ffmpeg-devel <ffmpeg-devel-boun...@ffmpeg.org> On Behalf Of
Marton Balint
Sent: Freitag, 13. Juni 2025 22:38
To: FFmpeg development discussions and patches <ffmpeg-
de...@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 <softwo...@hotmail.com>
Allows to write segments as temp files (.tmp) which
are renamed on completion.
Signed-off-by: softworkz <softwo...@hotmail.com>
---
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-requ...@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-requ...@ffmpeg.org with subject "unsubscribe".
