Re: [FFmpeg-devel] [PATCH] Adding mkdir option for img2enc (2nd attempt)
Hi, On 12/27/2017 12:27 PM, Dr Alan Barclay wrote: > Resending the two (git format-patch) patches, without the top lines > removed (which I thought I needed to do as some patch emails didn't seem > to have them). > > All comments and help appreciated. [...] > Subject: [PATCH 1/2] Move mkdir_p (renamed ff_mkdir_p) from hlsenc.c to > utils.c. > > --- > libavformat/hlsenc.c | 35 +-- > libavformat/internal.h | 7 +++ > libavformat/utils.c| 33 + > 3 files changed, 41 insertions(+), 34 deletions(-) On a technical level, this patch looks OK. > Subject: [PATCH 2/2] Adding mkdir option for img2enc. > > --- > libavformat/img2enc.c | 8 > 1 file changed, 8 insertions(+) I'm not sure how others feel about the premise (mkdir in img2enc). I personally don't mind - though, maybe it should be default instead of an option? (Maybe a bad idea.) > +if (img->use_mkdir) { > +char *temp_filename = av_strdup(filename); > +const char *temp_path = av_dirname(temp_filename); > +ff_mkdir_p(temp_path); > +av_free(temp_filename); > +} This lacks error checks for av_strdup and ff_mkdir_p. - Derek ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Adding mkdir option for img2enc (2nd attempt)
Hi, Resending the two (git format-patch) patches, without the top lines removed (which I thought I needed to do as some patch emails didn't seem to have them). All comments and help appreciated. Thanks and Regards, Alan. On 27/12/17 01:41, Michael Niedermayer wrote: On Tue, Dec 26, 2017 at 10:44:31PM +, Dr Alan Barclay wrote: Hi All, Please would someone with an interest in img2enc take a look at my revised patches for a minor feature addition and consider committing it to the main line for me. Example: ffmpeg -i ~/trailer.mp4 -strftime 1 -mkdir 1 %Y/%m/%d/out_%H-%M-%S.jpg Without the new mkdir option, this command will fail if the directory hierarchy for the jpg files does not already exist, which can be difficult to predict for time-stamped directories. This patch adds a mkdir option to img2enc which invites it to make whatever directory hierarchy is necessary for each output file. When used in conjunction with the strftime then the jpg files will be located in a newly created (time-stamped) directory as processing progresses. My typical usage scenario is capturing a long-running live video feed (perhaps time-lapsed) and storing the resulting images in a time-stamped directory hierarchy fashion, rather than as a numbered sequence of files in a single directory. If you look at the code you will see that only a half dozen lines of code were required in img2enc. The function for creating directories already existed in hlsenc.c but I've moved into utils.c as I presumed that was a more generic location for it. All comments appreciated. Thanks ad Regards, Alan. On 17/12/17 22:46, Carl Eugen Hoyos wrote: 2017-12-17 23:41 GMT+01:00 Dr Alan Barclay : Please would someone with an interest in img2enc take a look at my minor feature addition and consider committing it to the main line for me. To be acceptable, the patch has to be split in two and please move the definition into internal.h Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel hlsenc.c | 35 +-- internal.h |7 +++ utils.c| 33 + 3 files changed, 41 insertions(+), 34 deletions(-) 9560fd03958f79f77b01c0e02c55d98e3dc7b937 0001-Move-mkdir_p-renamed-ff_mkdir_p-from-hlsenc.c-to-uti.patch --- libavformat/hlsenc.c | 35 +-- libavformat/internal.h | 7 +++ libavformat/utils.c| 33 + 3 files changed, 41 insertions(+), 34 deletions(-) these patches are missing commit messages patches should be genrated with git format-patch or git send-email [...] ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >From 70675348150c43f898ee97466c123fbf7ff65bcb Mon Sep 17 00:00:00 2001 From: "Dr. Alan Barclay" Date: Tue, 26 Dec 2017 22:17:59 + Subject: [PATCH 1/2] Move mkdir_p (renamed ff_mkdir_p) from hlsenc.c to utils.c. --- libavformat/hlsenc.c | 35 +-- libavformat/internal.h | 7 +++ libavformat/utils.c| 33 + 3 files changed, 41 insertions(+), 34 deletions(-) diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c index bbc2742dc7..633ddc309e 100644 --- a/libavformat/hlsenc.c +++ b/libavformat/hlsenc.c @@ -208,39 +208,6 @@ typedef struct HLSContext { AVIOContext *sub_m3u8_out; } HLSContext; -static int mkdir_p(const char *path) { -int ret = 0; -char *temp = av_strdup(path); -char *pos = temp; -char tmp_ch = '\0'; - -if (!path || !temp) { -return -1; -} - -if (!strncmp(temp, "/", 1) || !strncmp(temp, "\\", 1)) { -pos++; -} else if (!strncmp(temp, "./", 2) || !strncmp(temp, ".\\", 2)) { -pos += 2; -} - -for ( ; *pos != '\0'; ++pos) { -if (*pos == '/' || *pos == '\\') { -tmp_ch = *pos; -*pos = '\0'; -ret = mkdir(temp, 0755); -*pos = tmp_ch; -} -} - -if ((*(pos - 1) != '/') || (*(pos - 1) != '\\')) { -ret = mkdir(temp, 0755); -} - -av_free(temp); -return ret; -} - static int is_http_proto(char *filename) { const char *proto = avio_find_protocol_name(filename); return proto ? (!av_strcasecmp(proto, "http") || !av_strcasecmp(proto, "https")) : 0; @@ -1407,7 +1374,7 @@ static int hls_start(AVFormatContext *s, VariantStream *vs) return AVERROR(ENOMEM); } dir = av_dirname(fn_copy); -if (mkdir_p(dir) == -1 && errno != EEXIST) { +if (ff_mkdir_p(dir) == -1 && errno != EEXIST) { av_log(oc, AV_LOG_ERROR, "Could not create directory %s with use_localtime_mkdir\n", dir); av_free(fn_copy); r
Re: [FFmpeg-devel] [PATCH] Adding mkdir option for img2enc (2nd attempt)
On Tue, Dec 26, 2017 at 10:44:31PM +, Dr Alan Barclay wrote: > Hi All, > > Please would someone with an interest in img2enc take a look at my revised > patches for a minor feature addition and consider committing it to the main > line for me. > > Example: > ffmpeg -i ~/trailer.mp4 -strftime 1 -mkdir 1 %Y/%m/%d/out_%H-%M-%S.jpg > > Without the new mkdir option, this command will fail if the directory > hierarchy for the jpg files does not already exist, which can be difficult > to predict for time-stamped directories. > > This patch adds a mkdir option to img2enc which invites it to make whatever > directory hierarchy is necessary for each output file. When used in > conjunction with the strftime then the jpg files will be located in a newly > created (time-stamped) directory as processing progresses. > > My typical usage scenario is capturing a long-running live video feed > (perhaps time-lapsed) and storing the resulting images in a time-stamped > directory hierarchy fashion, rather than as a numbered sequence of files in > a single directory. > > If you look at the code you will see that only a half dozen lines of code > were required in img2enc. The function for creating directories already > existed in hlsenc.c but I've moved into utils.c as I presumed that was a > more generic location for it. > > All comments appreciated. > > Thanks ad Regards, > Alan. > > > On 17/12/17 22:46, Carl Eugen Hoyos wrote: > >2017-12-17 23:41 GMT+01:00 Dr Alan Barclay : > > > >>Please would someone with an interest in img2enc take a look > >>at my minor feature addition and consider committing it to the > >>main line for me. > >To be acceptable, the patch has to be split in two and please > >move the definition into internal.h > > > >Carl Eugen > >___ > >ffmpeg-devel mailing list > >ffmpeg-devel@ffmpeg.org > >http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > hlsenc.c | 35 +-- > internal.h |7 +++ > utils.c| 33 + > 3 files changed, 41 insertions(+), 34 deletions(-) > 9560fd03958f79f77b01c0e02c55d98e3dc7b937 > 0001-Move-mkdir_p-renamed-ff_mkdir_p-from-hlsenc.c-to-uti.patch > --- > libavformat/hlsenc.c | 35 +-- > libavformat/internal.h | 7 +++ > libavformat/utils.c| 33 + > 3 files changed, 41 insertions(+), 34 deletions(-) these patches are missing commit messages patches should be genrated with git format-patch or git send-email [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Elect your leaders based on what they did after the last election, not based on what they say before an election. signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] Adding mkdir option for img2enc (2nd attempt)
Hi All, Please would someone with an interest in img2enc take a look at my revised patches for a minor feature addition and consider committing it to the main line for me. Example: ffmpeg -i ~/trailer.mp4 -strftime 1 -mkdir 1 %Y/%m/%d/out_%H-%M-%S.jpg Without the new mkdir option, this command will fail if the directory hierarchy for the jpg files does not already exist, which can be difficult to predict for time-stamped directories. This patch adds a mkdir option to img2enc which invites it to make whatever directory hierarchy is necessary for each output file. When used in conjunction with the strftime then the jpg files will be located in a newly created (time-stamped) directory as processing progresses. My typical usage scenario is capturing a long-running live video feed (perhaps time-lapsed) and storing the resulting images in a time-stamped directory hierarchy fashion, rather than as a numbered sequence of files in a single directory. If you look at the code you will see that only a half dozen lines of code were required in img2enc. The function for creating directories already existed in hlsenc.c but I've moved into utils.c as I presumed that was a more generic location for it. All comments appreciated. Thanks ad Regards, Alan. On 17/12/17 22:46, Carl Eugen Hoyos wrote: 2017-12-17 23:41 GMT+01:00 Dr Alan Barclay : Please would someone with an interest in img2enc take a look at my minor feature addition and consider committing it to the main line for me. To be acceptable, the patch has to be split in two and please move the definition into internal.h Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel --- libavformat/hlsenc.c | 35 +-- libavformat/internal.h | 7 +++ libavformat/utils.c| 33 + 3 files changed, 41 insertions(+), 34 deletions(-) diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c index bbc2742dc7..633ddc309e 100644 --- a/libavformat/hlsenc.c +++ b/libavformat/hlsenc.c @@ -208,39 +208,6 @@ typedef struct HLSContext { AVIOContext *sub_m3u8_out; } HLSContext; -static int mkdir_p(const char *path) { -int ret = 0; -char *temp = av_strdup(path); -char *pos = temp; -char tmp_ch = '\0'; - -if (!path || !temp) { -return -1; -} - -if (!strncmp(temp, "/", 1) || !strncmp(temp, "\\", 1)) { -pos++; -} else if (!strncmp(temp, "./", 2) || !strncmp(temp, ".\\", 2)) { -pos += 2; -} - -for ( ; *pos != '\0'; ++pos) { -if (*pos == '/' || *pos == '\\') { -tmp_ch = *pos; -*pos = '\0'; -ret = mkdir(temp, 0755); -*pos = tmp_ch; -} -} - -if ((*(pos - 1) != '/') || (*(pos - 1) != '\\')) { -ret = mkdir(temp, 0755); -} - -av_free(temp); -return ret; -} - static int is_http_proto(char *filename) { const char *proto = avio_find_protocol_name(filename); return proto ? (!av_strcasecmp(proto, "http") || !av_strcasecmp(proto, "https")) : 0; @@ -1407,7 +1374,7 @@ static int hls_start(AVFormatContext *s, VariantStream *vs) return AVERROR(ENOMEM); } dir = av_dirname(fn_copy); -if (mkdir_p(dir) == -1 && errno != EEXIST) { +if (ff_mkdir_p(dir) == -1 && errno != EEXIST) { av_log(oc, AV_LOG_ERROR, "Could not create directory %s with use_localtime_mkdir\n", dir); av_free(fn_copy); return AVERROR(errno); diff --git a/libavformat/internal.h b/libavformat/internal.h index e76ac12371..f471019a45 100644 --- a/libavformat/internal.h +++ b/libavformat/internal.h @@ -551,6 +551,13 @@ static inline int ff_rename(const char *oldpath, const char *newpath, void *logc } /** + * Make the specified directory. + * + * @param path path for directory + */ +int ff_mkdir_p(const char *path); + +/** * Allocate extradata with additional AV_INPUT_BUFFER_PADDING_SIZE at end * which is always set to 0. * diff --git a/libavformat/utils.c b/libavformat/utils.c index 84e49208b8..aac58010b2 100644 --- a/libavformat/utils.c +++ b/libavformat/utils.c @@ -5606,3 +5606,36 @@ FF_ENABLE_DEPRECATION_WARNINGS return st->internal->avctx->time_base; #endif } + +int ff_mkdir_p(const char *path) { +int ret = 0; +char *temp = av_strdup(path); +char *pos = temp; +char tmp_ch = '\0'; + +if (!path || !temp) { +return -1; +} + +if (!strncmp(temp, "/", 1) || !strncmp(temp, "\\", 1)) { +pos++; +} else if (!strncmp(temp, "./", 2) || !strncmp(temp, ".\\", 2)) { +pos += 2; +} + +for ( ; *pos != '\0'; ++pos) { +if (*pos == '/' || *pos == '\\') { +tmp_ch = *pos; +*pos = '\0'; +ret = mkdir(temp, 0755); +*po