On Sat, Jan 14, 2017 at 3:57 AM, Michael Niedermayer <michae...@gmx.at> wrote:
> On Tue, Dec 27, 2016 at 09:47:47PM -0800, Michael Bradshaw wrote: > > + // ms precision is the de-facto standard timescale for mkv files > > + mkv->timecode_scale = 1000000; > > + > > + // If the user has supplied a desired time base, use it if possible > > + if (s->nb_streams > 0) { > > + AVRational time_base = s->streams[0]->time_base; > > + for (i = 1; i < s->nb_streams; i++) { > > inconsistent indention > Fixed. > + if (time_base.num != s->streams[i]->time_base.num || > > + time_base.den != s->streams[i]->time_base.den) { > > + time_base = av_make_q(0, 0); > > + break; > > + } > > + } > > This looks just at one stream, what if other streams have other > timebases ? > you could pick a common tb so that timestamps in any of the requested > ones can be represented. While this wont always be possible due to > bits per int constraint i dont think completely ignoring other streams > is safe. > The other streams aren't completely ignored. They're checked to make sure they all have the same time base (and if they don't, it falls back to the default 1/1000 time base). > + // Make sure the time base is valid, can losslessly be converted to > > + // nanoseconds, and isn't longer than 1 second > > + if (time_base.num > 0 && > > + time_base.den > 0 && > > + 1000000000 % time_base.den == 0 && > > + time_base.num <= time_base.den) { > > > + mkv->timecode_scale = (int)av_rescale(time_base.num, > 1000000000, time_base.den); > > assuming someone asks for 1001/24000 he would get something else > and something that is lower precission than what the default would > have been prior to this patch IIUC Before this patch the time base was always set to 1/1000, which is what this patch defaults to if the streams don't have a common time base that can be losslessly converted to nanoseconds, so this patch should never make things less precise than before. I have mixed feelings on changing this patch so that it picks a common time base between multiple streams or truncates a repeating decimal. Matroska only stores timestamps as multiples of nanoseconds, and I fear automatically computing a time base might push the time base closer and closer to 1/1000000000. Using a tiny timebase like that causes extra chunking in Matroska, as each block in a Cluster only stores its timestamp as a 16-bit int. I'd rather my other patch[1] be used to let the user request a specific time base than trying to auto-compute a time base. I've updated the patch to fix the indentation as well as ignore the time base hint for WebM, since WebM muxers are required to use a time base of 1/1000[2]. Apologies for the gmail attachment; I'm still figuring out git send-email. [1]: http://ffmpeg.org/pipermail/ffmpeg-devel/2016-December/205017.html [2]: http://www.webmproject.org/docs/container/#muxer-guidelines
0001-avformat-matroskaenc-Accept-time-base-hint.patch
Description: Binary data
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel