[FFmpeg-devel] [PATCH] ffprobe: add -ss option

[Clément Bœsch]

On Mon, Sep 16, 2013 at 06:44:52PM +0200, Stefano Sabatini wrote:
> On date Monday 2013-09-16 18:09:38 +0200, Stefano Sabatini encoded:
> > On date Sunday 2013-09-15 20:15:08 +0200, Stefano Sabatini encoded:
> [...]
> > Updated, #N duration specification not yet implemented (I'll probably
> > do in a further patch). The new syntax is more flexible but the result
> > seems a bit overkill (+189 lines of code).
> 
> Updated with that change. I'll ruminate upon the syntax a few days and
> push if nobody wants to review or proposes some changes (also
> documentation review is welcome).
> -- 
> FFmpeg = Frenzy and Frenzy Muttering Practical Ecstatic God

> From fbee237bfbf257a923b573542c5edcff19f3b68b Mon Sep 17 00:00:00 2001
> From: Stefano Sabatini <stefasab at gmail.com>
> Date: Mon, 22 Oct 2012 16:01:29 +0200
> Subject: [PATCH] ffprobe: add read_intervals option
> 
> This is also useful to test seeking on an input file.
> 
> TODO: add Changelog entry
> 
> This also address trac ticket #1437.
> ---
>  doc/ffprobe.texi |  53 ++++++++++++++
>  ffprobe.c        | 212 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 263 insertions(+), 2 deletions(-)
> 
> diff --git a/doc/ffprobe.texi b/doc/ffprobe.texi
> index 20f5f4a..f6980a9 100644
> --- a/doc/ffprobe.texi
> +++ b/doc/ffprobe.texi
> @@ -230,6 +230,59 @@ corresponding stream section.
>  Count the number of packets per stream and report it in the
>  corresponding stream section.
>  
> + at item -read_intervals @var{read_intervals}
> +
> +Read only the specified intervals. @var{read_intervals} must be a
> +sequence of intervals separated by ",".
> +

We can't tell from this if ffprobe is going to seek or just ignore out of
the range. Also, is there any accuracy/behaviour differences between
packets demuxing and frame decoding?

> +Each interval is specified as a time duration specification specifying
> +the interval start (the point to which to seek). If the start seeking
> +point is not specified, no seeking will be performed when reading the
> +corresponding interval.
> +

> +The end seeking point or duration is specified after the optional

"seeking end point"?

> +start point by the "+" or "%" character. In case the "+" character is
> +specified, the leading string is interpreted as the duration of the
> +interval, otherwise as the interval ending position. If the duration
> +specification starts with "#", it is interpreted as a number of
> +packets to read (not including the flushing packets). Otherwise
> +duration or seeking point are parsed as time duration specifications.
> +
> +If no duration or ending point is specified, the program will read
> +until the end of the input.
> +
> +The formal syntax is given by:
> + at example

> + at var{INTERVAL}  ::= [@var{START}]([+ at var{DURATION}|%@var{END}])

Sorry, I'm not comfortable with the BNF; what are the ( ) for?

> + at var{INTERVALS} ::= @var{INTERVAL}[, at var{INTERVALS}]
> + at end example
> +

> +For example, the following specification:

"specification" -> "@var{read_intervals}" maybe?

> + at example
> +10+20,01:30%01:45
> + at end example
> +
> +will make ffprobe seek to time 10, read packets in the interval

@command{ffprobe}

> +10-10+20,

"10 to 20 seconds" maybe?

>            then read packets from position 01:30 (1 minute and thirty
> +seconds) to position 01:45.
> +

Maybe use some @code{} for the timestamps?

> +To read only 42 frames starting from position 01:23, the following
> +syntax can be used:
> + at example
> +01:23+#42
> + at end example
> +
> +To read just the first 20 seconds, you can use the syntax:
> + at example
> ++20
> + at end example
> +
> +To read until position 02:30 since the input beginning, you can use
> +the syntax:
> + at example
> +%02:30
> + at end example
> +
>  @item -show_private_data, -private
>  Show private data, that is data depending on the format of the
>  particular shown element.
> diff --git a/ffprobe.c b/ffprobe.c
> index 17ec7e2..4ad88a4 100644
> --- a/ffprobe.c
> +++ b/ffprobe.c
> @@ -37,7 +37,9 @@
>  #include "libavutil/pixdesc.h"
>  #include "libavutil/dict.h"
>  #include "libavutil/libm.h"
> +#include "libavutil/parseutils.h"
>  #include "libavutil/timecode.h"
> +#include "libavutil/timestamp.h"
>  #include "libavdevice/avdevice.h"
>  #include "libswscale/swscale.h"
>  #include "libswresample/swresample.h"
> @@ -73,6 +75,16 @@ static int show_private_data            = 1;
>  static char *print_format;
>  static char *stream_specifier;
>  
> +typedef struct {

> +    int64_t start, end, duration; ///< start, end, duration time in microseconds

in AV_TIME_BASE?

> +    int has_start, has_end, has_duration;
> +    int id;                       ///< identifier
> +    int duration_frames;
> +} ReadInterval;
> +
> +static ReadInterval *read_intervals;
> +static int read_intervals_nb = 0;
> +
>  /* section structure definition */
>  
>  #define SECTION_MAX_NB_CHILDREN 10
> @@ -1593,16 +1605,65 @@ static av_always_inline int process_frame(WriterContext *w,
>      return got_frame;
>  }
>  
> -static void read_packets(WriterContext *w, AVFormatContext *fmt_ctx)
> +static void log_read_interval(const ReadInterval *interval, void *log_ctx, int log_level)
> +{
> +    av_log(log_ctx, log_level,
> +           "id:%d start:%s end:%s ", interval->id,
> +           interval->has_start    ? av_ts2timestr(interval->start,    &AV_TIME_BASE_Q) : "N/A",
> +           interval->has_end      ? av_ts2timestr(interval->end,      &AV_TIME_BASE_Q) : "N/A");
> +    if (interval->has_duration && interval->duration_frames)
> +        av_log(log_ctx, log_level, "duration:#%"PRId64"\n", interval->duration);
> +    else
> +        av_log(log_ctx, log_level, "duration:%s\n",
> +               interval->has_duration ? av_ts2timestr(interval->duration, &AV_TIME_BASE_Q) : "N/A");
> +}
> +
> +static int read_interval_packets(WriterContext *w, AVFormatContext *fmt_ctx,
> +                                 ReadInterval *interval)
>  {
>      AVPacket pkt, pkt1;
>      AVFrame frame;
> -    int i = 0;
> +    int ret = 0, i = 0, frame_count = 0;
>  
>      av_init_packet(&pkt);
>  
> +    av_log(NULL, AV_LOG_VERBOSE, "Processing read interval ");
> +    log_read_interval(interval, NULL, AV_LOG_VERBOSE);
> +
> +    if (interval->has_start) {
> +        av_log(NULL, AV_LOG_VERBOSE, "Seeking to read interval start point %s\n",
> +               av_ts2timestr(interval->start, &AV_TIME_BASE_Q));

> +        if ((ret = av_seek_frame(fmt_ctx, -1, interval->start, AVSEEK_FLAG_BACKWARD)) < 0) {
> +            av_log(NULL, AV_LOG_ERROR, "Could not seek to position %"PRId64"\n",
> +                   interval->start);
> +            return ret;

You should use the seeking API v2.

Also, there is a frame seek flag IIRC (not sure if it's really implemented
for most formats, but that will be a good way to test it).

> +        }
> +    }
> +
>      while (!av_read_frame(fmt_ctx, &pkt)) {
>          if (selected_streams[pkt.stream_index]) {
> +            AVRational tb;
> +
> +            if (!interval->has_start &&
> +                pkt.pts != AV_NOPTS_VALUE) {
> +                interval->start = pkt.pts;
> +                interval->has_start = 1;
> +
> +                if (!interval->has_end) {
> +                    interval->end = interval->start + interval->duration;
> +                    interval->has_end = 1;
> +                }
> +            }
> +
> +            tb = fmt_ctx->streams[pkt.stream_index]->time_base;
> +            if (interval->has_duration && interval->duration_frames) {
> +                if (frame_count >= interval->duration)
> +                    break;
> +            } else if (pkt.pts != AV_NOPTS_VALUE &&
> +                       av_rescale_q(pkt.pts, tb, AV_TIME_BASE_Q) >= interval->end)
> +                break;
> +
> +            frame_count++;
>              if (do_read_packets) {
>                  if (do_show_packets)
>                      show_packet(w, fmt_ctx, &pkt, i++);
[...]
> +static int parse_read_intervals(const char *intervals_spec)
> +{
> +    int ret, n, i;
> +    char *p, *spec = av_strdup(intervals_spec);
> +    if (!spec)
> +        return AVERROR(ENOMEM);
> +
> +    /* preparse specification, get number of intervals */
> +    for (n = 0, p = spec; *p; p++)
> +        if (*p == ',')
> +            n++;
> +    n++;

looks risky if spec=""

[...]

No other comment from me so far.

-- 
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130917/2661c385/attachment.asc>