Skip to content
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

mkvalidator doesn't detect wrong CueRelativePosition, CueDuration and CueClusterPosition #28

Open
mkver opened this issue Aug 22, 2017 · 1 comment · May be fixed by #39
Open

mkvalidator doesn't detect wrong CueRelativePosition, CueDuration and CueClusterPosition #28

mkver opened this issue Aug 22, 2017 · 1 comment · May be fixed by #39

Comments

@mkver
Copy link

mkver commented Aug 22, 2017

Hello,

mkvmerge started writing CueRelativePosition and CueDuration elements beginning in version 5.9; but there was a bug in the early implementation: Up until version 6.3 CueRelativePosition pointed to the inner block of a blockgroup (if a subtitle packet is stored in a block and not a simpleblock). But mkvalidator doesn't seem to check whether CueRelativePosition (and probably CueDuration, too) is wrong and says that such files appear to be valid.
[Edit]: After noticing mkvalidator not complaining about a file with a wrong CueClusterPosition entry I looked at the code a bit and found out that CueClusterPosition is not only not checked, but that the check for the validity of the cues is highly inefficient: Currently all blocks that precede (in storage order) the block with the right timestamp (if it exists) are checked, although there is no need to check all these blocks: It is only needed to check the cluster that is actually referenced via CueClusterPosition whether it contains an entry with the right timestamp and TrackID. If there isn't a cluster at the right place or it doesn't contain (simple)block with the right timestamp, then we already know that an error is appropriate and the file is invalid; there is no point in checking whether a different cluster contains a block with the right TrackID and timestamp.
[Edit2]: And judging from the code, only the first CueTrackPositions is actually checked (i.e. if there is no (Simple)Block with the TrackID given in the CueTrack in any subsequent CueTrackPositions, then no error is raised).

Greetings
Andi

@mkver mkver changed the title mkvalidator doesn't detect wrong CueRelativePosition mkvalidator doesn't detect wrong CueRelativePosition, CueDuration and CueClusterPosition May 5, 2018
@mkver
Copy link
Author

mkver commented Feb 8, 2019

I have fixed this myself. PR #39 is (among other things) about this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant