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

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

collin
Copy link

@collin collin commented Feb 28, 2024

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
Copy link
Author

collin commented Feb 28, 2024

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;
}

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 this pull request may close these issues.

1 participant