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 h265 tests #55

Merged
merged 6 commits into from
Jan 16, 2024
Merged

Add h265 tests #55

merged 6 commits into from
Jan 16, 2024

Conversation

gBillal
Copy link
Contributor

@gBillal gBillal commented Jan 5, 2024

No description provided.

@gBillal gBillal requested a review from varsill as a code owner January 5, 2024 21:30
Copy link
Contributor

@varsill varsill left a comment

Choose a reason for hiding this comment

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

Thank you very much for the contribution!
I have spotted some minor things that might be worth changing.

@@ -28,7 +28,7 @@ defmodule Membrane.H264.StreamFormatTest do
}

defp perform_test(filename, timeout) do
in_path = Path.expand("../fixtures/input-#{filename}.h264", __DIR__)
in_path = Path.expand("../../fixtures/h264/input-#{filename}.h264", __DIR__)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can also change it in the following way since you have already done it in a few other places:

Suggested change
in_path = Path.expand("../../fixtures/h264/input-#{filename}.h264", __DIR__)
in_path = Path.expand("test/fixtures/h264/input-#{filename}.h264", __DIR__)

do_prepare_bytestream_buffers(binary)
end

def prepare_buffers(binary, mode, output_stream_structure, stable_reprefixing?) do
Copy link
Contributor

@varsill varsill Jan 9, 2024

Choose a reason for hiding this comment

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

[NIT] I would think of explicitly calling that function prepare_h264_buffers rather than having prepare_buffers and prepare_h265_buffers

@varsill varsill self-assigned this Jan 10, 2024
@gBillal gBillal requested a review from varsill January 10, 2024 15:48
Copy link
Contributor

@varsill varsill left a comment

Choose a reason for hiding this comment

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

Great, thank you very much for your contribution! Soon we will release a new version of a plugin and change the name to membrane_h26x_plugin

@varsill varsill merged commit 205ac35 into membraneframework:master Jan 16, 2024
@gBillal gBillal deleted the add-h265-tests branch January 16, 2024 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants