[FFmpeg-devel] [RFC] image2 and filename patterns
Alexander Strasser
eclipse7 at gmx.net
Sun Jan 27 15:59:07 CET 2013
Hi Stefano,
a few comments on the patches follow below.
Stefano Sabatini wrote:
> On date Thursday 2013-01-24 15:59:26 +0100, Stefano Sabatini encoded:
> > I'm trying to address ticket #1969 and I'm collecting a few ideas and
> > seeing some (minor) nuisances.
> >
> > My idea is to implement something like this:
> > ffmpeg INPUT -f image2 -pattern_type sequence_ts foo-%06d.png
> >
> > which would force to encode the timestamps in microseconds (or
> > selectable) in the output files.
> >
> > -pattern_type would be based on the image2 demuxer homonymous option,
> > values could be for example:
> > sequence
> > sequence_ts
> >
> > Currently file overwrite is implemented through the not very intuitive
> > option named 'updatefirst', a possibly cleaner solution would be to
> > adopt another pattern like '-pattern_type file' (which would also
> > avoid pattern escaping on the user side).
> >
> > The 'file' and 'sequence_ts' could be ported to the image2 demuxer
> > (the first would avoid the need for a -loop option at the application
> > level, for example in ffmpeg or in the movie filter).
> >
> > In the mentioned ticket a pattern of the form '%pts' was suggested,
> > but I'm opposed to this for various reasons (need to convert %t
> > sequence to an av_get_frame_filename() pattern, impredictability in
> > case of arbitrary filenames provided by the user).
>
> And patches...
> --
> FFmpeg = Formidable and Free Mind-dumbing Patchable Egregious Gospel
> >From 5ddaad7f865d6bf1db9bf4e1d0c03ab175d80e29 Mon Sep 17 00:00:00 2001
> From: Stefano Sabatini <stefasab at gmail.com>
> Date: Thu, 24 Jan 2013 17:15:36 +0100
> Subject: [PATCH] lavf/img2enc: introduce -pattern_type option
>
> This option allows to specify cleanly how to interpret the output
> filename pattern, and is required by the incoming sequence_ts change.
>
> start_number range is extended from [1, max] to [-1, max], so that -1 can
> be interpreted as "undefined" and cause a configuration failure in case
> start_number is specified with a non-sequence pattern type.
>
> The only notable interface change is that -updatefirst 1 OUTPUT will not
> expand OUTPUT, which looks like a good idea.
>
> TODO: bump micro
> ---
> doc/muxers.texi | 51 ++++++++++++++++++++++++++++++++++---------------
> libavformat/img2enc.c | 40 ++++++++++++++++++++++++++++++++------
> 2 files changed, 70 insertions(+), 21 deletions(-)
>
> diff --git a/doc/muxers.texi b/doc/muxers.texi
> index 965a4bb..3ae6543 100644
> --- a/doc/muxers.texi
> +++ b/doc/muxers.texi
> @@ -196,7 +196,33 @@ Image file muxer.
> The image file muxer writes video frames to image files.
>
> The output filenames are specified by a pattern, which can be used to
> -produce sequentially numbered series of files.
> +produce sequentially numbered series of files. The syntax and meaning
> +of the pattern is specified by the option @var{pattern_type}.
> +
> +The pattern may contain a suffix which is used to automatically
> +determine the format of the image files to write.
> +
> +This demuxer accepts the following options:
> + at table @option
> + at item start_number @var{number}
> +Start the sequence from @var{number}. Default value is 1. Must be a
> +positive number. This option can be specified only if
> + at option{pattern_type} is set to @code{sequence}.
> +
> + at item updatefirst 1|0
> +If set to 1, update the first written image file again and
> +again. Default value is 0. This option is deprecated in favor of
> + at option{pattern_type} @code{file}.
> +
> + at item pattern_type
> +Select the pattern type used to interpret the provided filename.
> +
> + at var{pattern_type} accepts one of the following values.
> + at table @option
> + at item sequence
> +Select a sequence pattern type, used to specify a sequence of files
> +indexed by sequential numbers.
> +
> The pattern may contain the string "%d" or "%0 at var{N}d", this string
> specifies the position of the characters representing a numbering in
> the filenames. If the form "%0 at var{N}d" is used, the string
> @@ -205,11 +231,16 @@ digits. The literal character '%' can be specified in the pattern with
> the string "%%".
>
> If the pattern contains "%d" or "%0 at var{N}d", the first filename of
> -the file list specified will contain the number 1, all the following
> -numbers will be sequential.
> +the file list specified will contain the number specified by
> + at option{start_number}, all the following numbers will be sequential.
>
> -The pattern may contain a suffix which is used to automatically
> -determine the format of the image files to write.
> + at item file
> +Select a single file output. The frames will be written on the same
> +file.
Maybe "literal" would be a better name for the pattern.
So if am not misunderstanding the code something like that:
@item literal
Not really a pattern; will achieve that the specified value will be
used literally. The sinlge resulting file will be overwritten on every
output picture.
> + at end table
> +
> +Default value is @var{sequence}.
> + at end table
>
> For example the pattern "img-%03d.bmp" will specify a sequence of
> filenames of the form @file{img-001.bmp}, @file{img-002.bmp}, ...,
> @@ -240,16 +271,6 @@ Note also that the pattern must not necessarily contain "%d" or
> ffmpeg -i in.avi -f image2 -frames:v 1 img.jpeg
> @end example
>
> - at table @option
> - at item start_number @var{number}
> -Start the sequence from @var{number}. Default value is 1. Must be a
> -positive number.
> -
> - at item updatefirst 1|0
> -If set to 1, update the first written image file again and
> -again. Default value is 0.
> - at end table
> -
> The image muxer supports the .Y.U.V image file format. This format is
> special in that that each image frame consists of three files, for
> each of the YUV420P components. To read or write this image file format,
> diff --git a/libavformat/img2enc.c b/libavformat/img2enc.c
> index 67b5819..f906492 100644
> --- a/libavformat/img2enc.c
> +++ b/libavformat/img2enc.c
> @@ -36,6 +36,8 @@ typedef struct {
> int is_pipe;
> int split_planes; /**< use independent file for each Y, U, V plane */
> char path[1024];
> + enum { PT_SEQUENCE, PT_FILE } pattern_type;
This and following affected parts of the code should be adopted if we
decide to rename PT_FILE .
> + int start_number;
> int updatefirst;
> } VideoMuxData;
>
> @@ -54,6 +56,18 @@ static int write_header(AVFormatContext *s)
> else
> img->is_pipe = 1;
>
> + if (img->start_number >= 0 && img->pattern_type != PT_SEQUENCE) {
> + av_log(s, AV_LOG_ERROR, "start_number option cannot be set if pattern_type is not 'sequence'\n");
> + return AVERROR(EINVAL);
> + } else {
> + img->img_number = img->start_number < 0 ? 1 : img->start_number;
> + }
> +
> + if (img->updatefirst) {
> + av_log(s, AV_LOG_WARNING, "updatefirst option is deprecated, forcing pattern_type file\n");
> + img->pattern_type = PT_FILE;
> + }
> +
> str = strrchr(img->path, '.');
> img->split_planes = str
> && !av_strcasecmp(str + 1, "y")
> @@ -75,13 +89,22 @@ static int write_packet(AVFormatContext *s, AVPacket *pkt)
> int i;
>
> if (!img->is_pipe) {
> - if (av_get_frame_filename(filename, sizeof(filename),
> - img->path, img->img_number) < 0 && img->img_number > 1 && !img->updatefirst) {
> - av_log(s, AV_LOG_ERROR,
> - "Could not get frame filename number %d from pattern '%s' (either set updatefirst or use a pattern like %%03d within the filename pattern)\n",
> + switch (img->pattern_type) {
> + case PT_SEQUENCE:
> + if (av_get_frame_filename(filename, sizeof(filename), img->path, img->img_number) < 0 &&
> + img->img_number > img->start_number) {
> + av_log(s, AV_LOG_ERROR,
> + "Could not get frame filename number %d from pattern '%s' "
> + "(either set pattern_type option to 'file' or use a sequence pattern like %%03d within the filename pattern)\n",
> img->img_number, img->path);
> - return AVERROR(EINVAL);
> + return AVERROR(EINVAL);
> + }
> + break;
> + case PT_FILE:
> + av_strlcpy(filename, s->filename, sizeof(filename));
> + break;
> }
> +
> for (i = 0; i < 4; i++) {
> if (avio_open2(&pb[i], filename, AVIO_FLAG_WRITE,
> &s->interrupt_callback, NULL) < 0) {
> @@ -129,7 +152,12 @@ static int write_packet(AVFormatContext *s, AVPacket *pkt)
> #define ENC AV_OPT_FLAG_ENCODING_PARAM
> static const AVOption muxoptions[] = {
> { "updatefirst", "update the first image file", OFFSET(updatefirst), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 1, ENC },
> - { "start_number", "set first number in the sequence", OFFSET(img_number), AV_OPT_TYPE_INT, { .i64 = 1 }, 1, INT_MAX, ENC },
> + { "start_number", "set first number in the sequence", OFFSET(start_number), AV_OPT_TYPE_INT, { .i64 =-1 }, -1, INT_MAX, ENC },
> +
> + { "pattern_type", "set pattern type", OFFSET(pattern_type), AV_OPT_TYPE_INT, {.i64=PT_SEQUENCE}, 0, INT_MAX, ENC, "pattern_type"},
> + { "sequence", "select number sequence pattern type", 0, AV_OPT_TYPE_CONST, {.i64=PT_SEQUENCE}, .flags=ENC, .unit="pattern_type" },
> + { "file", "select single file pattern type", 0, AV_OPT_TYPE_CONST, {.i64=PT_FILE}, .flags=ENC, .unit="pattern_type" },
> +
> { NULL },
> };
>
> --
> 1.7.9.5
>
> >From 29926806ceedc65e57009fd73773076e848a5577 Mon Sep 17 00:00:00 2001
> From: Stefano Sabatini <stefasab at gmail.com>
> Date: Thu, 24 Jan 2013 17:36:09 +0100
> Subject: [PATCH] lavf/img2end: introduce -pattern_type sequence_ts
>
> Should address ticket #1969.
>
> TODO: update docs, bump micro
> ---
> libavformat/img2enc.c | 17 ++++++++++++++++-
> 1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/libavformat/img2enc.c b/libavformat/img2enc.c
> index f906492..72bd5d3 100644
> --- a/libavformat/img2enc.c
> +++ b/libavformat/img2enc.c
> @@ -36,7 +36,7 @@ typedef struct {
> int is_pipe;
> int split_planes; /**< use independent file for each Y, U, V plane */
> char path[1024];
> - enum { PT_SEQUENCE, PT_FILE } pattern_type;
> + enum { PT_SEQUENCE, PT_SEQUENCE_TS, PT_FILE } pattern_type;
> int start_number;
> int updatefirst;
> } VideoMuxData;
> @@ -100,6 +100,20 @@ static int write_packet(AVFormatContext *s, AVPacket *pkt)
> return AVERROR(EINVAL);
> }
> break;
> + case PT_SEQUENCE_TS:
> + {
> + AVStream *st = s->streams[pkt->stream_index];
> + int64_t pts = av_rescale_q(pkt->pts, st->time_base, AV_TIME_BASE_Q);
> + if (av_get_frame_filename(filename, sizeof(filename), img->path, pts) < 0 &&
Will this work out with int on all platforms. The last parameter is
declared int AFAIK.
> + img->img_number > img->start_number) {
> + av_log(s, AV_LOG_ERROR,
> + "Could not get frame filename number %"PRId64" from pattern '%s' of type sequence_ts",
> + pts, img->path);
> + return AVERROR(EINVAL);
> + }
> + }
> + break;
> +
> case PT_FILE:
> av_strlcpy(filename, s->filename, sizeof(filename));
> break;
> @@ -156,6 +170,7 @@ static const AVOption muxoptions[] = {
>
> { "pattern_type", "set pattern type", OFFSET(pattern_type), AV_OPT_TYPE_INT, {.i64=PT_SEQUENCE}, 0, INT_MAX, ENC, "pattern_type"},
> { "sequence", "select number sequence pattern type", 0, AV_OPT_TYPE_CONST, {.i64=PT_SEQUENCE}, .flags=ENC, .unit="pattern_type" },
> + { "sequence_ts", "select timestamp sequence pattern type", 0, AV_OPT_TYPE_CONST, {.i64=PT_SEQUENCE_TS}, .flags=ENC, .unit="pattern_type" },
> { "file", "select single file pattern type", 0, AV_OPT_TYPE_CONST, {.i64=PT_FILE}, .flags=ENC, .unit="pattern_type" },
>
> { NULL },
> --
> 1.7.9.5
I would say I am in favor of these patches. Just the PT_FILE seems a bit
weird to me.
Alexander
More information about the ffmpeg-devel
mailing list