On Tue, Jan 31, 2017 at 10:18 PM, wm4 <nfx...@googlemail.com> wrote: > On Tue, 31 Jan 2017 12:02:01 -0800 > Chris Cunningham <chcunning...@chromium.org> wrote: > > > Thanks for taking a look. > > > > Definitely missing a "break;" - will fix in subsequent patch. > > > > Agree timestamps should be relative (didn't realize this). Vignesh points > > out that "0" in the test file is due to a bug in ffmpeg (and probably > other > > muxers) where this value is not written as a relative timestamp, but > > instead as the timestamp of the previous frame. https://github.com/FFmp > > eg/FFmpeg/blob/master/libavformat/matroskaenc.c#L2053 > > <https://www.google.com/url?q=https%3A%2F%2Fgithub.com% > 2FFFmpeg%2FFFmpeg%2Fblob%2Fmaster%2Flibavformat% > 2Fmatroskaenc.c%23L2053&sa=D&sntz=1&usg=AFQjCNGs8m6GsWbhTvCZl0Q_juGAldQblA > > > > Just a few lines below this reads > > mkv->last_track_timestamp[track_number - 1] = ts - mkv->cluster_pts; > > which looks like it intends to write a relative value. Though "ts" can > be a DTS, while the other value is always a PTS. > > > Anyway, I'm all for following Haali. Its not obvious how best to do > this. I > > don't think there's any great default value to indicate "not-set" and the > > generic embl parsing code that reads the reference timestamp doesn't > really > > lend itself to setting an additional field like > > MatroskaBlock.has_reference. I can sort this out, but I'll pause in case > > you have a recommendation in-mind. > > I don't know a nice way either. Here are 3 potential ways I came up > with: > > 1. Use a better dummy value, like INT64_MIN, which is almost 100% > unlikely to appear in a valid file. (This is probably equivalent to > your intention in original patch.) > 2. Add a new "presence_flag_offset" field to EbmlSyntax - the EBML > reader will write a 1 to it if the element was found. (This is a bit > annoying, because 0 is a valid offset, and you surely wouldn't want > to change all the other element definitions.) > 3. Add a new type like EBML_PRESENCE, which writes whether the element > was present or not to data_offset, instead of the element's content > or its default value. (There are some other special "types" like > EBML_STOP, so maybe this idea isn't all too hacky.) > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
Sorry, for crossed threads - thanks for these ideas. I like the first option best. Will upload a new patch using INT64_MIN and add the break; you pointed out earlier. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
