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.

Sign up for GitHub

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

Add Interstitial Tag Support #144

Merged
merged 13 commits into from
Nov 1, 2024
Merged

Add Interstitial Tag Support #144

merged 13 commits into from
Nov 1, 2024

Conversation

rmigneco
Copy link
Collaborator

@rmigneco rmigneco commented Oct 23, 2024 •
edited
Loading

Description

This PR adds support for Interstitial Tag Attributes as outlined in the Appendix D of the HLS Spec

Change Notes

  • Adds PantosValue attributes to support interstitials
  • Updates the EXT_X_DATERANGETagValidator to include additional values and include specific validation rules for HLS Interstitials
  • Adds unit tests for the new validation rules
  • Creates HLSInterstitialValues to represent the attribute options
  • Creates InterstitialTagBuilder which facilitates creating a HLSTag type configured specifically for interstitials
  • Unit tests for InterstitialTagBuilder

Pre-submission Checklist

  • I ran the unit tests locally before checking in.
  • I made sure there were no compiler warnings before checking in.
  • I have written useful documentation for all public code.
  • I have written unit tests for this new feature.

@rmigneco
Added PantosValue for Interstitial Tags
7dbeace
@rmigneco
Added Validation Issues for HLS Interstitial tags
a2c61ef 
- checking that with HLS CLASS attribute, only asset list or id can be accepted
@rmigneco
Added Special Value types for interstital attributes
d6ea03b 
- completed the Generic Tag validator for Date Range
@rmigneco
Added additional date range validation tests
63e3743
@rmigneco
Added Interstitial Value Tests
60ef9c6 
- testing seek restrictions and alignment
- renamed Interstitial Alignment
@rmigneco
Added interstitial tag builder
a94bf97 
- included required initializers
- included builder functions
@rmigneco
Changed visibility of Dateformatter so they could be re-use
76a6b32
@rmigneco
Added discardable result to builder functions
9b66030 
- completed tag return in build func
- added documentation
@rmigneco
Added Tag Builder Tests
47617fa 
- ensure HLS Interstitial Value types conform to equatable
@rmigneco
Removed TODO comment
1b55efd
theRealRobG
theRealRobG reviewed Oct 25, 2024
View reviewed changes
Copy link
Collaborator

@theRealRobG theRealRobG left a comment

Choose a reason for hiding this comment

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

Looks good! Just some small comments and questions.

mambaSharedFramework/HLSInterstitialValueTypes.swift Outdated Show resolved Hide resolved
mambaSharedFramework/InterstitialTagBuilder.swift Outdated Show resolved Hide resolved
mambaSharedFramework/InterstitialTagBuilder.swift Show resolved Hide resolved
mambaSharedFramework/InterstitialTagBuilder.swift Outdated Show resolved Hide resolved
@rmigneco
Updated Snap enum case names
89b1726 
- updated affected tests
@rmigneco
Trim whitespace
c7f6484
@rmigneco
Added plannedDuration to interstitial tag builder
c3fff36 
- fixed qutoe escape issues for numerical values in builder
- upated unit tests
@rmigneco
Copy link
Collaborator Author

Looks good! Just some small comments and questions.

@theRealRobG got your changes incorporated - LMK if you'd like to get this into 2.0 as well

@rmigneco rmigneco merged commit 9ec6fcf into develop_1.x Nov 1, 2024
4 checks passed
@rmigneco rmigneco deleted the interstitial-tags branch November 1, 2024 13:44
@rmigneco rmigneco mentioned this pull request Nov 1, 2024
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@theRealRobG theRealRobG theRealRobG approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@rmigneco @theRealRobG