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

Demuxer: emit PAT/PMT when seen #24

Merged
merged 12 commits into from
Apr 17, 2021

Conversation

tmm1
Copy link
Contributor

@tmm1 tmm1 commented Apr 5, 2021

This is a simple implementation of the fix discussed in #20

For now I am simply calling parseData, and only trying to flush early for PAT and PMT. In the future we could use a smarter/cheaper heuristic than a full parseData, and can also expand this to other types of PSI and PES.

Includes #21 and #23. I can rebase this whenever either of those are updated.

cc @barbashov @asticode

@coveralls
Copy link

coveralls commented Apr 5, 2021

Coverage Status

Coverage increased (+0.6%) to 77.156% when pulling 96b3101 on tmm1:packet-pool-accumulator into c44c340 on asticode:master.

Copy link
Owner

@asticode asticode left a comment

Choose a reason for hiding this comment

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

I like the idea of the accumulator (I remember I couldn't find a proper name for this logic but loves accumulator) but I feel some minor changes need to be done.

Also, those changes now have conflicts since I've started merging your PRs.

demuxer.go Outdated Show resolved Hide resolved
demuxer.go Outdated Show resolved Hide resolved
demuxer_test.go Show resolved Hide resolved
demuxer_test.go Show resolved Hide resolved
packet_pool.go Outdated Show resolved Hide resolved
packet_pool.go Outdated Show resolved Hide resolved
packet_pool.go Outdated Show resolved Hide resolved
packet_pool.go Outdated Show resolved Hide resolved
Copy link
Contributor Author

@tmm1 tmm1 left a comment

Choose a reason for hiding this comment

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

I've updated the PR as requested, to:

  • decouple Demuxer and packetPool
  • merge master to fix conflicts
  • improve hexToBytes to allow whitespace
  • add TODO for partial parsing

I plan to remove the parseData in a follow-up PR.

I also had to turn programMap into a pointer so I could pass it down into the pool.

demuxer_test.go Show resolved Hide resolved
demuxer.go Show resolved Hide resolved
packet_pool.go Outdated Show resolved Hide resolved
packet_pool.go Outdated Show resolved Hide resolved
Copy link
Contributor Author

@tmm1 tmm1 left a comment

Choose a reason for hiding this comment

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

Updated as requested.

packet_pool.go Outdated Show resolved Hide resolved
packet_pool.go Outdated Show resolved Hide resolved
@tmm1
Copy link
Contributor Author

tmm1 commented Apr 16, 2021

@asticode how does this look now?

@asticode asticode merged commit 7a46f0b into asticode:master Apr 17, 2021
@asticode
Copy link
Owner

Cheers

@tmm1 tmm1 deleted the packet-pool-accumulator branch April 17, 2021 15:54
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.

3 participants