[FFmpeg-devel] [PATCH] avformat/matroskaenc: Write duration early during mkv_write_header

[James Almer]

On 7/18/2016 9:37 PM, Rostislav Pehlivanov wrote:
> On 18 July 2016 at 01:34, Soft Works <softworkz at hotmail.com> wrote:
> 
>> This commit addresses the following scenario:
>>
>> we are using ffmpeg to transcode or remux mkv (or something else) to mkv.
>> The result is being streamed on-the-fly to an HTML5 client (streaming
>> starts while ffmpeg is still running). The problem here is that the client
>> is unable to detect the duration because the duration is only written to
>> the mkv at the end of the transcoding/remoxing process. In matroskaenc.c,
>> the duration is only written during mkv_write_trailer but not during
>> mkv_write_header.
>>
>> The approach:
>>
>> FFMPEG is currently putting quite some effort to estimate the durations of
>> source streams, but in many cases the source stream durations are still
>> left at 0 and these durations are nowhere mapped to or used for output
>> streams. As much as I would have liked to deduct or estimate output
>> durations based on input stream durations - I realized that this is a hard
>> task (as Nicolas already mentioned in a previous conversation). It would
>> involve changes to the duration calculation/estimation/deduction for input
>> streams and propagating these durations to output streams or the output
>> context in a correct way.
>> So I looked for a simple and small solution with better chances to get
>> accepted. In webmdashenc.c I found that a duration is written during
>> write_header and this duration is taken from the streams' metadata, so I
>> decided for a similar approach.
>>
>> And here's what it does:
>>
>> At first it is checking the duration of the AVFormatContext. In typical
>> cases this value is not set, but: It is set in cases where the user has
>> specified a recording_time or an end_time via the -t or -to parameters.
>> Then it is looking for a DURATION metadata field in the metadata of the
>> output context (AVFormatContext::metadata). This would only exist in case
>> the user has explicitly specified a metadata DURATION value from the
>> command line.
>> Then it is iterating all streams looking for a "DURATION" metadata (this
>> works unless the option "-map_metadata -1" has been specified) and
>> determines the maximum value.
>> The precendence is as follows: 1. Use duration of AVFormatContext - 2. Use
>> explicitly specified metadata duration value - 3. Use maximum (mapped)
>> metadata duration over all streams.
>>
>> To test this:
>>
>> 1. With explicit recording time:
>> ffmpeg -i file:"src.mkv" -loglevel debug -t 01:38:36.000 -y "dest.mkv"
>>
>> 2. Take duration from metadata specified via command line parameters:
>> ffmpeg -i file:"src.mkv" -loglevel debug -map_metadata -1 -metadata
>> Duration="01:14:33.00" -y "dest.mkv"
>>
>> 3. Take duration from mapped input metadata:
>> ffmpeg -i file:"src.mkv" -loglevel debug -y "dest.mkv"
>>
>> Regression risk:
>>
>> Very low IMO because it only affects the header while ffmpeg is still
>> running. When ffmpeg completes the process, the duration is rewritten to
>> the header with the usual value (same like without this commit).
>>
>> Signed-off-by: SoftWorkz <softworkz at hotmail.com>
>> ---
>>  libavformat/matroskaenc.c | 39 +++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 39 insertions(+)
>>
>> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
>> index 53353bd..75ee9fb 100644
>> --- a/libavformat/matroskaenc.c
>> +++ b/libavformat/matroskaenc.c
>> @@ -44,6 +44,7 @@
>>  #include "libavutil/mastering_display_metadata.h"
>>  #include "libavutil/mathematics.h"
>>  #include "libavutil/opt.h"
>> +#include "libavutil/parseutils.h"
>>  #include "libavutil/random_seed.h"
>>  #include "libavutil/rational.h"
>>  #include "libavutil/samplefmt.h"
>> @@ -1487,6 +1488,30 @@ static int mkv_write_attachments(AVFormatContext *s)
>>      return 0;
>>  }
>>
>> +static int64_t get_metadata_duration(AVFormatContext *s)
>> +{
>> +    int i = 0;
>> +    int64_t max = 0;
>> +    int64_t us;
>> +
>> +    AVDictionaryEntry *explicitDuration = av_dict_get(s->metadata,
>> "DURATION", NULL, 0);
>> +    if (explicitDuration && (av_parse_time(&us, explicitDuration->value,
>> 1) == 0) && us > 0) {
>> +        av_log(s, AV_LOG_DEBUG, "get_metadata_duration found duration in
>> context metadata: %" PRId64 "\n", us);
>> +        return us;
>> +    }
>> +
>> +    for (i = 0; i < s->nb_streams; i++) {
>> +        int64_t us;
>> +        AVDictionaryEntry *duration =
>> av_dict_get(s->streams[i]->metadata, "DURATION", NULL, 0);
>> +
>> +        if (duration && (av_parse_time(&us, duration->value, 1) == 0))
>> +            max = FFMAX(max, us);
>> +    }
>> +
>> +    av_log(s, AV_LOG_DEBUG, "get_metadata_duration returned: %" PRId64
>> "\n", max);
>> +    return max;
>> +}
>> +
>>  static int mkv_write_header(AVFormatContext *s)
>>  {
>>      MatroskaMuxContext *mkv = s->priv_data;
>> @@ -1495,6 +1520,7 @@ static int mkv_write_header(AVFormatContext *s)
>>      AVDictionaryEntry *tag;
>>      int ret, i, version = 2;
>>      int64_t creation_time;
>> +    int64_t metadataDuration;
> 
> 
> We don't use CamelCase for anything but structs. Don't you see the variable
> above and basically everywhere?
> Change it to metadata_duration.

And reduce its scope while at it. You can declare it in the same block
you're using it.