gpac / mp4box.js

JavaScript version of GPAC's MP4Box tool
https://gpac.github.io/mp4box.js/
BSD 3-Clause "New" or "Revised" License
1.89k stars 318 forks source link

Adds check to emit fragments on keyframe boundaries when rapAlignement is set #388

Open collin opened 4 months ago

collin commented 4 months ago

Related to #30.

I'm not sure how to approach adding a test for this because the .mp4 files are not present in the repository, but this has been successful for me.

I do think that it might be better to make the nb_samples and rapAlignment options mutually exclusive.

collin commented 4 months ago

Digging through a bit more I've observed some other issues with fragmentation/configuration.

Something like this would make emitting a segment dependent on either rapAlignement or nb_samples, but not both.

const nextSample = trak && trak.samples[trak.nextSample];
const emitSegment =
  // if rapAlignement is true, we emit a fragment when we have a rap sample coming up next
  (fragTrak.rapAlignement === true && (nextSample && nextSample.is_sync)) ||
  // if rapAlignement is false, we emit a fragment when we have the required number of samples
  (!fragTrak.rapAlignement &&
    trak.nextSample % fragTrak.nb_samples === 0) ||
  // if this is the last sample, we emit the fragment
  last ||
  // if we have more samples than the number of samples requested, we emit the fragment
  trak.nextSample >= trak.samples.length;

Additionally, it's currently not possible to set the rapAlignment option to false.

This code, in ISOFile.prototype.setSegmentOptions is setting the option. However, if options.rapAlignement is false, the conditional will never execute, and rapAlignement will keep its default value, true.

if (options) {
    if (options.nbSamples) fragTrack.nb_samples = options.nbSamples;
    if (options.rapAlignement) fragTrack.rapAlignement = options.rapAlignement;
}

Something like this would be better, as it only checks for the presence of a key, and will return true even if value is false.

if (options) {
    if ("nbSamples" in options) fragTrack.nb_samples = options.nbSamples;
    if ("rapAlignement" in options) fragTrack.rapAlignement = options.rapAlignement;
}