Skip to content

Commit

Permalink
mkvalidator: Fix checking of cues
Browse files Browse the repository at this point in the history 
The earlier code had several flaws:

1. It was very slow, because finding a matching block always started at
with the very first cluster.
2. Only one CueTrackPositions (namely the first one) has been checked
per CuePoint, although there can be multiple CueTrackPositions.
3. If e.g. no CueTrack element was present, one got a generic error
(error code 0x200), because a mandatory element was missing; but
MATROSKA_CueTrackNum returned -1 (for invalid TrackNum) and so one also
received a nonsense error 0x312: "CueEntry Track #-1 and timecode xxx ms
not found".
4. The CueClusterPosition and CueRelativePosition elements have not been
checked at all.

All four points have been corrected.
(mkvmerge versions 5.9 to 6.3 (inclusive) wrote CueRelativePosition
elements that (in case of BlockGroups) pointed to the inner blocks and
not the outer BlockGroups. mkvalidator now detects such files as
invalid.)

Signed-off-by: Andreas Rheinhardt <[email protected]>
  • Loading branch information
@mkver
mkver committed Feb 8, 2019
1 parent 4667ef8 commit f119785
Showing 1 changed file with 113 additions and 21 deletions.
134 changes: 113 additions & 21 deletions mkvalidator/mkvalidator.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -854,42 +854,134 @@ static int CheckLacingKeyframe(void)
return Result;
}

static int CheckCueEntries(ebml_master *Cues)
static int CheckCueEntries(ebml_master *Cues, filepos_t SegmentDataOffset)
{
int Result = 0;
timecode_t TimecodeEntry, PrevTimecode = INVALID_TIMECODE_T;
int16_t TrackNumEntry;
matroska_cluster **Cluster;
matroska_block *Block;
int ClustNum = 0;

if (!RSegmentInfo)
Result |= OutputError(0x310,T("A Cues (index) is defined but no SegmentInfo was found"));
else if (ARRAYCOUNT(RClusters,matroska_cluster*))
{
matroska_cuepoint *CuePoint = (matroska_cuepoint*)EBML_MasterFindChild(Cues, &MATROSKA_ContextCuePoint);
matroska_cluster **Cluster = ARRAYBEGIN(RClusters,matroska_cluster*);
matroska_cuepoint *CuePoint = (matroska_cuepoint*)EBML_MasterFindChild(Cues, &MATROSKA_ContextCuePoint);
timecode_t TimecodeEntry, PrevTimecode = INVALID_TIMECODE_T;
int CueNum = 0;

while (CuePoint)
{
if (!Quiet && ClustNum++ % 24 == 0)
if (!Quiet && CueNum++ % 24 == 0)
TextWrite(StdErr,T("."));
MATROSKA_LinkCueSegmentInfo(CuePoint,RSegmentInfo);
TimecodeEntry = MATROSKA_CueTimecode(CuePoint);
TrackNumEntry = MATROSKA_CueTrackNum(CuePoint);

if (TimecodeEntry == INVALID_TIMECODE_T)
goto NextCuePoint;

if (TimecodeEntry < PrevTimecode && PrevTimecode != INVALID_TIMECODE_T)
OutputWarning(0x311,T("The Cues entry for timecode %") TPRId64 T(" ms is listed after entry %") TPRId64 T(" ms"),Scale64(TimecodeEntry,1,1000000),Scale64(PrevTimecode,1,1000000));

// find a matching Block
for (Cluster = ARRAYBEGIN(RClusters,matroska_cluster*);Cluster != ARRAYEND(RClusters,matroska_cluster*); ++Cluster)
{
Block = MATROSKA_GetBlockForTimecode(*Cluster, TimecodeEntry, TrackNumEntry);
if (Block)
break;
}
if (Cluster == ARRAYEND(RClusters,matroska_cluster*))
Result |= OutputError(0x312,T("CueEntry Track #%d and timecode %") TPRId64 T(" ms not found"),(int)TrackNumEntry,Scale64(TimecodeEntry,1,1000000));
PrevTimecode = TimecodeEntry;
CuePoint = (matroska_cuepoint*)EBML_MasterFindNextElt(Cues, (ebml_element*)CuePoint, 0, 0);
for (ebml_element *CueTrackPositions = EBML_MasterFindChild(CuePoint, &MATROSKA_ContextCueTrackPositions);
CueTrackPositions; CueTrackPositions = EBML_MasterNextChild(CuePoint, CueTrackPositions))
{
ebml_element *Block = NULL, *Element = EBML_MasterFindChild(CueTrackPositions,&MATROSKA_ContextCueTrack);
filepos_t ClusterOffset, CueRelativePosition;
int Direction;
int16_t TrackNumEntry;
bool_t ClusterExists;
tchar_t Error[384];

if (!Element)
continue;

TrackNumEntry = EL_Int(Element);

Element = EBML_MasterFindChild(CueTrackPositions, &MATROSKA_ContextCueClusterPosition);
if (!Element)
continue;

ClusterOffset = EL_Int(Element) + SegmentDataOffset;

Element = EBML_MasterFindChild(CueTrackPositions, &MATROSKA_ContextCueRelativePosition);
CueRelativePosition = Element ? EL_Int(Element) : INVALID_FILEPOS_T;

Direction = EL_Pos(*Cluster) <= ClusterOffset ? 1 : -1;

do
{
if (EL_Pos(*Cluster) == ClusterOffset)
{
Block = MATROSKA_GetNextBlockForTimecode(*Cluster, NULL, TimecodeEntry, TrackNumEntry, 1);
break;
}

if ((Direction == 1 && (EL_Pos(*Cluster) > ClusterOffset || Cluster == ARRAYEND(RClusters, matroska_cluster*) - 1)) ||
(Direction ==-1 && (EL_Pos(*Cluster) < ClusterOffset || Cluster == ARRAYBEGIN(RClusters, matroska_cluster*))))
break;

Cluster += Direction;
}
while (1);

if (Block)
{
ebml_element *PreviousBlock;

if (CueRelativePosition == INVALID_FILEPOS_T || EL_Pos(Block) - EBML_ElementPositionData((ebml_element*)*Cluster) == CueRelativePosition)
continue;

while (Block && EL_Pos(Block) - EBML_ElementPositionData((ebml_element*)*Cluster) < CueRelativePosition)
{
PreviousBlock = Block;
Block = MATROSKA_GetNextBlockForTimecode(*Cluster, PreviousBlock, TimecodeEntry, TrackNumEntry, 1);
}

Block = Block ? Block : PreviousBlock;

if (EL_Pos(Block) - EBML_ElementPositionData((ebml_element*)*Cluster) == CueRelativePosition)
continue;
}

stprintf_s(Error, TSIZEOF(Error), T("An invalid Cues entry points to a (Simple)Block in track #%d with timecode %")
TPRId64 T(" ms supposedly in the cluster with offset %") TPRId64, TrackNumEntry, Scale64(TimecodeEntry,1,1000000), ClusterOffset);

if (CueRelativePosition != INVALID_FILEPOS_T)
stcatprintf_s(Error, TSIZEOF(Error), T(" (relative offset %") TPRId64 T(")"), CueRelativePosition);

if (Block)
{
stcatprintf_s(Error, TSIZEOF(Error), T(". The actual relative offset is %") TPRId64 T("."),
EL_Pos(Block) - EBML_ElementPositionData((ebml_element*)*Cluster));
Result |= OutputError(0x314, Error);
continue;
}

if (EL_Pos(*Cluster) != ClusterOffset)
tcscat_s(Error, TSIZEOF(Error), T(". No cluster with this offset exists"));

for (Cluster = ARRAYBEGIN(RClusters, matroska_cluster*); Cluster != ARRAYEND(RClusters, matroska_cluster*); ++Cluster)
{
Block = MATROSKA_GetNextBlockForTimecode(*Cluster, NULL, TimecodeEntry, TrackNumEntry, 1);
if (Block)
break;
}

if (!Block)
{
tcscat_s(Error, TSIZEOF(Error), T(". No (Simple)Block with this timecode exists in this track at all."));
Result |= OutputError(0x312, Error);
}
else
{
stcatprintf_s(Error, TSIZEOF(Error), T(". A (Simple)Block with this timecode is in the cluster with offset %") TPRId64
T(" (relative offset %") TPRId64 T(")."), EL_Pos(*Cluster), EL_Pos(Block) - EBML_ElementPositionData((ebml_element*)*Cluster));
Result |= OutputError(0x313, Error);
}
}

PrevTimecode = TimecodeEntry;

NextCuePoint:
CuePoint = (matroska_cuepoint*)EBML_MasterFindNextElt(Cues, (ebml_element*)CuePoint, 0, 0);
}
}
return Result;
Expand Down Expand Up @@ -1384,7 +1476,7 @@ int main(int argc, const char *argv[])
OutputWarning(0x800,T("The segment has Clusters but no Cues section (bad for seeking)"));
}
else
Result |= CheckCueEntries(RCues);
Result |= CheckCueEntries(RCues, EBML_ElementPositionData((ebml_element*)RSegment));
if (!RTrackInfo)
{
Result = OutputError(0x41,T("The segment has Clusters but no TrackInfo section"));
Expand Down

0 comments on commit f119785

Please sign in to comment.