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

[Stefano Sabatini]

On date Tuesday 2013-09-17 14:22:17 +0200, Clément Bœsch encoded:
> 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?

It mark a group, in this case (A|B) means A or B.

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

Changed and itemized.

> 
> > +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).

I see that new codebase is mostly using avformat_seek_file(), any
reason why av_seek_frame() is not deprecated?

Also:
int avformat_seek_file(AVFormatContext *s, int stream_index, int64_t min_ts, int64_t ts, int64_t max_ts, int flags);

Thinking about some way to express the other parameters (min, max,
reference streams, flags). The flags could be set in a separate global
variable.
What about:
1:02 at 5
to mean +-5 seconds?

It can be probably extended later.

[...]
> > +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=""

In that case it is considered like a single empty specification, and
will cause a parsing error.

> 
> [...]
> 
> No other comment from me so far.
-- 
FFmpeg = Freak and Furious Muttering Powerful Evil Game
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-ffprobe-add-read_intervals-option.patch
Type: text/x-diff
Size: 11981 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130917/8a6f9676/attachment.bin>