ietf-wg-cellar / matroska-specification

Matroska specification.
http://ietf-wg-cellar.github.io/matroska-specification
Other
121 stars 45 forks source link

CueTime in Segment Ticks instead of Matroska Ticks #814

Closed sebastiendu closed 8 months ago

sebastiendu commented 9 months ago

This aims to align the specification with the usage. mkvinfo and other tools such as VLC read CueTimes in Segment Ticks (usually milliseconds), not Matroska Ticks (nanoseconds).

mbunkus commented 9 months ago

Yep, CuePoint has always used values scaled by TimestampScale. Dunno when the mixup in the specs happened.

sebastiendu commented 9 months ago

Yep, CuePoint has always used values scaled by TimestampScale. Dunno when the mixup in the specs happened.

It happened with commit https://github.com/ietf-wg-cellar/matroska-specification/commit/d2ea6bf2f246d2685c6c6375bc30d3563b2b75fe which might have introduced more inconsistencies.

mbunkus commented 9 months ago

Hmm, the wording of e.g. the segment duration is somewhat more verbose and therefore not as confusing:

Duration of the Segment in nanoseconds, expressed in Segment Ticks which is based on TimestampScale; see (#timestamp-ticks)

Maybe this would be the more appropriate wording for CuePoint, too, as the resulting value after scaling via TimestampScale is indeed a number of nanoseconds.

sebastiendu commented 9 months ago

It happened with commit d2ea6bf which might have introduced more inconsistencies.

From that commit only remain SeekPreRoll, CodecDelay and DiscardPadding in notes.md and Duration in ebml_matroska.xml - I know the SeekPreRoll and Duration lines are correct, but I did not test CodecDelay nor DiscardPadding (as I am not using these in my code).

mbunkus commented 9 months ago

From that commit only remain SeekPreRoll, CodecDelay and DiscardPadding in notes.md and Duration in ebml_matroska.xml - I know the SeekPreRoll and Duration lines are correct, but I did not test CodecDelay nor DiscardPadding (as I am not using these in my code).

CodecDelay and DiscardPadding are not scaled and contain raw nanoseconds (that's how libMatroska and therefore MKVToolNix handles them).

sebastiendu commented 9 months ago

CodecDelay and DiscardPadding are not scaled and contain raw nanoseconds (that's how libMatroska and therefore MKVToolNix handles them).

Ok then, I have no other fix to submit.

sebastiendu commented 9 months ago

Hmm, the wording of e.g. the segment duration is somewhat more verbose and therefore not as confusing:

Duration of the Segment in nanoseconds, expressed in Segment Ticks which is based on TimestampScale; see (#timestamp-ticks)

That is not the current wording in branch master.

Maybe this would be the more appropriate wording for CuePoint, too, as the resulting value after scaling via TimestampScale is indeed a number of nanoseconds.

I find it more confusing to say "in nanoseconds" and "in Segment Ticks" at the same time.

mbunkus commented 9 months ago

Yeah, I copy-pasted it from the commit you identified as introducing the change.

Maybe an entirely different wording is required.

robUx4 commented 8 months ago

Indeed, that's the proper definition at least based on all the code I can find. In the past it was defined as "Absolute timestamp according to the Segment time base.".

The wording is correct as it matches other elements using the Segment Tick.

robUx4 commented 8 months ago

Merging but it might become an errata as we can't submit changes and the Draft is in finalization stage to become an RFC.

robUx4 commented 8 months ago

It seems the CueRefTime has the same problem. It's handled with the TimestampScale in libmatroska.