[FFmpeg-devel] [PATCH] lavf/movenc: suggest video_track_timescale for invalid timescale

[Josh de Kock]

On 11/10/2016 16:47, Alexey Eromenko wrote:
> So, in which cases a large timebase would help ?
>
> Okay, I have allowed to manually over-ride the auto-detect setting via
> -video_track_timescale parameter, if someone really knows what he's
> doing. But the default settings (after my patch) work fine, with
> Windows Media Player, Quicktime, iTunes and VLC, both for normal input
> and broken input videos, both in MP4 and MOV output containers.
>
> GMail keep wrapping text, but this is the closes I could get:
>
> ffmpeg-apple-fix-v3.patch
>
> diff -uNr -U 6 ffmpeg-git-orig/libavformat/movenc.c
> ffmpeg-git/libavformat/movenc.c
> --- ffmpeg-git-orig/libavformat/movenc.c 2016-10-11 10:42:02.599749985 -0400
> +++ ffmpeg-git/libavformat/movenc.c 2016-10-11 11:10:22.793393487 -0400
> @@ -5669,22 +5669,27 @@
>                  track->timescale = mov->video_track_timescale;
>              } else {
>                  track->timescale = st->time_base.den;
>                  while(track->timescale < 10000)
>                      track->timescale *= 2;
>              }
> +            if (track->timescale > 100000 && (!mov->video_track_timescale)) {
> +                unsigned int timescale_new = (unsigned
> int)((double)(st->time_base.den)
> +                * 1000 / (double)(st->time_base.num));
> +                av_log(s, AV_LOG_WARNING,
> +                       "WARNING codec timebase is very high. If
> duration is too long,\n"
This is still wrapping. See
https://www.ffmpeg.org/developer.html#Submitting-patches-1

> +                       "file may not be playable by Apple Quicktime.
> Auto-setting\n"
> +                       "a shorter timebase %u instead of %d.\n",
> timescale_new, track->timescale);
> +                track->timescale = timescale_new;
> +            }
>              if (st->codecpar->width > 65535 || st->codecpar->height > 65535) {
> -                av_log(s, AV_LOG_ERROR, "Resolution %dx%d too large
> for mov/mp4\n", st->codecpar->width, st->codecpar->height);
> +                av_log(s, AV_LOG_ERROR, "Resolution %dx%d too large
> for mov/mp4\n",
> +                st->codecpar->width, st->codecpar->height);
>                  ret = AVERROR(EINVAL);
>                  goto error;
>              }
> -            if (track->mode == MODE_MOV && track->timescale > 100000)
> -                av_log(s, AV_LOG_WARNING,
> -                       "WARNING codec timebase is very high. If
> duration is too long,\n"
> -                       "file may not be playable by quicktime.
> Specify a shorter timebase\n"
> -                       "or choose different container.\n");
>              if (track->mode == MODE_MOV &&
>                  track->par->codec_id == AV_CODEC_ID_RAWVIDEO &&
>                  track->tag == MKTAG('r','a','w',' ')) {
>                  enum AVPixelFormat pix_fmt = track->par->format;
>                  if (pix_fmt == AV_PIX_FMT_NONE &&
> track->par->bits_per_coded_sample == 1)
>                      pix_fmt = AV_PIX_FMT_MONOWHITE;
>
>

I don't think it's good to have the default behaviour destroy the 
original timestamps.

-- 
Josh