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 ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel