On Wed, Mar 16, 2016 at 10:24:09PM +0100, Thilo Borgmann wrote: [...] Not sure if I'm commenting on the last version...
> From b6b889c06edc7872f0a31fd0482793a199ddd28e Mon Sep 17 00:00:00 2001
> From: Thilo Borgmann <thilo.borgm...@mail.de>
> Date: Wed, 16 Mar 2016 22:18:20 +0100
> Subject: [PATCH 2/2] lavf: Add coreimage filter for GPU based image filtering
> on OSX.
lavfi, not lavf
>
> ---
> Changelog | 1 +
> MAINTAINERS | 1 +
> configure | 2 +
> doc/filters.texi | 165 +++++++++++
> libavfilter/Makefile | 1 +
> libavfilter/allfilters.c | 2 +
> libavfilter/vf_coreimage.m | 716
> +++++++++++++++++++++++++++++++++++++++++++++
> 7 files changed, 888 insertions(+)
> create mode 100644 libavfilter/vf_coreimage.m
>
don't forget lavfi minor version bump.
> diff --git a/Changelog b/Changelog
> index 1f57f5e..5053a86 100644
> --- a/Changelog
> +++ b/Changelog
> @@ -12,6 +12,7 @@ version <next>:
> - ciescope filter
> - protocol blacklisting API
> - MediaCodec H264 decoding
> +- coreimage filter (GPU based image filtering on OSX)
>
>
> version 3.0:
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 531c21d..a993a67 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -370,6 +370,7 @@ Filters:
> vf_colorbalance.c Paul B Mahol
> vf_colorkey.c Timo Rothenpieler
> vf_colorlevels.c Paul B Mahol
> + vf_coreimage.m Thilo Borgmann
> vf_deband.c Paul B Mahol
> vf_dejudder.c Nicholas Robbins
> vf_delogo.c Jean Delvare (CC <jdelv...@suse.com>)
> diff --git a/configure b/configure
> index 1b189328..da51e06 100755
> --- a/configure
> +++ b/configure
> @@ -5255,6 +5255,7 @@ frei0r_filter_extralibs='$ldl'
> frei0r_src_filter_extralibs='$ldl'
> ladspa_filter_extralibs='$ldl'
> nvenc_encoder_extralibs='$ldl'
> +coreimage_filter_extralibs="-framework QuartzCore -framework AppKit
> -framework OpenGL"
don't you need the same for coreimagesrc?
>
> if ! disabled network; then
> check_func getaddrinfo $network_extralibs
> @@ -5483,6 +5484,7 @@ enabled avisynth && { { check_lib2 "windows.h"
> LoadLibrary; } ||
> die "ERROR: LoadLibrary/dlopen not found for
> avisynth"; }
> enabled cuda && check_lib cuda.h cuInit -lcuda
> enabled chromaprint && require chromaprint chromaprint.h
> chromaprint_get_version -lchromaprint
> +enabled coreimage_filter && { check_header_objcc QuartzCore/CoreImage.h ||
> disable coreimage_filter; }
ditto
> enabled decklink && { check_header DeckLinkAPI.h || die "ERROR:
> DeckLinkAPI.h header not found"; }
> enabled frei0r && { check_header frei0r.h || die "ERROR: frei0r.h
> header not found"; }
> enabled gmp && require2 gmp gmp.h mpz_export -lgmp
> diff --git a/doc/filters.texi b/doc/filters.texi
> index c093927..f6767ec 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -4955,6 +4955,111 @@ convolution="-2 -1 0 -1 1 1 0 1 2:-2 -1 0 -1 1 1 0 1
> 2:-2 -1 0 -1 1 1 0 1 2:-2 -
> Copy the input source unchanged to the output. This is mainly useful for
> testing purposes.
>
> +@anchor{coreimage}
> +@section coreimage
> +Video filtering on GPU using Apple's CoreImage API on OSX.
> +
> +Hardware acceleration is based on an OpenGL context. Usually, this means it
> is
> +processed by video hardware. However, software-based OpenGL implementations
> +exist which means there is no guarantee for hardware processing. It depends
> on
> +the respective OSX.
> +
> +There are many filters and image generators provided by Apple that come with
> a
> +large variety of options. The filter has to be referenced by its name along
> +with its options.
> +
> +The coreimage filter accepts the following options:
> +@table @option
> +@item list_filters
> +List all available filters and generators along with all their respective
> +options as well as possible minimum and maximum values along with the default
> +values.
> +@example
> + list_filters=true
> +@end example
Indent is done at style level. Same issue below.
[...]
> +The @ref{coreimagesrc} video source can be used for generating input images
The anchor doesn't seem to exist
[...]
> +typedef struct CoreImageContext {
> + const AVClass *class;
> +
> + int is_video_source; ///< filter is used as video source
nit: unaligned vertically
> +
> + int w, h; ///< video size
> + AVRational sar; ///< sample aspect ratio
> + AVRational frame_rate; ///< video frame rate
> + AVRational time_base; ///< stream time base
> + int64_t duration; ///< duration expressed in
> microseconds
> + unsigned int nb_frame; ///< sequential frame number
> + int64_t pts; ///< increasing presentation time
> stamp
> + AVFrame *picref; ///< cached reference containing the
> painted picture
> +
> + CFTypeRef glctx; ///< OpenGL context
> + CGContextRef cgctx; ///< Bitmap context for image copy
> + CFTypeRef input_image; ///< Input image container for
> passing into Core Image API
> + CGColorSpaceRef color_space; ///< Common color space for input
> image and cgcontext
> + int bits_per_component; ///< Shared bpc for input-output
> operation
> +
> + char *filter_string; ///< The complete user provided
> filter definition
> + CFTypeRef *filters; ///< CIFilter object for all
> requested filters
> + int num_filters; ///< Amount of filters in *filters
> +
> + char *output_rect; ///< Rectangle to be filled with
> filter intput
> + int list_filters; ///< Option used to list all
> available filters including generators
> + int list_generators; ///< Option used to list all
> available generators
> +} CoreImageContext;
> +
> +/** Configure image and stream properties for output link of filter chain.
> + */
Unnecessary comment
> +static int config_output(AVFilterLink *link)
> +{
> + CoreImageContext *ctx = link->src->priv;
> +
> + link->w = ctx->w;
> + link->h = ctx->h;
> + link->sample_aspect_ratio = ctx->sar;
> + link->frame_rate = ctx->frame_rate;
> + link->time_base = ctx->time_base;
> +
> + const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(link->format);
> + ctx->bits_per_component = av_get_bits_per_pixel(desc) /
> desc->nb_components;
> +
> + return 0;
> +}
> +
> +/** Determine image properties from input link of filter chain.
> + */
> +static int config_input(AVFilterLink *link)
> +{
> + CoreImageContext *ctx = link->dst->priv;
> + const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(link->format);
> + ctx->bits_per_component = av_get_bits_per_pixel(desc) /
> desc->nb_components;
isn't this going to conflict in some ways with the config_output? Don't
you need only the config_output() callback?
> +
> + return 0;
> +}
> +
> +/** Print a list of all available filters including options and respective
> value ranges and defaults.
> + */
> +static void list_filters(CoreImageContext *ctx)
> +{
> + // querying filters and attributes
> + NSArray *filter_categories = nil;
> +
> + if (ctx->list_generators && !ctx->list_filters) {
> + filter_categories = [NSArray arrayWithObjects:kCICategoryGenerator,
> nil];
> + }
> +
> + NSArray *filter_names = [CIFilter
> filterNamesInCategories:filter_categories];
> + NSEnumerator *filters = [filter_names objectEnumerator];
> +
> + NSString *filter_name;
> + while (filter_name = [filters nextObject]) {
> + av_log(ctx, AV_LOG_INFO, "Filter: %s\n", [filter_name UTF8String]);
> + NSString *input;
> +
> + CIFilter *filter = [CIFilter filterWithName:filter_name];
> + NSDictionary *filter_attribs = [filter attributes]; // <nsstring, id>
> + NSArray *filter_inputs = [filter inputKeys]; // <nsstring>
> +
> + for (input in filter_inputs) {
> + NSDictionary *input_attribs = [filter_attribs valueForKey:input];
> + NSString *input_class = [input_attribs
> valueForKey:kCIAttributeClass];
> + if ([input_class isEqualToString:@"NSNumber"]) {
> + NSNumber *value_default = [input_attribs
> valueForKey:kCIAttributeDefault];
> + NSNumber *value_min = [input_attribs
> valueForKey:kCIAttributeSliderMin];
> + NSNumber *value_max = [input_attribs
> valueForKey:kCIAttributeSliderMax];
> +
> + av_log(ctx, AV_LOG_INFO, "\tOption: %s\t[%s]\t[%s %s][%s]\n",
> + [input UTF8String],
> + [input_class UTF8String],
> + [[value_min stringValue] UTF8String],
> + [[value_max stringValue] UTF8String],
> + [[value_default stringValue] UTF8String]);
> + } else {
> + av_log(ctx, AV_LOG_INFO, "\tOption: %s\t[%s]\n",
> + [input UTF8String],
> + [input_class UTF8String]);
> + }
> + }
> + }
> +}
> +
> +/** Define input and output formats for this filter.
> + */
pointless comment
> +static int query_formats(AVFilterContext *fctx)
> +{
> + static const enum AVPixelFormat inout_fmts_rgb[] = {
> + AV_PIX_FMT_ARGB,
> + AV_PIX_FMT_NONE
> + };
> +
> + AVFilterFormats *inout_formats;
> + int ret;
> +
> + if (!(inout_formats = ff_make_format_list(inout_fmts_rgb))) {
> + return AVERROR(ENOMEM);
> + }
> +
> + if ((ret = ff_formats_ref(inout_formats, &fctx->inputs[0]->out_formats))
> < 0 ||
> + (ret = ff_formats_ref(inout_formats, &fctx->outputs[0]->in_formats))
> < 0) {
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +/** Define output formats for this filter.
> + */
ditto
[...]
> + frame = av_frame_clone(ctx->picref);
> + if (!frame) {
> + return AVERROR(ENOMEM);
> + }
> +
> + frame->pts = ctx->pts;
> + frame->key_frame = 1;
> + frame->interlaced_frame = 0;
> + frame->pict_type = AV_PICTURE_TYPE_I;
> + frame->sample_aspect_ratio = ctx->sar;
> +
> + ctx->pts++;
> + ctx->nb_frame++;
looks like you can get rid of this and use {in,out}link->frame_count
[...]
> +#define CLAMP_WARNING { \
> +av_log(ctx, AV_LOG_WARNING, "Value of \"%f\" for option \"%s\" is out of
> range [%f %f], clamping to \"%f\".\n", \
> + input, \
> + [input_key UTF8String], \
> + min_value.floatValue, \
> + max_value.floatValue, \
> + used_value.floatValue); \
> +}
remove the { } and ';', or use a do while (0) form
[...]
> + if (!ctx->glctx) {
> + av_log(ctx, AV_LOG_ERROR, "CIContext not created.\n");
> + return -1;
AVERROR_EXTERNAL
> + }
> +
> + // Creating an empty input image as input container for the context
> + ctx->input_image = CFBridgingRetain([CIImage emptyImage]);
> +
> + return 0;
> +}
> +
> +/** Initialize as video source.
> + */
> +static av_cold int init_src(AVFilterContext *fctx)
> +{
> + CoreImageContext *ctx = fctx->priv;
> +
> + ctx->is_video_source = 1;
> + ctx->time_base = av_inv_q(ctx->frame_rate);
> + ctx->nb_frame = 0;
> + ctx->pts = 0;
> +
> +
> + return init(fctx);
> +}
> +
> +/** Uninitialize all filters, contexts and free all allocated memory.
> + */
these kind of comments really serve no purpose...
> +static av_cold void uninit(AVFilterContext *fctx)
> +{
[...]
> +#define GENERATOR_OPTIONS
> \
> + {"size", "set video size", OFFSET(w),
> AV_OPT_TYPE_IMAGE_SIZE, {.str = "320x240"}, 0, 0, FLAGS}, \
> + {"s", "set video size", OFFSET(w),
> AV_OPT_TYPE_IMAGE_SIZE, {.str = "320x240"}, 0, 0, FLAGS}, \
> + {"rate", "set video rate", OFFSET(frame_rate),
> AV_OPT_TYPE_VIDEO_RATE, {.str = "25"}, 0, 0, FLAGS}, \
> + {"r", "set video rate", OFFSET(frame_rate),
> AV_OPT_TYPE_VIDEO_RATE, {.str = "25"}, 0, 0, FLAGS}, \
> + {"duration", "set video duration", OFFSET(duration),
> AV_OPT_TYPE_DURATION, {.i64 = -1}, -1, INT64_MAX, FLAGS}, \
> + {"d", "set video duration", OFFSET(duration),
> AV_OPT_TYPE_DURATION, {.i64 = -1}, -1, INT64_MAX, FLAGS}, \
> + {"sar", "set video sample aspect ratio", OFFSET(sar),
> AV_OPT_TYPE_RATIONAL, {.dbl= 1}, 0, INT_MAX, FLAGS},
nit: space before =
> +
> +#define FILTER_OPTIONS
>
> \
> + {"list_filters", "list available filters",
> OFFSET(list_filters), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, .flags =
> FLAGS, "list_filters"}, \
> + {"list_generators", "list available generators",
> OFFSET(list_generators), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, .flags =
> FLAGS, "list_generators"}, \
the unit field is not necessary anymore since you moved to BOOL
> + {"filter", "names and options of filters to apply",
> OFFSET(filter_string), AV_OPT_TYPE_STRING, { .str = NULL }, .flags =
> FLAGS}, \
> + {"output_rect", "output rectangle within output image",
> OFFSET(output_rect), AV_OPT_TYPE_STRING, { .str = NULL }, .flags =
> FLAGS},
> +
> +
> +// definitions for coreimage video filter
> +static const AVOption coreimage_options[] = {
> + FILTER_OPTIONS
> + { NULL }
> +};
> +
> +static const AVClass coreimage_class = {
> + .class_name = "coreimage",
> + .item_name = av_default_item_name,
> + .option = coreimage_options,
> + .version = LIBAVUTIL_VERSION_INT,
> + .category = AV_CLASS_CATEGORY_FILTER,
> +};
> +
AVFILTER_DEFINE_CLASS()
[...]
No other comment from me but I haven't look at the functional part.
--
Clément B.
signature.asc
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
