Add encoder #29
Add encoder #29
Conversation
|
Awesome! I will try to take a look tomorrow |
| @@ -0,0 +1,19 @@ | |||
| defmodule Xav.Packet do | |||
There was a problem hiding this comment.
Maybe it would be better to go for Frame and RawFrame? WDYT? I have networking background and for me packet is strictly related to network protocols.
There was a problem hiding this comment.
I'm trying to follow ffmpeg naming, even for the fields. RawFrame will also work.
| @doc """ | ||
| Create a new encoder. | ||
|
|
||
| It accepts the following options:\n#{NimbleOptions.docs(@video_encoder_schema)} |
There was a problem hiding this comment.
Would be nice to either use NimbleOptions everywhere or nowhere. Alternatively we can make an update in another PR
There was a problem hiding this comment.
Yes I'll do that too, I want just to focus on the encoder for now. I'll update the converter and the other modules if needed.
For now, after the encoder, I want to look at the subject of hardware accelerated encoding/decoding. |
|
@gBillal you should have write permissions. In particular, feel free to merge PR without my revie |
|
|
||
| defp to_packets(result) do | ||
| Enum.map(result, fn {data, dts, pts, keyframe?} -> | ||
| %Xav.Packet{data: data, dts: dts, pts: pts, keyframe?: keyframe?} |
There was a problem hiding this comment.
I wonder whether to return a list of packets or merge them all into a single packet/frame?
Do you know why ffmpeg outputs multiple packets? How are they divided?
There was a problem hiding this comment.
Actually we cannot merge them together. A packet represent a single video frame (in the case of audio it may be multiple samples in the same packet), a packet has properties specific to a single frame (pts, dts and keyframe?). Also some containers/decoders requires a single packet as the unit to work on with.
There was a problem hiding this comment.
Is this then expected that encoding a single raw frame can result in multiple output packets?
There was a problem hiding this comment.
Yes it's possible, usually the encoder will buffer packets to output them in decoding order which may be different than presentation order.
I think it's also better code wise, the same code is used for encoding and flushing and it's easier to include the encoder in elixir streams/enum rather than handling nil or {:ok, packet}.
There was a problem hiding this comment.
Perfect, thanks! If you have time, it would be nice to mention this in the docs
Yes I have access but I cannot push a hex package or should I leave release creation to you ? |
|
Yeah, let's leave it to me or ping me when it's needed |
A first version of a video encoder (
h264andhevc).@mickel8 you can start reviewing it in case you have a different architecture in mind.