[FFmpeg-devel] [PATCH] Adding mkdir option for img2enc (2nd attempt)

Derek Buitenhuis derek.buitenhuis at gmail.com
Wed Dec 27 18:01:04 EET 2017


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



More information about the ffmpeg-devel mailing list