[FFmpeg-devel] [PATCH 3/4] concatdec: add support for specifying outpoint of files

[Marton Balint]

On Mon, 6 Jul 2015, Nicolas George wrote:

> L'octidi 18 messidor, an CCXXIII, Marton Balint a écrit :
>> Signed-off-by: Marton Balint <cus at passwd.hu>
>> ---
>>  doc/demuxers.texi       | 13 +++++++++++++
>>  libavformat/concatdec.c | 22 ++++++++++++++++++++--
>>  2 files changed, 33 insertions(+), 2 deletions(-)
>>
>> diff --git a/doc/demuxers.texi b/doc/demuxers.texi
>> index 4bad1c8..4ba797e 100644
>> --- a/doc/demuxers.texi
>> +++ b/doc/demuxers.texi
>> @@ -129,6 +129,19 @@ directive) will be reduced based on their specified In point.
>>  Because of potential packets before the specified In point, packet timestamps
>>  may overlap between two concatenated files.
>>
>> + at item @code{out @var{timestamp}}
>> +Out point of the file. When the demuxer reaches the specified timestamp in any
>> +of the streams, it handles it as an end of file condition. Out point is
>> +exclusive, which means that the demuxer will not output packets which has a
>> +timestamp greater or equal to Out point.
>> +
>> +This directive works best with intra frame codecs, because for non-intra frame
>> +ones you will usually not get enough packets to decode the last few frames
>> +before the specified Out point.
>> +
>> +The duration of the files (if not specified by the @code{duration}
>> +directive) will be reduced based on their specified Out point.
>> +
>>  @item @code{stream}
>>  Introduce a stream in the virtual file.
>>  All subsequent stream-related directives apply to the last introduced
>> diff --git a/libavformat/concatdec.c b/libavformat/concatdec.c
>> index 11e6759..eaf34b0 100644
>> --- a/libavformat/concatdec.c
>> +++ b/libavformat/concatdec.c
>> @@ -44,6 +44,7 @@ typedef struct {
>>      int64_t duration;
>>      ConcatStream *streams;
>>      int64_t inpoint;
>> +    int64_t outpoint;
>>      int nb_streams;
>>  } ConcatFile;
>>
>> @@ -145,6 +146,7 @@ static int add_file(AVFormatContext *avf, char *filename, ConcatFile **rfile,
>>      file->start_time = AV_NOPTS_VALUE;
>>      file->duration   = AV_NOPTS_VALUE;
>>      file->inpoint    = AV_NOPTS_VALUE;
>> +    file->outpoint   = AV_NOPTS_VALUE;
>>
>>      return 0;
>>
>> @@ -360,7 +362,7 @@ static int concat_read_header(AVFormatContext *avf)
>>              }
>>              if ((ret = add_file(avf, filename, &file, &nb_files_alloc)) < 0)
>>                  goto fail;
>> -        } else if (!strcmp(keyword, "duration") || !strcmp(keyword, "in")) {
>> +        } else if (!strcmp(keyword, "duration") || !strcmp(keyword, "in") || !strcmp(keyword, "out")) {
>>              char *dur_str = get_keyword(&cursor);
>>              int64_t dur;
>>              if (!file) {
>> @@ -377,6 +379,8 @@ static int concat_read_header(AVFormatContext *avf)
>>                  file->duration = dur;
>>              else if (!strcmp(keyword, "in"))
>>                  file->inpoint = dur;
>> +            else if (!strcmp(keyword, "out"))
>> +                file->outpoint = dur;
>>          } else if (!strcmp(keyword, "stream")) {
>>              if (!avformat_new_stream(avf, NULL))
>>                  FAIL(AVERROR(ENOMEM));
>> @@ -443,6 +447,8 @@ static int open_next_file(AVFormatContext *avf)
>>          cat->cur_file->duration = cat->avf->duration;
>>          if (cat->cur_file->inpoint != AV_NOPTS_VALUE)
>>              cat->cur_file->duration -= (cat->cur_file->inpoint - file_start_time);
>> +        if (cat->cur_file->outpoint != AV_NOPTS_VALUE)
>> +            cat->cur_file->duration -= cat->avf->duration - (cat->cur_file->outpoint - file_start_time);
>>      }
>>
>>      if (++fileno >= cat->nb_files) {
>> @@ -504,6 +510,16 @@ static int64_t get_cur_file_inpoint(ConcatContext *cat)
>>      return file_inpoint;
>>  }
>>
>> +/* Returns true if the packet pts is greater or equal to the specified outpoint. */
>> +static int packet_after_outpoint(ConcatContext *cat, AVPacket *pkt)
>> +{
>> +    if (cat->cur_file->outpoint != AV_NOPTS_VALUE && pkt->pts != AV_NOPTS_VALUE) {
>> +        return av_compare_ts(pkt->pts, cat->avf->streams[pkt->stream_index]->time_base,
>> +                             cat->cur_file->outpoint, AV_TIME_BASE_Q) >= 0;
>> +    }
>> +    return 0;
>> +}
>> +
>>  static int concat_read_packet(AVFormatContext *avf, AVPacket *pkt)
>>  {
>>      ConcatContext *cat = avf->priv_data;
>> @@ -520,7 +536,9 @@ static int concat_read_packet(AVFormatContext *avf, AVPacket *pkt)
>>
>>      while (1) {
>>          ret = av_read_frame(cat->avf, pkt);
>> -        if (ret == AVERROR_EOF) {
>
>> +        if (ret == AVERROR_EOF || packet_after_outpoint(cat, pkt)) {
>> +            if (ret == 0)
>> +                av_packet_unref(pkt);
>
> This will not work if the streams in the file are not exactly interleaved.
> Some formats store subtitles, and even audio, a few frames early, that will
> trigger EOF even though there may be frames below the outpoint still coming.

Yeah, i know. Actually I don't see how would it be possible to give a 
stronger kind of guarantee efficiently and reliably to the user. That is 
why I sticked to the simplest-to-implement method. For the moment - if the 
user will use this directive to cut parts in an interleaved video, he 
should know his interleaving delays (and his GOP sizes :))

> At least, the documentation should have a warning about it.

Okay, I will add one.

>
>>              if ((ret = open_next_file(avf)) < 0)
>>                  return ret;
>>              continue;
>
> No other objection, but if you change the name of the inpoint option, maybe
> outpoint is no longer the best choice.

"eof <timestamp>" came to my mind, if you still insist on changing these 
names :). But let me know if you have something in mind you prefer.

Regards,
Marton