Skip to content

Str set scte pts support#27

Open
saiteja-2000 wants to merge 11 commits into
stoth68000:masterfrom
amagimedia-open:str-set-scte-pts-support
Open

Str set scte pts support#27
saiteja-2000 wants to merge 11 commits into
stoth68000:masterfrom
amagimedia-open:str-set-scte-pts-support

Conversation

@saiteja-2000

Copy link
Copy Markdown

No description provided.

@dheitmueller

Copy link
Copy Markdown
Contributor

I've done some preliminary review of this patch series. I suspect the memory related fixes and the private descriptor in SCTE-35 to 104 conversion probably need to be merged in some form.

The changes to expose a new API to "get the PTS of a SCTE-35" message is something I don't think we want to port. The information is already available and this is simply a helper function of questionable utility.

And the patches to read the test vectors out of TS files are half-baked. They rip out the existing test vectors from tools/parse.c and substitutes it with reading test vectors from a TS file, but it's not like the contributor provided alternative test vectors in TS.

Also, I'm not sure we really want to drag a TS and PES parser into the library, as there are plenty of other tools which already do this. And I definitely don't think it should be included as part of the public API. If somebody really wanted to build an app which extracts SCTE-35 messages from TS files and prints them out, that would be better to have a standalone project which pulls in libdvbpsi/bitstream as well as libklscte35 and does what he/she wants, rather than trying to bury the parts of libdvbpsi/bitstream into the libklscte35 library itself.

In practice, the binaries in the "tools" directory are really just about having test vectors to validate the library functionality. It was never intended for people to add a bunch of misc programs which do other stuff like parse TS files.

My thinking is I should create a branch and cherry-pick in the patches we should merge (ensuring the original author is credited, and then this pull request should be closed).

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.

4 participants