Conversation
| /* 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; | ||
|
|
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
"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.
There was a problem hiding this comment.
I agree that it was assumed by virtue of the fact that all files were necessarily m4v files. Now we're giving folks an option on what to name the files, which implicitly allows them to also adjust the format - and we're giving them an option to pipe everything to stdout, but forcing them to use the ipod format when going to stdout.
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 stdout, and change the filename.
[It's also worth noting that simply having the ability to send to stdout means that you could also pipe it back into ffmpeg and reformat it that way, but you may start to lose the fidelity of the video that way due to lossy encodings, etc.]
|
@grintor You may need to rebase this on top of the current |
|
Ok this latest commit closes GUACAMOLE-974 in addition to the GUACAMOLE-1370 the original pull request by adding your requested feature. I also noticed that my original pull request closes GUACAMOLE-973 as well. |
Added a -o option to guacenc allowing the output path to be specified explicitly. It defaults to FILE.m4v as before when unspecified for backwards compatibility. Specifying "-" as the output path streams the encoded video to stdout.
Added a -c option selecting the libavcodec encoder (default "mpeg4") and a -F option selecting the libavformat container, mirroring ffmpeg's -c:v and -f. The container is otherwise guessed from the output filename's extension, or defaults to "ipod" when streaming to a pipe, which has no extension. Generated output filenames use the chosen container's conventional extension, and unknown format names are rejected before any encoding begins. Restored the refusal to overwrite existing output files, which was lost in the migration to libavformat but is still documented in the man page.
|
Ok the commits are on top of the current main branch now. |
|
I built b248d24 on Ubuntu 24.04 (FFmpeg 6.1.1, libvpx 1.14.0) and ran it against a small synthetic recording and a 1280x720 one. Everything below was checked with ffprobe:
One thing I ran into: The man page does say the codec has to fit the container, so I don't think this needs to hold up the PR. Once it's merged I'd be glad to send a small follow-up under GUACAMOLE-2298 that picks the default extension from the codec when neither Separately, and the same in 1.6.0: guacenc exits 0 even when every file fails (bad codec, codec/container mismatch, refusing to overwrite). That matters a bit more now that output can go to a pipe. I can open an issue for it if it's worth changing. Thanks for adding the format option. It covers most of what I filed GUACAMOLE-2298 for. |
|
Good call on the exit code @vlouvet Sorry to duplicate your work on GUACAMOLE-2298 I hadn't seen that. |
3 participants
Added a -o option to guacenc allowing the output path to be specified explicitly. It defaults to FILE.m4v as before when unspecified for backwards compatibility. Specifying "-" as the output path streams the encoded video to stdout. Normal MP4 (which is what's being used for file output still) doesn't support streaming, so we use fragmented MP4 here when outputting to stdout. This paves the way to streaming that stdout to a video player or even the browser in real time as it's being encoded.