Skip to content

Conversation

@Luna712
Copy link
Contributor

@Luna712 Luna712 commented Dec 17, 2025

This also requires minSdk 23 (#2078) first.

It also applies all patches for UpdatedDefaultExtractorsFactory and UpdatedMatroskaExtractor from upstream, but converting from Java to Kotlin there is some potential I did it wrong so may need more reviewing or testing.

@Luna712 Luna712 marked this pull request as ready for review December 18, 2025 22:38
Copy link
Collaborator

@fire-light42 fire-light42 left a comment

Choose a reason for hiding this comment

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

This fails on the videos we tried to patch with our UpdatedMatroskaExtractor. See sample_three_seekheads.mkv and sample_recursive_seekhead.mkv.

@Luna712
Copy link
Contributor Author

Luna712 commented Dec 20, 2025

This fails on the videos we tried to patch with our UpdatedMatroskaExtractor. See sample_three_seekheads.mkv and sample_recursive_seekhead.mkv.

Nice thanks for samples I was wondering how to test that as yeah wasn't sure. Will look.

@fire-light42
Copy link
Collaborator

This fails on the videos we tried to patch with our UpdatedMatroskaExtractor. See sample_three_seekheads.mkv and sample_recursive_seekhead.mkv.

Nice thanks for samples I was wondering how to test that as yeah wasn't sure. Will look.

It is perhaps better to push our changes upstream and wait for 1.9.1. However the review team at android is really bad, and only exists to bikeshed. So it is a 50/50 if that is even viable.

Missed some upstream fixes in rollback commits
@Luna712
Copy link
Contributor Author

Luna712 commented Dec 20, 2025

This fails on the videos we tried to patch with our UpdatedMatroskaExtractor. See sample_three_seekheads.mkv and sample_recursive_seekhead.mkv.

@fire-light42

Should be fixed, tested with those files now, some changes in rollback commits weren't applied (I assumed rollback was only rollback and not other random changes also so didn't check to apply those)

This fails on the videos we tried to patch with our UpdatedMatroskaExtractor. See sample_three_seekheads.mkv and sample_recursive_seekhead.mkv.

Nice thanks for samples I was wondering how to test that as yeah wasn't sure. Will look.

It is perhaps better to push our changes upstream and wait for 1.9.1. However the review team at android is really bad, and only exists to bikeshed. So it is a 50/50 if that is even viable.

@fire-light42

Seeing as how a PR already exists for it upstream and no progress on it perhaps we should still do this for now? As I really am not confident it will be merged by 1.9.1.

@Luna712 Luna712 requested a review from fire-light42 December 20, 2025 17:54
@Luna712
Copy link
Contributor Author

Luna712 commented Dec 20, 2025

@fire-light42 just to mention it since you approved this now, #2078 should be merged first, this also bumps minSdk just to make it pass but the other PR also does useLegacyPackaging which without causes the APK size to pretty much double.

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.

2 participants