On Mon, Nov 20, 2017 at 10:35:39 +0000, Matteo Naccari wrote:
I don't recall all of the previous review remarks, but these may be
new:
> LICENSE.md | 1 +
> configure | 6 +
Please add a changelog entry.
> librubberband
> + libturing
> libvidstab
You used a tab instead spaces.
> + #if defined(_MSC_VER)
> +#define TURING_API_IMPORTS 1
> +#endif
Stray whitespace in front of "#if".
> + int error_code = 0;
> + int i = 0;
I believe these initializations are never used.
> + if (error_code = add_option("turing", &encoder_options)) {
> + goto fail;
> + }
This assignment in an if() clause will give a compiler warning if you
don't add an extra set of brackets around the assingment.
Preferred method is actually:
error_code = add_option("turing", &encoder_options);
if (error_code) {
goto fail;
}
> + if (error_code = add_option("--frames=0", &encoder_options)) {
> + goto fail;
> + }
Same here, and further occurences below.
> + error_code = AVERROR(ENOMEM);
Stray whitespace.
> + int ret = 0;
Unused assignment.
> + av_strlcpy(option_ctx->s, current_option, (option_length + 1));
> + option_ctx->s += 1 + option_length;
> + option_ctx->options_added++;
> + option_ctx->buffer_filled += option_length + 1;
It's a bit confusing to read, if you use "option_length + 1" twice, and
"1 + option_length" the third time.
> +static const AVOption options[] = {
> + { "turing-params", "configure additional turing encoder parameters",
> offsetof(libturingEncodeContext, options), AV_OPT_TYPE_STRING,{ .str = NULL
> }, 0, 0, AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_ENCODING_PARAM },
IMHO you can drop the word "configure ".
Cheers,
Moritz
_______________________________________________
ffmpeg-devel mailing list
[email protected]
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
