[FFmpeg-devel] [PATCH] matroskaenc.c DefaultDuration wrong value on stream copy

[Aurelien Jacobs]

On Tue, Jan 11, 2011 at 09:52:43PM +0100, Martin Tessarek wrote:
> on a stream copy with e.g.
> ffmpeg -i input -vcodec copy output.mkv
> the MATROSKA_ID_TRACKDEFAULTDURATION is set incorrectly to 1000, which some players or DLNA server interpret as 1000 FPS and then refuse to play the MKV.

According to the Matroska spec, track default_duration is:
  Number of nanoseconds (i.e. not scaled) per frame.
So if it's really set to 1000, the player should interpret this as
1000000 fps (which is obviously wrong).

> here my proposed patch:
> 
> --- matroskaenc.c	2010-11-23 09:24:41.000000000 +0100
> +++ matroskaenc.c	2011-01-11 21:31:41.000000000 +0100
> @@ -580,7 +580,8 @@
>          switch (codec->codec_type) {
>              case AVMEDIA_TYPE_VIDEO:
>                  put_ebml_uint(pb, MATROSKA_ID_TRACKTYPE, MATROSKA_TRACK_TYPE_VIDEO);
> -                put_ebml_uint(pb, MATROSKA_ID_TRACKDEFAULTDURATION, av_q2d(codec->time_base)*1E9);
> +                if(!st->stream_copy)
> +                    put_ebml_uint(pb, MATROSKA_ID_TRACKDEFAULTDURATION, av_q2d(codec->time_base)*1E9);

This is not OK.
The matroska encoder shouldn't care whether it is a stream copy or not.
Now, the problem we have is that codec->time_base is not really a
representation of the frame duration. It might be OK to use it with
constant frame rate sources, but not with variable frame rate.
In case of variable frame rate it might be better to try to use
avg_frame_rate to compute a default_duration, or even simpler, don't set
default_duration at all (it is not a mandatory mkv element).
It is also possible to add a sanity check. For example, just don't write
default_duration when codec->time_base is <5ms.

Aurel