[FFmpeg-devel] [PATCH] Apple Quicktime fix

[Ronald S. Bultje]

Hi,

On Oct 11, 2016 2:40 PM, "Alexey Eromenko" <al4321 at gmail.com> wrote:
>
> On Tue, Oct 11, 2016 at 8:29 PM, Josh de Kock <josh at itanimul.li> wrote:
> > On 11/10/2016 18:24, Alexey Eromenko wrote:
> >>
> >> ---
> >>  libavformat/movenc.c | 14 +++++++++-----
> >>  1 file changed, 9 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
> >> index 8b4aa5f..0e2fc55 100644
> >> --- a/libavformat/movenc.c
> >> +++ b/libavformat/movenc.c
> >> @@ -5666,16 +5666,20 @@ static int mov_write_header(AVFormatContext *s)
> >>                  while(track->timescale < 10000)
> >>                      track->timescale *= 2;
> >>              }
> >> +            if (track->timescale > 100000 &&
> >> (!mov->video_track_timescale)) {
> >
> > As I said before, having this as 'default' behaviour would interfere
with
> > large, but correct timescales. This should only be a suggestion to the
user.
> >
>
> This happens only for very high timescale from source video, and it
> seems to work on Apple QuickTime/iTunes, WMP and VLC.
> For the vast majority videos (99%+), I _do not_ touch the timebase.
> And when I do touch, I give a WARNING to the user.
> Plus I offer to override this decision via a parameter.
> It should be stable and regression-free for most of our users.
> Is there any downside for this approach ?
>
> >> +                unsigned int timescale_new = (unsigned
> >> int)((double)(st->time_base.den)
> >> +                * 1000 / (double)(st->time_base.num));
> >
> > You surely don't need all these casts.
>
> Unless I'm guaranteed to get int64 on ALL platforms, I don't see
> how-to write this code without casts to double-float. Int32 will
> over-flow, even unsigned.
> I can do the multiply after the division, but afraid of losing precision.
> And since this is not a real-time code anyway, I consider this an
> "okay" trade-off.
> Feel free to replace it with a better alternative.

We use int64_t all over the place, it's OK to rely on it since it
guarantees the same result across archs, which floats don't.

Ronald