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

DASH UrlTemplate parses template if it contains more than one time ea… #574

Merged
merged 9 commits into from
Oct 2, 2023

Conversation

hugohlln
Copy link
Contributor

With DASH, it's possible to have multiple times each DASH identifier in a template (for example by using a proxy, see Chaos streaming proxy).
This commit allows to compile templates containing more than 4 identifiers (it was not possible before)

@google-cla
Copy link

google-cla bot commented Aug 10, 2023

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@microkatz
Copy link
Contributor

Hello @hugohlln,

Thank you for looking to contribute to ExoPlayer. Before we review your Pull Request, you'll need to sign the Contributor License Agreement (CLA). Thank you!

@hugohlln
Copy link
Contributor Author

hugohlln commented Aug 18, 2023

Hi @microkatz,

Thanks for yout reply.

Sorry, it's done

@brignolij
Copy link

@hugohlln thanks for the fix, will be very helpful.

@sebimag
Copy link

sebimag commented Sep 28, 2023

@hugohlln Wonderful fix this will help my team and allow us to continue working with exoPlayer

@tiagofsky
Copy link

QA will need this, i'm currently struggling with using a proxy with the exoplayer this fix will be very helpful! Thanks @hugohlln

@microkatz
Copy link
Contributor

Hi @hugohlln,

I really appreciate your patience. Also thank you to the other developers for adding your support to this pull request in the comments!

@hugohlln, would you mind adding some unit tests in UrlTemplateTest.java with some segments that have more than one identifier?

@hugohlln
Copy link
Contributor Author

hugohlln commented Sep 28, 2023

@microkatz

Thanks for your reply.

I've just added tests in UrlTemplateTest and also changed the regex to match identifiers with escaped dollars.

All tests are sucessfully running

@microkatz
Copy link
Contributor

I'm going to send this for internal review now. You may see some more commits being added as I make changes in response to review feedback. Please refrain from pushing any more substantive changes as it will complicate the internal review - thanks!

@copybara-service copybara-service bot merged commit b06d823 into androidx:main Oct 2, 2023
1 check passed
oceanjules pushed a commit that referenced this pull request Oct 9, 2023
PiperOrigin-RevId: 570037211
(cherry picked from commit b06d823)
@androidx androidx locked and limited conversation to collaborators Dec 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants