On Thu, Jul 12, 2018 at 21:17:42 +0900, Stephen Seo wrote:
> + --enable-lensfun enable lensfun lens correction [no]
I may be mistaken, but aren't (new) dependencies to external libraries
coded as "libfoo"? So "--enable-liblensfun"?
> EXTERNAL_LIBRARY_VERSION3_LIST="
> gmp
> + lensfun
Same here then of course.
> +The focal length of the image/video (zoom; expected constant for video). For
^^^^
I now what you mean, but the wording does not seem clear enough.
Perhaps note that either constant value is assumed throughout the
video, or a formula has to be provided via timeline support.
> +@table @samp
> +@item nearest
> +@item linear
> +(default)
Unwrap that last line (though it shouldn't matter).
> OBJS-$(CONFIG_ZSCALE_FILTER) += vf_zscale.o
> +OBJS-$(CONFIG_LENSFUN_FILTER) += vf_lensfun.o
These are meant to be alphabetical.
> +#define LANCZOS_RESOLUTION 256
> + av_log(ctx, AV_LOG_INFO, "Using camera %s\n",
> lensfun->camera->Model);
[...]
> + av_log(ctx, AV_LOG_FATAL, "Failed to find camera in lensfun
> database\n");
If you're logging the model on success, you might as well also log it
on failure.
> + av_log(ctx, AV_LOG_INFO, "Using lens %s\n", lensfun->lens->Model);
[...]
> + av_log(ctx, AV_LOG_FATAL, "Failed to find lens in lensfun
> database\n");
Same here.
> + if (!lensfun->modifier) {
> + if (lensfun->camera && lensfun->lens) {
[...]
> + } else {
> + return AVERROR_INVALIDDATA;
> + }
I'm not sure this is what AVERROR_INVALIDDATA is meant to be used for.
> + // apply only subpixsel distortion
^ subpixel
> + for (index = 0; index < 4 * LANCZOS_RESOLUTION; ++index) {
> + a = sqrt((float)index / LANCZOS_RESOLUTION);
> + if (a == 0.0f) {
> + lensfun->interpolation[index] = 1.0f;
> + } else {
> + lensfun->interpolation[index] = lanczos_kernel(a);
> + }
> + }
Is it not so that a is only == 0.0f if index == 0? Instead of doing a
floating point operation and a sketchy floating point compare (I know,
comparison with zero is okay), just check for index == 0 before
calculating a?
Something like:
for (index = 0; index < 4 * LANCZOS_RESOLUTION; ++index) {
if (index == 0) {
lensfun->interpolation[index] = 1.0f;
} else {
a = sqrt((float)index / LANCZOS_RESOLUTION);
lensfun->interpolation[index] = lanczos_kernel(a);
}
}
> + thread_data->data_out[x * 3 + rgb_index + y *
> thread_data->linesize_out] = interpolated < 0.0f ? 0.0f : interpolated >
> 255.0f ? 255 : interpolated;
^ 255.0f
(Assign 255 as float, like you do with the other constants here.)
> + distortion_correction_thread_data.width = inlink->w;
> + distortion_correction_thread_data.height = inlink->h;
> + distortion_correction_thread_data.distortion_coords =
> lensfun->distortion_coords;
> + distortion_correction_thread_data.data_in = in->data[0];
> + distortion_correction_thread_data.data_out = out->data[0];
> + distortion_correction_thread_data.linesize_in = in->linesize[0];
> + distortion_correction_thread_data.linesize_out = out->linesize[0];
> + distortion_correction_thread_data.interpolation =
> lensfun->interpolation;
> + distortion_correction_thread_data.mode = lensfun->mode;
> + distortion_correction_thread_data.interpolation_type =
> lensfun->interpolation_type;
I think there's a more compact way to do this with a compound literal
assignment or designated intializers, and ffmpeg code allows for these
particular C99 features.
Cheers,
Moritz
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
