Skip to content

Conversation

@gBillal
Copy link
Collaborator

@gBillal gBillal commented Jan 14, 2025

Follow up of #25

end

test "scale video frame", %{frame_480p: frame_480p} do
converter = Xav.VideoConverter.new!(width: 368)
Copy link
Contributor

Choose a reason for hiding this comment

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

check incorrrect width and height

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Should we add validation on the elixir code ? since we don't initialize the converter on creation ffmpeg won't do any validation until you first try to convert a frame.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I would do this. Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

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

Just this thing and we are ready to merge

@gBillal
Copy link
Collaborator Author

gBillal commented Jan 19, 2025

@mickel8 since we're raising in the nif on invalid input, I deleted the new!/1 function and made the new/1 returns the converter instead of {:ok, converter}.

@gBillal gBillal requested a review from mickel8 January 19, 2025 18:56
@mickel8 mickel8 merged commit 56253a4 into elixir-webrtc:master Jan 19, 2025
@mickel8
Copy link
Contributor

mickel8 commented Jan 19, 2025

Thanks!

@mickel8
Copy link
Contributor

mickel8 commented Jan 19, 2025

@gBillal looks like somethings failes on macos-arm. Do you have any means to check what's wrong? It looks like scaled binary does not match 🤔

@gBillal gBillal deleted the support-scaling branch January 19, 2025 22: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