Add egress image output #457
Conversation
…to benjamin/egress_image_output
livekit_egress.proto
Outdated
|
|
||
| message ImageOutput { | ||
| uint32 capture_interval = 1; // in seconds (required) | ||
| int32 width = 2; // (required) |
There was a problem hiding this comment.
could we default to either Track dimensions (for track/participant composite), or a fixed size? would simplify usage.
| string filename_prefix = 4; // (optional) | ||
| ImageFileSuffix filename_suffix = 5; // (optional, default INDEX) | ||
| ImageCodec image_codec = 6; // (optional) | ||
| bool disable_manifest = 7; // disable upload of manifest file (default false) |
There was a problem hiding this comment.
will images have a separate manifest too?
There was a problem hiding this comment.
Good question. Currently, we upload one (identical) manifest per output (segment + file). For consistency, images should have their own, if we want a manifest for images.
There is a bit of a question of what a manifest for images should be. One option would be a manifest file uploaded at the end of the session with a summary of the session, similar to what we do for file/segments. More useful for images may be something closer to a m3u8 playlist: a file updated after each new image is uploaded that provides a list of all image filenames with their pts/time of the day. My initial idea when writing this proto file was the latter.
There was a problem hiding this comment.
thinking out loud. per image manifest would allow them to identify the JPEGs in near-real-time, as opposed to waiting until it's over.
livekit_models.proto
Outdated
| @@ -57,6 +57,11 @@ enum VideoCodec { | |||
| VP8 = 4; | |||
| } | |||
|
|
|||
| enum ImageCodec { | |||
| DEFAULT_IC = 0; | |||
There was a problem hiding this comment.
IC prefix is preferred
IC_DEFAULT
IC_JPEG
There was a problem hiding this comment.
Agreed that a prefix is cleaner. This is inconsistent with the current audio and video codec definitions though:
enum AudioCodec {
DEFAULT_AC = 0;
OPUS = 1;
AAC = 2;
}
enum VideoCodec {
DEFAULT_VC = 0;
H264_BASELINE = 1;
H264_MAIN = 2;
H264_HIGH = 3;
VP8 = 4;
}
Do we mind?
There was a problem hiding this comment.
I think @boks1971 proposed prefixes after some of these are already in-use. So I think we can keep the newly adopted patterns without going back and changing everything else; since doing so may cause some breakage.
…to benjamin/egress_image_output
biglittlebigben
force-pushed
the
benjamin/egress_image_output
branch
from
b5217d6
to
4bcfcf7
Compare
biglittlebigben
force-pushed
the
benjamin/egress_image_output
branch
from
14bc77f
to
ff399a8
Compare
| @@ -258,6 +286,12 @@ message UpdateStreamRequest { | |||
| repeated string remove_output_urls = 3; | |||
| } | |||
|
|
|||
| message UpdateOutputsRequest { | |||
There was a problem hiding this comment.
wdyt?
| message UpdateOutputsRequest { | |
| message UpdateImageOutputsRequest { |
There was a problem hiding this comment.
My initial logic was that this message could be eventually used to update other kinds of output. If we don't forsee this happening, sure.
There was a problem hiding this comment.
ah understood.. yeah if we are going to use it for other outputs.. then what we have is better
| @@ -36,6 +36,9 @@ service Egress { | |||
| // add or remove stream endpoints | |||
| rpc UpdateStream(UpdateStreamRequest) returns (EgressInfo); | |||
|
|
|||
| // add or remove outputs | |||
| rpc UpdateOutputs(UpdateOutputsRequest) returns (EgressInfo); | |||
There was a problem hiding this comment.
| rpc UpdateOutputs(UpdateOutputsRequest) returns (EgressInfo); | |
| rpc UpdateImageOutputs(UpdateOutputsRequest) returns (EgressInfo); |
No description provided.