Skip to content

Disable udta Parsing#233

Merged
Kevin-McGonigle merged 1 commit intomasterfrom
kev/disable-udta-parsing
Apr 3, 2023
Merged

Disable udta Parsing#233
Kevin-McGonigle merged 1 commit intomasterfrom
kev/disable-udta-parsing

Conversation

@Kevin-McGonigle
Copy link
Copy Markdown
Contributor

The User Data (udta) ISOBMFF box is currently treated as a standard container box, expecting other ISOBMFF boxes within. However, their contents don't always necesarily conform to the standard, resulting in some incorrect parsing, corrective buffer position updates and warning logs. This PR disables udta box parsing, which will skip parsing their contents.

Copy link
Copy Markdown
Contributor

@RubenWolff-wt RubenWolff-wt left a comment

Choose a reason for hiding this comment

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

lgtm

I have no idea what exactly that change does but it matches your description :)

@Kevin-McGonigle
Copy link
Copy Markdown
Contributor Author

lgtm

I have no idea what exactly that change does but it matches your description :)

If you're interested, head to peek-worker logs and you'll see a bunch that look like:

Unexpected IO position after parsing udta box at position X. Box size: Y. Expected position: X + Y. Actual position: Z.

Basically, we try to treat the contents of the udta box as if it confomed to the standard, but it often doesn't, so our parser freaks out and ends up in a weird state. We have a fail-safe built in that resets the buffer to the correct position and generates that log warning when something like this happens - but it's happening pretty often for udta boxes and polluting the logs a bit. So this PR basically stops treating the udta box as a container, which was prompting the parser to attempt to parse its contents, and now treats it as an unknown box, which will now just note the existence of the box without diving inside it.

Copy link
Copy Markdown
Contributor

@linkyndy linkyndy left a comment

Choose a reason for hiding this comment

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

Appreciate the nice explanation as well 🙌

@Kevin-McGonigle Kevin-McGonigle merged commit fbf9773 into master Apr 3, 2023
@Kevin-McGonigle Kevin-McGonigle deleted the kev/disable-udta-parsing branch April 3, 2023 11:21
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.

3 participants