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

[Michael Niedermayer]

On Wed, Oct 12, 2016 at 01:44:52PM +0100, Mark Thompson wrote:
> On 12/10/16 11:04, Carl Eugen Hoyos wrote:
> > 2016-10-11 21:06 GMT+02:00 Josh de Kock <josh at itanimul.li>:
> >> Fixes ticket #5882.
> > 
> > I don't object but I don't think it is correct.
> > 
> >> While it doesn't automatically set the timescale
> >> for the user as that would destroy data without prompt, it will tell
> >> the user how they could set the timescale (as this is mostly likely
> >> what they want).
> >>
> >> Signed-off-by: Josh de Kock <josh at itanimul.li>
> >> ---
> >>
> >>> Would it be useful to print the max duration?
> >>  I'm not entirely sure, open to suggestions though.
> >>
> >>  libavformat/movenc.c | 9 +++++++--
> >>  1 file changed, 7 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
> >> index d7c7158..6bada25 100644
> >> --- a/libavformat/movenc.c
> >> +++ b/libavformat/movenc.c
> >> @@ -5677,11 +5677,16 @@ static int mov_write_header(AVFormatContext *s)
> >>                  ret = AVERROR(EINVAL);
> >>                  goto error;
> >>              }
> >> -            if (track->mode == MODE_MOV && track->timescale > 100000)
> >> +            if (track->mode == MODE_MOV && track->timescale > 100000) {
> >> +                /* NOTE: forcing setting the suggested timescale manually means ffmpeg won't destroy
> >> +                 * timestamps without explicit instruction. */
> > 
> >> +                unsigned int suggested = (unsigned int)av_rescale(st->time_base.den, 1000, st->time_base.num);
> > 
> > If we go this way (I am not completely convinced about it), we definitely need
> > a heuristic that suggests 24000 for 10000000/417083 fps and 30000 if applicable.
> 
> How about the approach in the test program below?  This makes the whole timebase
> fraction; the suggested timescale would then be the denominator of that.  (It is
> currently just a program linked with lavu, I could make it into a patch if
> someone else thinks this might actually be a good idea.  It would need better
> error handling and some thoughts about overflow, too.)
> 
> It shows that we can do significantly better in the 417083/10000000 case than
> any of the currently-suggested approaches (divide by 1000, guess 1/24 or
> 1001/24000):
> 
> $ ./a.out 417083 10000000 100000
> 3715/89071
> $ bc -l
> 417083/10000000
> .04170830000000000000
> 3715/89071
> .04170830012012888594
> 417/10000
> .04170000000000000000
> 1/24
> .04166666666666666666
> 1001/24000
> .04170833333333333333
> 
> (The suggested timescale value would be 89071.)
> 
> ---
> 
> #include <stdio.h>
> 
> #include "libavutil/rational.h"
> 
> AVRational av_rational_reduce(AVRational qi, int max_den)

why dont you use av_reduce() directly ?


[...]


-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

In fact, the RIAA has been known to suggest that students drop out
of college or go to community college in order to be able to afford
settlements. -- The RIAA
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20161012/cb87fffe/attachment.sig>