Skip to content

Commit 2a17c84

Browse files
podborskipodborski
committed
Fix it35 sample entry parsing and identifier extraction
1 parent 9f176cb commit 2a17c84

5 files changed

Lines changed: 158 additions & 76 deletions

File tree

‎IsoLib/libisomediafile/src/ISOMovies.h‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -872,6 +872,26 @@ extern "C"
872872
MP4Track videoTrack, u32 trackReferenceType, MP4Track *outTrack,
873873
MP4Media *outMedia);
874874

875+
/**
876+
* @brief Read the t35_identifier and description from a serialized T.35 sample entry handle.
877+
* @ingroup SampleDescr
878+
*
879+
* Properly deserializes the handle returned by MP4GetMediaSampleDescription() and extracts
880+
* the T.35 identifier bytes and optional description string. The caller is responsible for
881+
* freeing *outIdentifier and *outDescription with free().
882+
*
883+
* @param sampleEntryH Handle containing the serialized 'it35' sample entry.
884+
* @param outIdentifier Output; receives a newly allocated copy of the t35_identifier bytes.
885+
* @param outIdentifierSize Output; receives the number of bytes in *outIdentifier.
886+
* @param outDescription Optional output; receives a newly allocated description string, or NULL
887+
* if no description is present. Pass NULL to ignore.
888+
* @return MP4NoErr on success, MP4NotFoundErr if no identifier is present, MP4BadParamErr on
889+
* invalid input.
890+
*/
891+
ISO_EXTERN(ISOErr)
892+
ISOGetT35SampleEntryFields(MP4Handle sampleEntryH, u8 **outIdentifier, u32 *outIdentifierSize,
893+
char **outDescription);
894+
875895
/*************************************************************************************************
876896
* VVC Sample descriptions
877897
************************************************************************************************/

‎IsoLib/libisomediafile/src/ISOSampleDescriptions.c‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2606,3 +2606,41 @@ ISONewT35MetadataTrack(MP4Movie theMovie, u32 timescale, const char *t35_prefix_
26062606
TEST_RETURN(err);
26072607
return err;
26082608
}
2609+
2610+
ISO_EXTERN(ISOErr)
2611+
ISOGetT35SampleEntryFields(MP4Handle sampleEntryH, u8 **outIdentifier, u32 *outIdentifierSize,
2612+
char **outDescription)
2613+
{
2614+
MP4Err err;
2615+
MP4T35MetadataSampleEntryPtr it35 = NULL;
2616+
2617+
if(sampleEntryH == NULL || outIdentifier == NULL || outIdentifierSize == NULL)
2618+
BAILWITHERROR(MP4BadParamErr);
2619+
2620+
*outIdentifier = NULL;
2621+
*outIdentifierSize = 0;
2622+
if(outDescription) *outDescription = NULL;
2623+
2624+
err = sampleEntryHToAtomPtr(sampleEntryH, (MP4AtomPtr *)&it35, MP4T35MetadataSampleEntryType);
2625+
if(err) goto bail;
2626+
2627+
if(it35->t35_identifier == NULL || it35->t35_identifier_size == 0) BAILWITHERROR(MP4NotFoundErr);
2628+
2629+
*outIdentifier = (u8 *)calloc(it35->t35_identifier_size, 1);
2630+
TESTMALLOC(*outIdentifier);
2631+
memcpy(*outIdentifier, it35->t35_identifier, it35->t35_identifier_size);
2632+
*outIdentifierSize = it35->t35_identifier_size;
2633+
2634+
if(outDescription && it35->description && it35->description[0] != '\0')
2635+
{
2636+
u32 len = (u32)strlen(it35->description) + 1;
2637+
*outDescription = (char *)calloc(len, 1);
2638+
TESTMALLOC(*outDescription);
2639+
memcpy(*outDescription, it35->description, len);
2640+
}
2641+
2642+
bail:
2643+
if(it35) it35->destroy((MP4AtomPtr)it35);
2644+
TEST_RETURN(err);
2645+
return err;
2646+
}

‎IsoLib/libisomediafile/src/T35MetadataSampleEntry.c‎

Lines changed: 40 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ static MP4Err createFromInputStream(MP4AtomPtr s, MP4AtomPtr proto, MP4InputStre
109109
MP4Err err;
110110
MP4T35MetadataSampleEntryPtr self = (MP4T35MetadataSampleEntryPtr)s;
111111
s64 bytesToRead;
112-
u32 descLen;
112+
u8 *buf = NULL;
113113
u32 i;
114114

115115
if(self == NULL) BAILWITHERROR(MP4BadParamErr)
@@ -119,53 +119,64 @@ static MP4Err createFromInputStream(MP4AtomPtr s, MP4AtomPtr proto, MP4InputStre
119119
GETBYTES(6, reserved);
120120
GET16(dataReferenceIndex);
121121

122-
/* Calculate remaining bytes to read */
122+
/* Read all remaining bytes into a flat buffer, then split on the first null byte.
123+
* Scanning byte-by-byte and "rewinding" by adjusting only bytesRead (while leaving
124+
* the stream cursor in place) does not work — readData always reads from the current
125+
* stream position. */
123126
bytesToRead = (s64)(self->size - self->bytesRead);
124127
if(bytesToRead < 0) BAILWITHERROR(MP4BadDataErr);
125128

126129
if(bytesToRead > 0)
127130
{
128-
/* Read description (null-terminated UTF-8 string) */
129-
/* First, scan for null terminator to find description length */
130-
descLen = 0;
131+
buf = (u8 *)calloc((u32)bytesToRead, 1);
132+
TESTMALLOC(buf);
133+
err = inputStream->readData(inputStream, (u32)bytesToRead, (char *)buf, NULL);
134+
if(err) goto bail;
135+
self->bytesRead += (u32)bytesToRead;
136+
137+
/* Find the null terminator that ends the description field */
138+
u32 nullPos = (u32)bytesToRead; /* default: no null found */
131139
for(i = 0; i < (u32)bytesToRead; i++)
132140
{
133-
u8 byte;
134-
err = inputStream->read8(inputStream, (u32 *)&byte, NULL);
135-
if(err) goto bail;
136-
descLen++;
137-
self->bytesRead++;
138-
if(byte == 0) break;
141+
if(buf[i] == 0)
142+
{
143+
nullPos = i;
144+
break;
145+
}
139146
}
140147

141-
/* Allocate and read description - we need to re-read the bytes */
142-
/* Rewind by backing up bytesRead */
143-
self->bytesRead -= descLen;
144-
148+
/* Description: bytes [0 .. nullPos] (including the null terminator) */
149+
u32 descLen = nullPos + 1; /* length including null */
145150
self->description = (char *)calloc(descLen, 1);
146151
TESTMALLOC(self->description);
147-
err = inputStream->readData(inputStream, descLen, (char *)self->description, NULL);
148-
if(err) goto bail;
149-
self->bytesRead += descLen;
150-
151-
/* Recalculate bytes remaining */
152-
bytesToRead = (s64)(self->size - self->bytesRead);
153-
154-
/* Read t35_identifier (everything remaining) */
155-
if(bytesToRead > 0)
152+
memcpy(self->description, buf, descLen);
153+
154+
/* t35_identifier: bytes after the null terminator */
155+
/* TODO: the length of t35_identifier is currently inferred as "all bytes remaining in the
156+
* box after the description null terminator", which prevents any optional boxes (e.g.
157+
* BitRateBox) from following it and makes the format non-extensible.
158+
* This must be resolved at the next MPEG meeting, either by:
159+
* (a) preceding t35_identifier with an explicit length field (e.g. unsigned int(8)), or
160+
* (b) wrapping description and t35_identifier in their own child boxes (e.g. 'hrsd' for
161+
* the human-readable description as already proposed in the amendment text).
162+
* Until then, optional boxes MUST NOT be appended after t35_identifier. */
163+
u32 identStart = descLen;
164+
if(identStart < (u32)bytesToRead)
156165
{
157-
self->t35_identifier_size = (u32)bytesToRead;
158-
self->t35_identifier = (u8 *)calloc(bytesToRead, 1);
166+
self->t35_identifier_size = (u32)bytesToRead - identStart;
167+
self->t35_identifier = (u8 *)calloc(self->t35_identifier_size, 1);
159168
TESTMALLOC(self->t35_identifier);
160-
err = inputStream->readData(inputStream, bytesToRead, (char *)self->t35_identifier, NULL);
161-
if(err) goto bail;
162-
self->bytesRead += (u32)bytesToRead;
169+
memcpy(self->t35_identifier, buf + identStart, self->t35_identifier_size);
163170
}
171+
172+
free(buf);
173+
buf = NULL;
164174
}
165175

166176
if(self->bytesRead != self->size) BAILWITHERROR(MP4BadDataErr)
167177

168178
bail:
179+
free(buf);
169180
TEST_RETURN(err);
170181
return err;
171182
}

‎IsoLib/t35_tool/extraction/DedicatedIt35Extractor.cpp‎

Lines changed: 59 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
extern "C" {
77
#include "MP4Movies.h"
88
#include "MP4Atoms.h"
9+
#include "ISOMovies.h"
910
}
1011

1112
#include <filesystem>
@@ -81,54 +82,67 @@ static MP4Err findIt35MetadataTrack(MP4Movie moov,
8182

8283
LOG_INFO("Found IT35 track with ID {}", trackID);
8384

84-
// Get T35MetadataSampleEntry to read description and t35_identifier
85-
MP4T35MetadataSampleEntryPtr it35Entry = (MP4T35MetadataSampleEntryPtr)(*sampleEntryH);
86-
87-
if (it35Entry && it35Entry->t35_identifier && it35Entry->t35_identifier_size > 0) {
88-
// Convert t35_identifier bytes to hex string
89-
std::string hexStr;
90-
hexStr.reserve(it35Entry->t35_identifier_size * 2);
91-
for (u32 i = 0; i < it35Entry->t35_identifier_size; i++) {
92-
char buf[3];
93-
snprintf(buf, sizeof(buf), "%02X", it35Entry->t35_identifier[i]);
94-
hexStr += buf;
95-
}
96-
97-
// Build prefix string: "HEX:Description"
98-
std::string filePrefix = hexStr;
99-
if (it35Entry->description && it35Entry->description[0] != '\0') {
100-
filePrefix += ":";
101-
filePrefix += it35Entry->description;
102-
LOG_DEBUG("Found T35 identifier: {} with description: '{}'", hexStr, it35Entry->description);
103-
} else {
104-
LOG_DEBUG("Found T35 identifier: {} (no description)", hexStr);
105-
}
106-
107-
// Parse both prefixes to compare hex part only
108-
T35Prefix requestedPrefix(t35PrefixStr);
109-
T35Prefix filePrefixParsed(filePrefix);
110-
111-
// Verify hex prefix matches (ignore description)
112-
if (requestedPrefix.hex() != filePrefixParsed.hex()) {
113-
LOG_DEBUG("T35 hex '{}' does not match requested hex '{}'",
114-
filePrefixParsed.hex(), requestedPrefix.hex());
115-
MP4DisposeHandle(sampleEntryH);
116-
continue; // Try next track
117-
}
118-
LOG_DEBUG("T35 hex matches requested hex");
119-
120-
// Warn if descriptions differ (informative only)
121-
if (!requestedPrefix.description().empty() &&
122-
!filePrefixParsed.description().empty() &&
123-
requestedPrefix.description() != filePrefixParsed.description()) {
124-
LOG_WARN("T.35 description mismatch: requested '{}' but file has '{}'",
125-
requestedPrefix.description(), filePrefixParsed.description());
126-
}
127-
} else {
85+
// Read t35_identifier and description from the serialized sample entry handle
86+
u8 *identifier = nullptr;
87+
u32 identifierSize = 0;
88+
char *description = nullptr;
89+
90+
MP4Err readErr = ISOGetT35SampleEntryFields(sampleEntryH, &identifier, &identifierSize,
91+
&description);
92+
MP4DisposeHandle(sampleEntryH);
93+
sampleEntryH = nullptr;
94+
95+
if(readErr || identifier == nullptr || identifierSize == 0)
96+
{
12897
LOG_WARN("Could not read t35_identifier from IT35 sample entry");
98+
free(identifier);
99+
free(description);
100+
continue;
129101
}
130102

131-
MP4DisposeHandle(sampleEntryH);
103+
// Convert t35_identifier bytes to hex string
104+
std::string hexStr;
105+
hexStr.reserve(identifierSize * 2);
106+
for(u32 j = 0; j < identifierSize; j++)
107+
{
108+
char buf[3];
109+
snprintf(buf, sizeof(buf), "%02X", identifier[j]);
110+
hexStr += buf;
111+
}
112+
free(identifier);
113+
114+
// Build prefix string: "HEX:Description"
115+
std::string filePrefix = hexStr;
116+
if(description && description[0] != '\0')
117+
{
118+
filePrefix += ":";
119+
filePrefix += description;
120+
LOG_DEBUG("Found T35 identifier: {} with description: '{}'", hexStr, description);
121+
}
122+
else
123+
{
124+
LOG_DEBUG("Found T35 identifier: {} (no description)", hexStr);
125+
}
126+
free(description);
127+
128+
// Parse both prefixes to compare hex part only
129+
T35Prefix requestedPrefix(t35PrefixStr);
130+
T35Prefix filePrefixParsed(filePrefix);
131+
132+
if(requestedPrefix.hex() != filePrefixParsed.hex())
133+
{
134+
LOG_DEBUG("T35 hex '{}' does not match requested hex '{}'",
135+
filePrefixParsed.hex(), requestedPrefix.hex());
136+
continue;
137+
}
138+
LOG_DEBUG("T35 hex matches requested hex");
139+
140+
if(!requestedPrefix.description().empty() && !filePrefixParsed.description().empty() &&
141+
requestedPrefix.description() != filePrefixParsed.description())
142+
{
143+
LOG_WARN("T.35 description mismatch: requested '{}' but file has '{}'",
144+
requestedPrefix.description(), filePrefixParsed.description());
145+
}
132146

133147
// Check for 'rndr' track reference
134148
MP4Track videoTrack = nullptr;

‎IsoLib/t35_tool/sources/SMPTE_ST2094_50.cpp‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -651,7 +651,7 @@ void SMPTE_ST2094_50::convertMetadataItemsToSyntaxElements(){
651651
if (sumCoefficients != Q_COMPONENT_MIXING_COEFFICIENT) { logMsg(LOGLEVEL_WARNING, "Sum component mixing coefficient for alternate %d is not equal to 1.0, they will be scaled to 1.0 at decoding", iAlt); }
652652
}
653653
if (elm.component_mixing_type[0] != elm.component_mixing_type[iAlt]) {
654-
elm.component_mixing_type = false;
654+
elm.has_common_component_mix_params_flag = false;
655655
}
656656

657657
// Create syntax elements for the gain curve function
@@ -1041,7 +1041,6 @@ void SMPTE_ST2094_50::convertSyntaxElementsToMetadataItems(){
10411041
}
10421042
if (sumComponent != Q_COMPONENT_MIXING_COEFFICIENT){
10431043
logMsg(LOGLEVEL_WARNING, "Sum component mixing coefficient for alternate %d is not equal to 1.0, they will be scaled to 1.0.", iAlt); }
1044-
}
10451044
for (int k = 0; k < MAX_NB_COMPONENT_MIXING_COEFFICIENT; k++) {
10461045
float value = 0.0f;
10471046
if (elm.has_component_mixing_coefficient_flag[iAlt][k]) {

0 commit comments

Comments
 (0)