-
Notifications
You must be signed in to change notification settings - Fork 782
GUACAMOLE-1370: guacenc output to stdout. #687
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,6 +29,7 @@ | |
| #include <libavformat/avformat.h> | ||
| #endif | ||
| #include <libavutil/common.h> | ||
| #include <libavutil/dict.h> | ||
| #include <libavutil/imgutils.h> | ||
| #include <libswscale/swscale.h> | ||
| #include <guacamole/client.h> | ||
|
|
@@ -41,6 +42,7 @@ | |
| #include <errno.h> | ||
| #include <fcntl.h> | ||
| #include <inttypes.h> | ||
| #include <stdbool.h> | ||
| #include <stdlib.h> | ||
| #include <string.h> | ||
| #include <unistd.h> | ||
|
|
@@ -54,8 +56,16 @@ guacenc_video* guacenc_video_alloc(const char* path, const char* codec_name, | |
| int ret; | ||
| int failed_header = 0; | ||
|
|
||
| /* The container format cannot be guessed from a pipe URL and must be | ||
| * specified explicitly. The "ipod" container is used, matching the | ||
| * container that would be guessed from the ".m4v" extension of the output | ||
| * files normally produced */ | ||
| bool is_pipe = (strncmp(path, "pipe:", 5) == 0); | ||
| const char* format_name = is_pipe ? "ipod" : NULL; | ||
|
|
||
|
Comment on lines
+59
to
+65
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this an appropriate assumption to make universally, or should this be configurable, as well? Are there situations where users may want to stream in a format different than iPod?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "ipod" was already an assumption being made universally before, just not explicitly. When writing to a file, the third argument to avformat_alloc_output_context2() is NULL so libavformat guesses the container from the extension of the output path. Since that was hardcoded to "FILE.m4v" it was always making the same guess based on ".m4v" -- 'ipod'. This is a pretty universally supported format, even in web browsers, so I think it's a good default. Maybe someone will want it to be configurable in the future, but we can just wait for that feature request.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree that it was assumed by virtue of the fact that all files were necessarily I'm not going to block this PR from going through if you don't want to do this, it's just something that seems to be in the same spirit as the ability to send it to [It's also worth noting that simply having the ability to send to |
||
| /* allocate the output media context */ | ||
| avformat_alloc_output_context2(&container_format_context, NULL, NULL, path); | ||
| avformat_alloc_output_context2(&container_format_context, NULL, | ||
| format_name, path); | ||
| if (container_format_context == NULL) { | ||
| guacenc_log(GUAC_LOG_ERROR, "Failed to determine container from output file name"); | ||
| goto fail_codec; | ||
|
|
@@ -130,8 +140,17 @@ guacenc_video* guacenc_video_alloc(const char* path, const char* codec_name, | |
| } | ||
| } | ||
|
|
||
| /* A pipe cannot be seeked backwards to finalize the container once | ||
| * encoding is complete, so fragmented MP4 must be used to produce a | ||
| * usable video stream */ | ||
| AVDictionary* header_options = NULL; | ||
| if (is_pipe) | ||
| av_dict_set(&header_options, "movflags", | ||
| "frag_keyframe+empty_moov", 0); | ||
|
|
||
| /* write the stream header, if needed */ | ||
| ret = avformat_write_header(container_format_context, NULL); | ||
| ret = avformat_write_header(container_format_context, &header_options); | ||
| av_dict_free(&header_options); | ||
| if (ret < 0) { | ||
| guacenc_log(GUAC_LOG_ERROR, "Error occurred while writing output file header."); | ||
| failed_header = true; | ||
|
|
@@ -163,8 +182,9 @@ guacenc_video* guacenc_video_alloc(const char* path, const char* codec_name, | |
| fail_output_file: | ||
| avio_close(container_format_context->pb); | ||
|
|
||
| /* Delete the file that was created if it was actually created */ | ||
| if (unlink(path) == -1 && errno != ENOENT) | ||
| /* Delete the file that was created if it was actually created (no file | ||
| * exists to be deleted if output was written to a pipe) */ | ||
| if (!is_pipe && unlink(path) == -1 && errno != ENOENT) | ||
| guacenc_log(GUAC_LOG_WARNING, "Failed output file \"%s\" could not " | ||
| "be automatically deleted: %s", path, strerror(errno)); | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.