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

Jump to bottom

fix(HLS): skip whitespace in attributes #3884

Merged
merged 2 commits into from Feb 11, 2022
Merged

fix(HLS): skip whitespace in attributes #3884

merged 2 commits into from Feb 11, 2022

Conversation

wjywbs
Copy link
Contributor

@wjywbs wjywbs commented Jan 20, 2022

Description

Example manifest:
#EXT-X-STREAM-INF:PROGRAM-ID=1, BANDWIDTH=1000000, RESOLUTION=1920x1080

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to
    not work as expected)
  • This change requires a documentation update

Checklist:

  • I have signed the Google CLA https://cla.developers.google.com
  • My code follows the style guidelines of this project
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • I have verified my change on multiple browsers on different platforms
  • I have run ./build/all.py and the build passes
  • I have run ./build/test.py and all tests pass

@wjywbs
Skip whitespace in HLS attributes.
1292567 
Example:
`#EXT-X-STREAM-INF:PROGRAM-ID=1, BANDWIDTH=1000000, RESOLUTION=1920x1080`
@avelad
Copy link
Collaborator

avelad commented Feb 4, 2022

@joeyparrish can you review it? Thanks!

1 similar comment
@avelad
Copy link
Collaborator

avelad commented Feb 9, 2022

@joeyparrish can you review it? Thanks!

joeyparrish
joeyparrish previously approved these changes Feb 10, 2022
View reviewed changes
Copy link
Member

@joeyparrish joeyparrish left a comment

Choose a reason for hiding this comment

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

Thank you for this, and for adding a test!

@joeyparrish joeyparrish added component: HLS The issue involves Apple's HLS manifest format type: bug Something isn't working correctly labels Feb 10, 2022
@joeyparrish
Copy link
Member

Looks like we merged another change that conflicts with this one. Can you please take a look and add sequenceMode: true to your expected HLS manifest?

      ✗ parses manifest attributes with space
	Error: Expected $.sequenceMode = true to equal false.

@joeyparrish
Copy link
Member

Oh, you'll need to rebase/merge, I expect

@joeyparrish
Add missing sequenceMode in expected HLS manifest
e446f72 
This + a rebase should get tests passing on this PR
@joeyparrish joeyparrish changed the title Skip whitespace in HLS attributes. fix(HLS): skip whitespace in attributes Feb 11, 2022
@joeyparrish
Copy link
Member

I commited a quick fix to your PR branch to add the missing sequenceMode attribute. I'm not sure if that's enough, or if the branch will have to be rebased, as well. If the tests pass, I'll merge it. Thanks!

@joeyparrish joeyparrish merged commit ea6c02a into shaka-project:master Feb 11, 2022
@github-actions github-actions bot mentioned this pull request Feb 12, 2022
Closed
joeyparrish added a commit that referenced this pull request Feb 16, 2022
@wjywbs @joeyparrish
fix(HLS): skip whitespace in attributes (#3884)
6305555 
Example:
`#EXT-X-STREAM-INF:PROGRAM-ID=1, BANDWIDTH=1000000, RESOLUTION=1920x1080`

Co-authored-by: Joey Parrish <joeyparrish@users.noreply.github.com>
@github-actions github-actions bot mentioned this pull request Feb 16, 2022
Merged
joeyparrish added a commit that referenced this pull request Feb 16, 2022
@wjywbs @joeyparrish
fix(HLS): skip whitespace in attributes (#3884)
095dfc7 
Example:
`#EXT-X-STREAM-INF:PROGRAM-ID=1, BANDWIDTH=1000000, RESOLUTION=1920x1080`

Co-authored-by: Joey Parrish <joeyparrish@users.noreply.github.com>
@github-actions github-actions bot mentioned this pull request Feb 16, 2022
Merged
joeyparrish added a commit that referenced this pull request Feb 16, 2022
@wjywbs @joeyparrish
fix(HLS): skip whitespace in attributes (#3884)
4bfc92e 
Example:
`#EXT-X-STREAM-INF:PROGRAM-ID=1, BANDWIDTH=1000000, RESOLUTION=1920x1080`

Co-authored-by: Joey Parrish <joeyparrish@users.noreply.github.com>
joeyparrish added a commit that referenced this pull request Feb 16, 2022
@wjywbs @joeyparrish
fix(HLS): skip whitespace in attributes (#3884)
fda4b80 
Example:
`#EXT-X-STREAM-INF:PROGRAM-ID=1, BANDWIDTH=1000000, RESOLUTION=1920x1080`

Co-authored-by: Joey Parrish <joeyparrish@users.noreply.github.com>
@github-actions github-actions bot mentioned this pull request Feb 16, 2022
Merged
@github-actions github-actions bot mentioned this pull request Mar 3, 2022
Merged
@github-actions github-actions bot mentioned this pull request Mar 22, 2022
Closed
@avelad avelad added this to the v4.0 milestone May 4, 2022
@github-actions github-actions bot added the status: archived Archived and locked; will not be updated label Jul 25, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@joeyparrish joeyparrish joeyparrish approved these changes

Assignees
No one assigned
Labels
component: HLS The issue involves Apple's HLS manifest format status: archived Archived and locked; will not be updated type: bug Something isn't working correctly
Projects
None yet
Milestone
v4.0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@wjywbs @avelad @joeyparrish