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

fix stco in trailing moov #1056

Merged
merged 1 commit into from Apr 17, 2024

Conversation

philippe44
Copy link
Contributor

This is a fix for https://forums.slimdevices.com/forum/user-forums/logitech-media-server/1699393-some-aac-files-result-in-error-decoder-does-not-support-file-format-code-0#post1699393.

When mp4 files have a trailing moov atom, the stco could be missed and the audio_offset would be wrong. Fix as well the total length check that should be only done once during the first parse

@philippe44 philippe44 changed the title fix stco in bottom moov fix stco in trailing moov Apr 17, 2024
@danielvijge
Copy link
Contributor

I can't say that much about the code quality for a review, but as the original author of the forum post mentioned, I did test this patch, and it does resolve the original issue.

@@ -252,13 +252,13 @@ sub parseStream {
my $offset = $args->{_offset};
my $log = logger('player.streaming');

while (length($args->{_scanbuf}) > $args->{_offset} + $args->{_need} + 8) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this just an optimization, which shouldn't have any impact?...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kind of - using $offset is just easier to read

Copy link
Member

@michaelherger michaelherger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll trust your judgement! Should I back port this to 8.5.2?

@michaelherger michaelherger merged commit 8f0ea8b into LMS-Community:public/9.0 Apr 17, 2024
@philippe44
Copy link
Contributor Author

Yes, absolutely needs to be back ported

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.

None yet

3 participants