Fix: Fix ignored test cases

[plantysnake]

Description

Fixed all ignored failing conformance tests, excluding video_2500000bps_0.mp4, as it seems to have broken second mdat (MPEGGroup/FileFormatConformance#131)

---

Mehd - ISO/IEC 14496-12 8.8.2.1

Box Type: ‘mehd’
Container: Movie Extends Box(‘mvex’)
Mandatory: No
Quantity: Zero or one

So, field should be optional/nullable. Found during testing segmented files with file-segmenter demo.

---

Stsz - ISO/IEC 14496-12 8.7.3.2.2

sample_size is integer specifying the default sample size. If all the samples are the same size,
this field contains that size value. If this field is set to 0, then the samples have different sizes,
and those sizes are stored in the sample size table. If this field is not 0, it specifies the constant
sample size, and no array follows.

So, during write, we do not want to check sample_sizes values themselves, but rather we want to check if sample_size is 0.

---

Parser's parseOneBox logic for size === 0 without parentSize and box 'mdat' didn't actually assign new value to size. This was fixed.

[plantysnake]

On unrelated note, iproBox has sinfs field, that is used in isofile.flattenItemInfo using getter protections, but is never populated due to FullBox parse just storing the buffer
Suggestion is to parse it like ContainerBox's parse, but further testing is needed

[DenizUgur]

Thank you for the PR. We're you able to investigate the issue with the last ignored test? Anything you'd like to share?

[plantysnake]

@DenizUgur
The last test case fails, because we have broken mdat box at the end of the buffer and we don't actually parse its data, i.e. at the end we still have 6 buffers of unparsed data:

[0:00:00.513] [ISOFile] Flushing remaining samples
[0:00:00.513] [MultiBufferStream] 6 stored buffer(s) (21893/375309 bytes), continuous ranges: [655360-1030668]

The issue can be solved by calling appendBuffer with dummy empty buffer with filePosition at expected box end during flush.
My proposed solution:

flush() {
  Log.info('ISOFile', 'Flushing possible remaining broken box data and remaining samples');
  const dummyBuffer = MP4BoxBuffer.fromArrayBuffer(new ArrayBuffer(0), this.lastBoxStartPosition);
  this.appendBuffer(dummyBuffer, true, true);
}

appendBuffer(ab: MP4BoxBuffer, last?: boolean, dummy?: boolean) {
  let nextFileStart: number;
  if (!this.checkBuffer(ab, dummy)) {
    return;
  }

  /* Parse whatever is in the existing buffers */
  this.parse();
  ....
}

checkBuffer(ab?: MP4BoxBuffer, dummy?: boolean) {
  if (!ab && !dummy) throw new Error('Buffer must be defined and non empty');
  if (ab.byteLength === 0 && !dummy) {
    Log.warn('ISOFile', 'Ignoring empty buffer (fileStart: ' + ab.fileStart + ')');
    this.stream.logBufferLevel();
    return false;
  }
  Log.info('ISOFile', 'Processing buffer (fileStart: ' + ab.fileStart + ')');
  ....
}

But I don't really like this approach.
Thoughts?

[DenizUgur]

I haven't looked into the problem so I don't have any strong opinion but appending a dummy buffer feels wrong.

[plantysnake]

There's also some additional issues present, that are not covered by conformance tests:

• sbgp box writing - version can be set incorrectly if version is 1, but grouping_type_parameter is set to 0.
  Proposed solution is to not determine version from this field

/** @bundle writing/sbgp.js */
write(stream: MultiBufferStream) {
  // if (this.grouping_type_parameter) this.version = 1;
  // else this.version = 0;
  this.flags = 0;
  ....
}

• seig samplegroup doesn't support multikeys from ISO/IEC 23001-07:2023 Section 6

  aligned(8) class CencSampleEncryptionInformationGroupEntry extends SampleGroupEntry( ‘seig’ )
  {
    unsigned int(1) multi_key_flag;
    unsigned int(7) reserved = 0;
    unsigned int(4) crypt_byte_block;
    unsigned int(4) skip_byte_block;
    unsigned int(8) isProtected;
    if (multi_key_flag == 1) {
      unsigned int(16) key_count;
    } else {
      key_count = 1;
    }
    for (i=1; I <= key_count; i++) {
      unsigned int(8) Per_Sample_IV_Size;
      unsigned int(8)[16] KID;
      if (Per_Sample_IV_Size == 0) {
        unsigned int(8) constant_IV_size;
        unsigned int(8)[constant_IV_size] constant_IV;
      }
    }
  }

  Proposed solution:

  class SeigKey {
    Per_Sample_IV_Size: number;
    KID: string;
    constant_IV_size: number;
    constant_IV: number | Uint8Array;
  
    parse(stream: MultiBufferStream, isProtected: number) {
      this.Per_Sample_IV_Size = stream.readUint8();
      this.KID = parseHex16(stream);
      this.constant_IV_size = 0;
      this.constant_IV = 0;
      if (isProtected === 1 && this.Per_Sample_IV_Size === 0) {
        this.constant_IV_size = stream.readUint8();
        this.constant_IV = stream.readUint8Array(this.constant_IV_size);
      }
    }
  }
  
  export class seigSampleGroupEntry extends SampleGroupEntry {
    static grouping_type = 'seig' as const;
  
    multi_key_flag: number;
    reserved: number;
    crypt_byte_block: number;
    skip_byte_block: number;
    isProtected: number;
    key_count: number;
    keys: Array<SeigKey> = [];
  
    parse(stream: MultiBufferStream) {
      // this.reserved = stream.readUint8();
      let tmp = stream.readUint8();
      this.multi_key_flag = (tmp >> 7) & 0x01;
      this.reserved = tmp & 0x7f;
      tmp = stream.readUint8();
      this.crypt_byte_block = (tmp >> 4) & 0xf;
      this.skip_byte_block = tmp & 0xf;
      this.isProtected = stream.readUint8();
      // this.Per_Sample_IV_Size = stream.readUint8();
      // this.KID = parseHex16(stream);
      // this.constant_IV_size = 0;
      // this.constant_IV = 0;
      // if (this.isProtected === 1 && this.Per_Sample_IV_Size === 0) {
      //     this.constant_IV_size = stream.readUint8();
      //     this.constant_IV = stream.readUint8Array(this.constant_IV_size);
      // }
  
      if (this.multi_key_flag === 1) {
        this.key_count = stream.readUint16();
      } else {
        this.key_count = 1;
      }
  
      for (let i = 0; i < this.key_count; i++) {
        const key = new SeigKey();
        key.parse(stream, this.isProtected);
        this.keys.push(key);
      }
    }
  }

• sgpd box has a couple of issues:

  • Additional default_sample_description_index field, that does not exist, but being written in write method instead of default_group_description_index field
  • SampleGroupEntry size won't be calculated if box version is 0, as it will be set to 0 and will result in entry.data being buffer(0). However, calculating entry size is tricky, I only found examples of such sgpd boxes with only one entry, so my solution might be completely wrong.
  • flags field should not be set to 0
    Proposed solution:

    default_group_description_index: number;
    // default_sample_description_index: number;
    entry_count: number;
    
    parse(stream: MultiBufferStream) {
      this.parseFullHeader(stream);
      let read_bytes = this.hdr_size;
      this.grouping_type = stream.readString(4);
      read_bytes += 4;
      Log.debug('BoxParser', 'Found Sample Groups of type ' + this.grouping_type);
      if (this.version === 1) {
        this.default_length = stream.readUint32();
        read_bytes += 4;
      } else {
        this.default_length = 0;
      }
      if (this.version >= 2) {
        this.default_group_description_index = stream.readUint32();
        read_bytes += 4;
      }
      this.entries = [];
      // const entry_count = stream.readUint32();
      this.entry_count = stream.readUint32();
      read_bytes += 4;
      // for (let i = 0; i < entry_count; i++) {
      for (let i = 0; i < this.entry_count; i++) {
        let entry: SampleGroupEntry;
        if (this.grouping_type in BoxRegistry.sampleGroupEntry) {
          entry = new BoxRegistry.sampleGroupEntry[this.grouping_type](this.grouping_type);
        } else {
          entry = new SampleGroupEntry(this.grouping_type);
        }
        this.entries.push(entry);
        if (this.version === 1) {
          if (this.default_length === 0) {
            entry.description_length = stream.readUint32();
            read_bytes += 4;
          } else {
            entry.description_length = this.default_length;
          }
        } else {
          entry.description_length = this.default_length;
        }
        if (entry.write === SampleGroupEntry.prototype.write) {
          Log.info(
            'BoxParser',
            'SampleGroup for type ' +
              this.grouping_type +
              ' writing not yet implemented, keeping unparsed data in memory for later write',
          );
          const entry_length =
            entry.description_length === 0 ? this.size - read_bytes : entry.description_length;
          // storing data
          // entry.data = stream.readUint8Array(entry.description_length);
          entry.data = stream.readUint8Array(entry_length);
          // rewinding
          // stream.seek(stream.getPosition() - entry_length);
          stream.seek(stream.getPosition() - entry_length);
        }
        entry.parse(stream);
      }
    }
    
    write(stream: MultiBufferStream) {
      // leave version as read
      // this.version;
      // this.flags = 0;
      this.size = 8;
      if (this.version === 1) {
        this.size += 4;
      }
      if (this.version >= 2) {
        this.size += 4;
      }
      for (let i = 0; i < this.entries.length; i++) {
        const entry = this.entries[i];
        if (this.version === 1) {
          if (this.default_length === 0) {
            this.size += 4;
          }
          // this.size += entry.data.length;
        }
        this.size += entry.data.length;
      }
      stream.writeString(this.grouping_type, undefined, 4);
      if (this.version === 1) {
        stream.writeUint32(this.default_length);
      }
      if (this.version >= 2) {
        // stream.writeUint32(this.default_sample_description_index);
        stream.writeUint32(this.default_group_description_index);
      }
      // stream.writeUint32(this.entries.length);
      stream.writeUint32(this.entry_count);
      for (let i = 0; i < this.entries.length; i++) {
        const entry = this.entries[i];
        if (this.version === 1) {
          if (this.default_length === 0) {
            stream.writeUint32(entry.description_length);
          }
        }
        entry.write(stream);
      }
    }

  • write assumes that no SampleGroupEntry has actual write method, that can modify box size, this should be checked in the future.

• uuid boxes of known types will break workflow, because static uuid field is not uuid field of the box.
  Proposed solution: populate box's uuid field in constructor from static uuid OR in parser's parseOneBox set box.uuid = uuid in both cases, when uuid is known and when it is not

  class UUIDBox extends Box {
    static override readonly fourcc = 'uuid' as const;
    static uuid?: string;
  
    constructor(size = 0) {
      super(size);
      this.uuid = (this.constructor as typeof UUIDBox).uuid;
    }
  }
  
  class UUIDFullBox extends FullBox {
    static override readonly fourcc = 'uuid' as const;
    static uuid?: string;
  
    constructor(size = 0) {
      super(size);
      this.uuid = (this.constructor as typeof UUIDFullBox).uuid;
    }
  }

  OR

  export function parseOneBox(
    ...
  
    if (headerOnly) {
      return { code: OK, type: type, size: size, hdr_size: hdr_size, start: start };
    } else {
      if (type in BoxRegistry.box) {
        box = new BoxRegistry.box[type](size);
      } else {
        if (type !== 'uuid') {
          Log.warn('BoxParser', `Unknown box type: '${type}'`);
          box = new Box(size);
          box.type = type as BoxFourCC;
          box.has_unparsed_data = true;
        } else {
          if (uuid in BoxRegistry.uuid) {
            box = new BoxRegistry.uuid[uuid](size);
          } else {
            Log.warn('BoxParser', `Unknown UUID box type: '${uuid}'`);
            box = new Box(size);
            box.type = type as BoxFourCC;
            // box.uuid = uuid;
            box.has_unparsed_data = true;
          }
          box.uuid = uuid;
        }

[DenizUgur]

@plantysnake These are really good observations. Thank you. Could you make a separate PR with your proposals? I'd like to keep this PR dedicated to fixing conformance files. Maybe the last commit you did can be moved there as well?

[plantysnake]

Sure thing, will do