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

saiz: add sample_count to box structure #392

Merged
merged 1 commit into from
Oct 29, 2024
Merged

Conversation

bradh
Copy link
Contributor

@bradh bradh commented Mar 10, 2024

This is useful when the default_sample_info_size is used and there are no entries in the sample_info_size array.

Resolves #390.

@paulhiggs
Copy link
Contributor

Alternatively, in `src/parsing/saiz.js' replace

	var count = stream.readUint32();
	this.sample_info_size = [];
	if (this.default_sample_info_size === 0) {
		for (var i = 0; i < count; i++) {
			this.sample_info_size[i] = stream.readUint8();
		}
	}

with

	this.sample_count = stream.readUint32();
	this.sample_info_size = [];
	if (this.default_sample_info_size === 0) {
		for (var i = 0; i < this.sample_count; i++) {
			this.sample_info_size[i] = stream.readUint8();
		}
	}

@rbouqueau
Copy link
Member

Thanks for this. I have no opinion, @DenizUgur or @cconcolato or @jeanlf maybe?

@DenizUgur
Copy link
Member

I have no opinion either, although I'd prefer @paulhiggs's version. Looks cleaner.

@bradh
Copy link
Contributor Author

bradh commented Oct 28, 2024

I'm personally OK with either, just getting one or the other merged would be good.

@DenizUgur
Copy link
Member

Could you revise it like that? I can then merge it.

This is useful when the `default_sample_info_size` is used and
there are no entries in the `sample_info_size` array.
@DenizUgur
Copy link
Member

Thanks

@DenizUgur DenizUgur merged commit 804d4b5 into gpac:master Oct 29, 2024
3 checks passed
@bradh bradh deleted the saiz_count branch October 29, 2024 07:26
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.

saiz box doesn't show sample_count
4 participants