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.

Sign up for GitHub

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

Jump to bottom

Transcoding #40

Open
wants to merge 38 commits into
base: master
Choose a base branch
from
Open

Transcoding #40

wants to merge 38 commits into from

Conversation

FelonEkonom
Copy link
Member

@FelonEkonom FelonEkonom commented Sep 25, 2024 •
edited
Loading

Closes #19

@FelonEkonom FelonEkonom changed the title Transcoding v2 Transcoding Sep 25, 2024
@FelonEkonom FelonEkonom self-assigned this Sep 25, 2024
@FelonEkonom FelonEkonom mentioned this pull request Sep 25, 2024
Closed
mat-hek
mat-hek reviewed Sep 30, 2024
View reviewed changes
lib/boombox/mp4.ex Outdated
alias Membrane.H264
alias Membrane.H265

defguardp is_h264_or_h265(format) when is_struct(format) and format.__struct__ in [H264, H265]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
defguardp is_h264_or_h265(format) when is_struct(format) and format.__struct__ in [H264, H265]
defguardp is_h26x(format) when is_struct(format) and format.__struct__ in [H264, H265]

lib/boombox/mp4.ex Outdated
Comment on lines 89 to 90
_not_h264 ->
%H264{stream_structure: :avc3, alignment: :au}
Copy link
Member

Choose a reason for hiding this comment

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

what about H265?

Copy link
Member Author

Choose a reason for hiding this comment

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

@varsill what types of H265 can be put inside MP4 and what types of conversion should be avoided there?

Copy link
Contributor

@varsill varsill Oct 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

You can put there :hvc1 and :hev1 and you need to treat :hvc1 just as you treat :avc1 and :hev1 as :avc3

lib/boombox/transcoders/video.ex Outdated
@@ -0,0 +1,161 @@
defmodule Boombox.Transcoders.Video do
Copy link
Member

Choose a reason for hiding this comment

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

WDYT about merging this with the Audio transcoder bin?

Copy link
Member Author

Choose a reason for hiding this comment

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

Now the way how each of transcoder is built is slightly different, I don't see much repeated code there

@FelonEkonom
Copy link
Member Author

FelonEkonom commented Nov 5, 2024 •
edited
Loading

@mat-hek this is ready to review

mat-hek
mat-hek approved these changes Nov 6, 2024
View reviewed changes
lib/boombox/transcoder/audio.ex Outdated
@@ -0,0 +1,91 @@
defmodule Boombox.Transcoder.Audio do
@moduledoc false
use Membrane.Bin
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
use Membrane.Bin

lib/boombox/webrtc.ex Outdated
@@ -3,10 +3,16 @@ defmodule Boombox.WebRTC do

import Membrane.ChildrenSpec
require Membrane.Pad, as: Pad
alias Boombox.Pipeline.{Ready, Wait}
alias Boombox.Pipeline.{Ready, State, Wait}
# alias Boombox.Transcoders
Copy link
Member

Choose a reason for hiding this comment

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

leftover

test/test_helper.exs Outdated
@@ -1 +1,2 @@
ExUnit.start(capture_log: true)
# ExUnit.start()
Copy link
Member

Choose a reason for hiding this comment

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

leftover

test/boombox_test.exs Outdated
Comment on lines 34 to 35
# System.shell("cp #{output} test/fixtures/ref_bun10s_h265.mp4")
# System.cmd("cp", [output, "test/fixtures/ref_bun10s_h265.mp4"])
Copy link
Member

Choose a reason for hiding this comment

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

leftover

test/boombox_test.exs Outdated
@@ -25,6 +26,16 @@ defmodule BoomboxTest do
Compare.compare(output, "test/fixtures/ref_bun10s_aac.mp4")
end

@tag :xd
Copy link
Member

Choose a reason for hiding this comment

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

leftover

mix.exs
Comment on lines +50 to +52
:membrane_webrtc_plugin,
github: "membraneframework/membrane_webrtc_plugin",
branch: "support-various-video-codecs-in-source"
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 needs some attention membraneframework/membrane_webrtc_plugin#11

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@varsill varsill varsill left review comments

@mat-hek mat-hek mat-hek approved these changes

Assignees

@FelonEkonom FelonEkonom

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Automatically apply transcoding when necessary
3 participants
@FelonEkonom @varsill @mat-hek