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

Add support for multiple SCTE-35 tag syntax #60

Closed
bradleyfalzon opened this issue May 31, 2016 · 8 comments
Closed

Add support for multiple SCTE-35 tag syntax #60

bradleyfalzon opened this issue May 31, 2016 · 8 comments
Assignees

Comments

@bradleyfalzon
Copy link
Collaborator

bradleyfalzon commented May 31, 2016

There appear to be various non-standard implementations of SCTE-35 integration with HLS.

The recent draft of HLS now has an official method to support SCTE-35 tags as well as the cue points. See: https://www.ietf.org/rfcdiff?url1=draft-pantos-http-live-streaming-18&url2=draft-pantos-http-live-streaming-19

I'd like this library to support the new standard, but #40 and #41 introduced support for a different standard. There's also at least two other formats I've seen.

I think we need a method to read multiple standards, store and manipulate them consistently and write multiple standards. With the focus being on the HLS RFC's method, and the existing format already supported by this library - but this may require some breaking changes.

This issue is open to discuss this.

Other support that could be added later: http://wwwimages.adobe.com/content/dam/Adobe/en/devnet/primetime/pdfs/PrimetimeDigitalProgramInsertionSignalingSpecification1_1.pdf

@bradleyfalzon
Copy link
Collaborator Author

@vishal24tuniki your thoughts on this? As I believe you're using the changes in #40 and #41 ?

@vishal24tuniki
Copy link
Contributor

@bradleyfalzon Yes i am using changes in #40 and #41
I will definitely go through the documentation and come back to you. Been pretty busy the last month or 2

@bradleyfalzon
Copy link
Collaborator Author

Thanks, I'd like to maintain ABI, and considering there's a few different SCTE formats, a short term pain by coming up with a more flexible interface maybe a better idea. Although, I think we can reuse much of the existing getters and setters.

@bradleyfalzon bradleyfalzon self-assigned this May 31, 2016
@bradleyfalzon
Copy link
Collaborator Author

Ideally, we'd be able to read all types of SCTE-35 formats, manipulate them with a single API internally and be able to write in all formats but having the user select which one (by default using HLS RFC EXT-X-DATERANGE method).

To do this properly, it's likely we'd break the existing functionality, but I haven't thought it through, how it would break it, or how long we can maintain the old functionality.

See #61 for the format I initially need to add support for.

This is still an ongoing issue, no changes will be made yet.

@bradleyfalzon
Copy link
Collaborator Author

bradleyfalzon commented Jun 8, 2016

OK, my initial proposal is in #61 - @vishal24tuniki this has a few breaking changes, I'm not convinced it'll be merged as is, but if this doesn't break your use of the library and @grafov is OK with it, I'd like to merge it as is (with the breaking changes).

The breaking changes aren't important, so happy either way, it's just renaming SCTE to SCTE35, but it is more correct and specific.

The new support should continue to work with existing SCTE-35 formats, but if you (@vishal24tuniki) could switch to the the newer SetSCTE35() method (if you're using SetSCTE() at all), that'd be ideal. See the source for SetSCTE for the necessary changes.

@bradleyfalzon bradleyfalzon changed the title Add support for EXT-X-DATERANGE and SCTE-35 Tags Add support for multiple SCTE-35 tags Jun 8, 2016
@bradleyfalzon bradleyfalzon changed the title Add support for multiple SCTE-35 tags Add support for multiple SCTE-35 tag syntax Jun 8, 2016
@bradleyfalzon
Copy link
Collaborator Author

bradleyfalzon commented Jun 21, 2016

We're using this revision quite successfully, and I'd like to consider merging these changes. If anyone wants more time to review and provide feedback that's fine, really I just want to know whether the backwards incompatible changes are acceptable, and if not (that's fine it's just renaming SCTE to SCTE35), we'll just remove those.

@bradleyfalzon
Copy link
Collaborator Author

bradleyfalzon commented Aug 3, 2016

@vishal24tuniki do you have any further thoughts on this, I'm not going to merge just yet, but I would like to apply to breaking change (renaming of fields and changing parameters), but it's also not 100% important to do either, so happy to reconsider it.

@bradleyfalzon
Copy link
Collaborator Author

@vishal24tuniki I'd love if you have a chance to check out #81, I've removed the breaking changes as it's taken too long to merge now. So I don't think you'll have any issues, but I have deprecated some functions that you'll need to switch over one day.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants