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?
> > + 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.
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".
