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 HEVC muxing support for CMAF #99

Merged
merged 2 commits into from
Feb 9, 2024

Conversation

gBillal
Copy link
Contributor

@gBillal gBillal commented Nov 16, 2023

No description provided.

@gBillal gBillal requested a review from mat-hek as a code owner November 16, 2023 15:42
Copy link
Member

@mat-hek mat-hek left a comment

Choose a reason for hiding this comment

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

Awesome 😎 one thing and we can merge

Comment on lines 330 to 332
defp key_frame?(%{metadata: %{h264: %{key_frame?: true}}}), do: true
defp key_frame?(%{metadata: %{h265: %{key_frame?: true}}}), do: true
defp key_frame?(_sample), do: false
Copy link
Member

Choose a reason for hiding this comment

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

Seems like this kind of function is repeated many times, maybe let's add a codec_helper.ex and put it there? I guess it should accept just the metadata to be most flexible.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added the function to the already existing Membrane.MP4.Helper module.

@mat-hek
Copy link
Member

mat-hek commented Nov 28, 2023

@gBillal thanks, next time don't forget to refresh my review, otherwise I don't know the PR's ready ;)

@mat-hek mat-hek merged commit 0815427 into membraneframework:master Feb 9, 2024
3 checks passed
@gBillal gBillal deleted the hevc-cmaf branch February 9, 2024 11:33
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