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 discontinuity field in TS and PES packets #3

Conversation

varsill
Copy link
Contributor

@varsill varsill commented Nov 14, 2023

This PR:

  • adds discontinuity field to the MPEG.TS.PES packets created from a given input buffer, based on the value passed to the push_buffer function along that buffer

@dmorn
Copy link
Member

dmorn commented Nov 14, 2023

Hi @varsill! This is great, the integration with events done on the plugin as well looks way better now. As soon as you're still working on this discontinuity thing, could you please write a test case? Ideally a ts file in test/fixtures and the full processing of it!

@dmorn
Copy link
Member

dmorn commented Nov 14, 2023

If it is easier for you we might just add it to the plugin!

@varsill
Copy link
Contributor Author

varsill commented Nov 14, 2023

Hi @varsill! This is great, the integration with events done on the plugin as well looks way better now. As soon as you're still working on this discontinuity thing, could you please write a test case? Ideally a ts file in test/fixtures and the full processing of it!

Hello, sure, I will do that!

If it is easier for you we might just add it to the plugin!

I will add it to the membrane_mpeg_ts_plugin then, I think such a test will cover more parts of the code. [EDIT] I think it might be better to add test in both places ;)

…ich asserts that the discontinuity flag is properly set. Change Logger.warn into Logger.warning to fix warnings
@varsill varsill marked this pull request as ready for review November 15, 2023 15:27
@dmorn dmorn merged commit 1e1d435 into kim-company:main Nov 20, 2023
1 check failed
@dmorn
Copy link
Member

dmorn commented Nov 20, 2023

Thanks @varsill!

@varsill
Copy link
Contributor Author

varsill commented Nov 21, 2023

Thanks for merging! ;)

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.

2 participants