-
Notifications
You must be signed in to change notification settings - Fork 51
Chapter timestamp clarifications #1015
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
| For `Simple Chapters`, this is the position of the chapter markers in the timeline. | ||
|
|
||
| For `Simple Chapters`, the start timestamp **MUST** be greater than or equal to the start timestamp of the preceding `ChapterAtom`. Otherwise the Chapter **SHOULD** be ignored. | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ordered Chapters are sorted by definition. I think the spec should note whether Simple Chapters should be sorted by the player, or are required to be sorted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Simple Chapters are not sorted. It's up to the player to reorder them if needed. And they should not be ignored if they are not ordered. This would break backward compatibility.
| `Chapter`. | ||
|
|
||
| The end timestamp **MUST** be greater than or equal to the start timestamp. | ||
| Otherwise the end timestamp **SHOULD** be ignored. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can the spec say what to do if the MUST is not met?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It already says so in the element:
The value MUST be greater than or equal to the
ChapterTimeStartof the sameChapterAtom.
| Otherwise the end timestamp **SHOULD** be ignored. | ||
|
|
||
| For `Simple Chapters`, the end timestamp **MUST** be equal to or smaller than the start timestamp of the next ChapterAtom. | ||
| Otherwise the start timestamp of the next Chapter **SHOULD** be used as the end timestamp of this Chapter. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having overlapping timestamps on Simple Chapters that are on the same level (i.e. not nested) seems like something to avoid.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ChapterTimeEnd is not mandatory for Simple Chapters (non ordered). And even when it's there it's not really used. It's more informational. And overlapping chapters are OK in Simple Chapters. They are not used for playback. For example you could have a DJ mix where 2 songs overlap. You can mark each song independently (and tag anme).
| For `Simple Chapters`, the end timestamp **MUST** be equal to or smaller than the start timestamp of the next ChapterAtom. | ||
| Otherwise the start timestamp of the next Chapter **SHOULD** be used as the end timestamp of this Chapter. | ||
|
|
||
| Chapters with a duration of 0 are allowed, and might for example be used as markers such as bookmarks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When the
ChapterTimeEndtimestamp is equal to theChapterTimeStarttimestamp, the timestamp is included in theChapter.
The definition of a chapter as a half-open range seems quite natural. But I'm not sure what this was supposed to mean.
It can be useful to put markers in a file or add chapter commands with ordered chapter commands without having to play anything
Chapters already act as markers, but I can see how zero-length chapters could work as a different kind of marker. They would work more like points instead of segments.
To "add chapter commands with ordered chapter commands without having to play anything" is again not really an understandable sentence.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Chapter commands are only supported in Ordered Chapters. The 0 duration feature is supported there.
| If the `Parent Chapter` of a `Nested Chapter` has a `ChapterTimeEnd`, the `ChapterTimeStart` of that `Nested Chapter` | ||
| **MUST** be smaller than or equal to the `ChapterTimeEnd` of the `Parent Chapter`. | ||
| For `Simple Chapters`, the `ChapterTimeStart` of a `Nested Chapter` **MUST** be greater than or equal to the `ChapterTimeStart` of its `Parent Chapter`, and it **MUST** be smaller than the end timestamp of the `Parent Chapter`. | ||
| Otherwise the Nested Chapter **SHOULD** be ignored. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ChapterTimeStart of a Nested Chapter should fall within the duration of its Parent Chapter, whether the Parent Chapter has an explicit ChapterTimeEnd or an implicit end timestamp because of the next chapter.
Also I don't think the start timestamp of the Nested Chapter should be allowed to be equal to the end timestamp of the parent chapter, because the definition of the ChapterTimeEnd element says: "Timestamp of the end of Chapter (timestamp excluded)".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For Simple Chapters there is no mandatory duration for a Chapter. So there can't be a rule around that.
| When used with `Ordered Chapters`, the `ChapterTimeStart` and `ChapterTimeEnd` values of a `Parent Chapter` **MUST** be ignored for playback, | ||
| as the proper playback sections are described in its `Nested Chapters`. | ||
| The `ChapterTimeEnd` **SHOULD NOT** be set in `Parent Chapters` and **MUST** be ignored for playback. | ||
| The `ChapterTimeStart` of the `Parent Chapter` **SHOULD** be equal to that of its first `Nested Chapter`, and the `ChapterTimeEnd` of the `Parent Chapter` **SHOULD NOT** be set. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems to me with Ordered Chapters not only the ChapterTimeEnd of a Parent Chapter becomes meaningless with Nested Chapters, but also the ChapterTimeStart.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can have multiple chapters inside a parent chapter. In Ordered Chapters they just cannot start at the same time (as the parent).
robUx4
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We cannot edit this document as you propose. It's either errata if there were mistakes that need to be corrected, or a new document with new rules needs to be created. But RFC9559 describes the rules that have been in use for Matroska v1 to v4.
| For `Simple Chapters`, this is the position of the chapter markers in the timeline. | ||
|
|
||
| For `Simple Chapters`, the start timestamp **MUST** be greater than or equal to the start timestamp of the preceding `ChapterAtom`. Otherwise the Chapter **SHOULD** be ignored. | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Simple Chapters are not sorted. It's up to the player to reorder them if needed. And they should not be ignored if they are not ordered. This would break backward compatibility.
| `Chapter`. | ||
|
|
||
| The end timestamp **MUST** be greater than or equal to the start timestamp. | ||
| Otherwise the end timestamp **SHOULD** be ignored. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It already says so in the element:
The value MUST be greater than or equal to the
ChapterTimeStartof the sameChapterAtom.
| Otherwise the end timestamp **SHOULD** be ignored. | ||
|
|
||
| For `Simple Chapters`, the end timestamp **MUST** be equal to or smaller than the start timestamp of the next ChapterAtom. | ||
| Otherwise the start timestamp of the next Chapter **SHOULD** be used as the end timestamp of this Chapter. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ChapterTimeEnd is not mandatory for Simple Chapters (non ordered). And even when it's there it's not really used. It's more informational. And overlapping chapters are OK in Simple Chapters. They are not used for playback. For example you could have a DJ mix where 2 songs overlap. You can mark each song independently (and tag anme).
| For `Simple Chapters`, the end timestamp **MUST** be equal to or smaller than the start timestamp of the next ChapterAtom. | ||
| Otherwise the start timestamp of the next Chapter **SHOULD** be used as the end timestamp of this Chapter. | ||
|
|
||
| Chapters with a duration of 0 are allowed, and might for example be used as markers such as bookmarks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Chapter commands are only supported in Ordered Chapters. The 0 duration feature is supported there.
| If the `Parent Chapter` of a `Nested Chapter` has a `ChapterTimeEnd`, the `ChapterTimeStart` of that `Nested Chapter` | ||
| **MUST** be smaller than or equal to the `ChapterTimeEnd` of the `Parent Chapter`. | ||
| For `Simple Chapters`, the `ChapterTimeStart` of a `Nested Chapter` **MUST** be greater than or equal to the `ChapterTimeStart` of its `Parent Chapter`, and it **MUST** be smaller than the end timestamp of the `Parent Chapter`. | ||
| Otherwise the Nested Chapter **SHOULD** be ignored. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For Simple Chapters there is no mandatory duration for a Chapter. So there can't be a rule around that.
| When used with `Ordered Chapters`, the `ChapterTimeStart` and `ChapterTimeEnd` values of a `Parent Chapter` **MUST** be ignored for playback, | ||
| as the proper playback sections are described in its `Nested Chapters`. | ||
| The `ChapterTimeEnd` **SHOULD NOT** be set in `Parent Chapters` and **MUST** be ignored for playback. | ||
| The `ChapterTimeStart` of the `Parent Chapter` **SHOULD** be equal to that of its first `Nested Chapter`, and the `ChapterTimeEnd` of the `Parent Chapter` **SHOULD NOT** be set. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can have multiple chapters inside a parent chapter. In Ordered Chapters they just cannot start at the same time (as the parent).
I hope it is okay to open a PR like this.