October 2007 - FFmpeg-soc - lists.ffmpeg.org

libavfilter review
by Michael Niedermayer 04 Nov '07

04 Nov '07

Hi after the matroska muxer and dirac heres my first try of a lavfilter review this is based on the files a day or 2 ago so please forgive me if iam complaining about things which have been changed already ffmpeg.diff: please split this patch, for example the restructuring which is not under ENABLE_AVFILTER could be in a seperate patch also dont hesitate to put a README.ffmpeg.diff or so in svn describing what exactly is done and why it might safe some time with reviewing avfiltergraph.c: this is quite complex, is this complexity really needed? this contains several methods to build filter graphs, why is a single one not enough? [...] > /* given the link between the dummy filter and an internal filter whose input > * is being exported outside the graph, this returns the externally visible > * link */ > static inline AVFilterLink *get_extern_input_link(AVFilterLink *link) not doxygen compatible [...] avfilter.c: [...] > AVFilterPicRef *avfilter_ref_pic(AVFilterPicRef *ref, int pmask) > { > AVFilterPicRef *ret = av_malloc(sizeof(AVFilterPicRef)); > memcpy(ret, ref, sizeof(AVFilterPicRef)); *ret= *ref; advantages: type checking and its simpler ... [...] > void avfilter_insert_pad(unsigned idx, unsigned *count, size_t padidx_off, > AVFilterPad **pads, AVFilterLink ***links, > AVFilterPad *newpad) > { > unsigned i; > > idx = FFMIN(idx, *count); > > *pads = av_realloc(*pads, sizeof(AVFilterPad) * (*count + 1)); > *links = av_realloc(*links, sizeof(AVFilterLink*) * (*count + 1)); > memmove(*pads +idx+1, *pads +idx, sizeof(AVFilterPad) * (*count-idx)); > memmove(*links+idx+1, *links+idx, sizeof(AVFilterLink*) * (*count-idx)); > memcpy(*pads+idx, newpad, sizeof(AVFilterPad)); > (*links)[idx] = NULL; > > (*count) ++; > for(i = idx+1; i < *count; i ++) > if(*links[i]) > (*(unsigned *)((uint8_t *)(*links[i]) + padidx_off)) ++; well, it does increase the *pad though iam not sure if not maybe it would be better to not use a byte offset into the struct to identify which of the 2 should be used ... [...] > /* find a format both filters support - TODO: auto-insert conversion filter */ > link->format = -1; > if(link->src->output_pads[link->srcpad].query_formats) > fmts[0] = link->src->output_pads[link->srcpad].query_formats(link); > else > fmts[0] = avfilter_default_query_output_formats(link); > fmts[1] = link->dst->input_pads[link->dstpad].query_formats(link); > for(i = 0; fmts[0][i] != -1; i ++) > for(j = 0; fmts[1][j] != -1; j ++) > if(fmts[0][i] == fmts[1][j]) { > link->format = fmts[0][i]; > goto format_done; > } the query_formats system is very flexible, it takes a AVFilterLink so a filter could have a fixed list of supported input pix_fmts and make the output query_formats depend on the input pix_fmt or the other was around well i dont think the other way around would work but how should the user know that? and whats the sense of this overly flexible system if it doesnt work with anything but the obvious simple supported output pix_fmt depends upon input formats, it would be alot clearer if query_formats would take a list/array of input pix_fmts as argument (or a array or pix_fmt, flags pairs) where the flags could indicate if the pix_fmt can be provided without colorspace conversation, but maybe thats not really usefull and a simpler prefer first possible format in the list system would work equally well also what happens in the following case: src -> filter -> destination src supports formats A and C destination supports formats B anc C and the filter supports A,B,C inputs and output=input if i understood the code correctly this would fail > > format_done: > av_free(fmts[0]); > av_free(fmts[1]); > if(link->format == -1) > return -1; > > if(!(config_link = link->src->output_pads[link->srcpad].config_props)) > config_link = avfilter_default_config_output_link; > if(config_link(link)) > return -1; > > if(!(config_link = link->dst->input_pads[link->dstpad].config_props)) > config_link = avfilter_default_config_input_link; > if(config_link(link)) > return -1; > > return 0; > } > > AVFilterPicRef *avfilter_get_video_buffer(AVFilterLink *link, int perms) > { > AVFilterPicRef *ret = NULL; > > if(link->dst->input_pads[link->dstpad].get_video_buffer) > ret = link->dst->input_pads[link->dstpad].get_video_buffer(link, perms); > > if(!ret) > ret = avfilter_default_get_video_buffer(link, perms); > > return ret; > } > > int avfilter_request_frame(AVFilterLink *link) > { > const AVFilterPad *pad = &link->src->output_pads[link->srcpad]; > > if(pad->request_frame) > return pad->request_frame(link); > else if(link->src->inputs[0]) > return avfilter_request_frame(link->src->inputs[0]); > else return -1; > } > > /* XXX: should we do the duplicating of the picture ref here, instead of > * forcing the source filter to do it? */ > void avfilter_start_frame(AVFilterLink *link, AVFilterPicRef *picref) > { > void (*start_frame)(AVFilterLink *, AVFilterPicRef *); > > start_frame = link->dst->input_pads[link->dstpad].start_frame; > if(!start_frame) > start_frame = avfilter_default_start_frame; > > start_frame(link, picref); you are mixing func_ptr = A if(!func_ptr) func_ptr= B; func_ptr(); and if(A) A(x) else B(x) and if(!(func_ptr= B)) func_ptr= A; func_ptr(); i think a little bit consistency would make things more readable [...] > static int filter_cmp(const void *aa, const void *bb) > { > const AVFilter *a = *(const AVFilter **)aa, *b = *(const AVFilter **)bb; > return strcmp(a->name, b->name); > } for some reason i always loved giving these cmp functions arguments with correct types instead of temporary variables (and casts) it IMHO looks cleaner > > AVFilter *avfilter_get_by_name(char *name) > { > AVFilter key = { .name = name, }; > AVFilter *key2 = &key; > AVFilter **ret; > > ret = bsearch(&key2, filters, filter_count, sizeof(AVFilter **), filter_cmp); > if(ret) > return *ret; > return NULL; > } do we search for filters often enough that a bsearch is worth it? (no iam not complaining, just asking) also a simple linked list is O(1) insert O(n) search we insert all filters, tough likely we use less than all your code is O(n log n) insert O(log n) search if above hypothesis is true that wouldnt be faster delaying the sort until the first avfilter_get_by_name() would help but well, the whole doesnt seem to matter speedwise so ill stop giving stupid suggestions on how to improve it here :) > > /* FIXME: insert in order, rather than insert at end + resort */ > void avfilter_register(AVFilter *filter) > { > filters = av_realloc(filters, sizeof(AVFilter*) * (filter_count+1)); > filters[filter_count] = filter; > qsort(filters, ++filter_count, sizeof(AVFilter **), filter_cmp); > } > [...] > AVFilterContext *avfilter_create(AVFilter *filter, char *inst_name) > { > AVFilterContext *ret = av_malloc(sizeof(AVFilterContext)); > > ret->av_class = av_mallocz(sizeof(AVClass)); > ret->av_class->item_name = filter_name; > ret->filter = filter; > ret->name = inst_name ? av_strdup(inst_name) : NULL; > ret->priv = av_mallocz(filter->priv_size); > > ret->input_count = pad_count(filter->inputs); > ret->input_pads = av_malloc(sizeof(AVFilterPad) * ret->input_count); > memcpy(ret->input_pads, filter->inputs, sizeof(AVFilterPad)*ret->input_count); why are the filter pads copied from AVFilter to AVFilterContext? wouldnt a pointer do? > ret->inputs = av_mallocz(sizeof(AVFilterLink*) * ret->input_count); > > ret->output_count = pad_count(filter->outputs); > ret->output_pads = av_malloc(sizeof(AVFilterPad) * ret->output_count); > memcpy(ret->output_pads, filter->outputs, sizeof(AVFilterPad)*ret->output_count); > ret->outputs = av_mallocz(sizeof(AVFilterLink*) * ret->output_count); > > return ret; > } also whats your oppinion about changing the name to avfilter_open() it after all doesnt create a AVFilter > > void avfilter_destroy(AVFilterContext *filter) > { > int i; > > if(filter->filter->uninit) > filter->filter->uninit(filter); > > for(i = 0; i < filter->input_count; i ++) { > if(filter->inputs[i]) > filter->inputs[i]->src->outputs[filter->inputs[i]->srcpad] = NULL; > av_free(filter->inputs[i]); > } > for(i = 0; i < filter->output_count; i ++) { > if(filter->outputs[i]) > filter->outputs[i]->dst->inputs[filter->outputs[i]->dstpad] = NULL; > av_free(filter->outputs[i]); > } > > av_free(filter->name); > av_free(filter->input_pads); > av_free(filter->output_pads); > av_free(filter->inputs); > av_free(filter->outputs); > av_free(filter->priv); > av_free(filter->av_class); > av_free(filter); id change that all to av_freep() to reduce the chance of using freed memory > } > > AVFilterContext *avfilter_create_by_name(char *name, char *inst_name) > { > AVFilter *filt; > > if(!(filt = avfilter_get_by_name(name))) return NULL; > return avfilter_create(filt, inst_name); > } i think the user can call these 2 functions himself, no need to provide a wraper and thus complicate the API by yet another function [...] avfiltergraphdesc.c: [...] > /* a comment is a line which is empty, or starts with whitespace, ';' or '#' */ > static inline int is_line_comment(char *line) doxygenize please [...] > static Section parse_section_name(char *line) > { > if(!strncmp(line, strFilters, strlen(strFilters))) return SEC_FILTERS; > else if(!strncmp(line, strLinks, strlen(strLinks))) return SEC_LINKS; > else if(!strncmp(line, strInputs, strlen(strInputs))) return SEC_INPUTS; > else if(!strncmp(line, strOutputs, strlen(strOutputs))) return SEC_OUTPUTS; an array of strings, Section would simplify this to a loop [...] > AVFilterGraphDesc *avfilter_graph_load_desc(const char *filename) > { > AVFilterGraphDesc *ret = NULL; > AVFilterGraphDescFilter *filter = NULL; > AVFilterGraphDescLink *link = NULL; > AVFilterGraphDescExport *input = NULL; > AVFilterGraphDescExport *output = NULL; > > Section section; > char line[LINESIZE]; > void *next; > FILE *in; > > /* TODO: maybe allow searching in a predefined set of directories to > * allow users to build up libraries of useful graphs? */ > if(!(in = fopen(filename, "r"))) > goto fail; i do not like this at all, the libavfilter core has no bushiness doing (f)open() its not hard to do graph_from_file_filter="`cat myfile`" or similar on the command line or let the app do the file reading and provide a char* to avfilter [...] > case SEC_FILTERS: > if(!(next = parse_filter(line))) > goto fail; > if(filter) > filter = filter->next = next; > else > ret->filters = filter = next; > break; AVFilterGraphDescFilter **filterp = &ret->filters; and *filterp= next; filterp= &(next->next); [...] avfilter.h: [...] > int64_t pts; ///< presentation timestamp in milliseconds milliseconds is insufficient 1/AV_TIME_BASE would be better [...] default.c: [...] > /** > * default config_link() implementation for output video links to simplify > * the implementation of one input one output video filters */ > int avfilter_default_config_output_link(AVFilterLink *link) > { > if(link->src->input_count && link->src->inputs[0]) { > link->w = link->src->inputs[0]->w; > link->h = link->src->inputs[0]->h; > } else { > /* XXX: any non-simple filter which would cause this branch to be taken > * really should implement its own config_props() for this link. */ > link->w = > link->h = 0; then why is this not a return -1; assert(0) or whatever [...] vf_fps.c: [...] > static int init(AVFilterContext *ctx, const char *args, void *opaque) > { > FPSContext *fps = ctx->priv; > int framerate; > > /* TODO: support framerates specified as decimals or fractions */ > if(args && sscanf(args, "%d", &framerate)) > fps->timebase = 1000 / framerate; > else > /* default to 25 fps */ > fps->timebase = 1000 / 25; timebase must be AVRational things like 25000/1001 must be possible, they are quite common [...] vf_overlay.c: [...] > switch(link->format) { > case PIX_FMT_RGB32: > case PIX_FMT_BGR32: > over->bpp = 4; > break; > case PIX_FMT_RGB24: > case PIX_FMT_BGR24: > over->bpp = 3; > break; > case PIX_FMT_RGB565: > case PIX_FMT_RGB555: > case PIX_FMT_BGR565: > case PIX_FMT_BGR555: > case PIX_FMT_GRAY16BE: > case PIX_FMT_GRAY16LE: > over->bpp = 2; > break; > default: > over->bpp = 1; > } this code is duplicated, it occurs in several filters [...] vsrc_ppm.c: [...] > static int init(AVFilterContext *ctx, const char *args, void *opaque) > { > PPMContext *ppm = ctx->priv; > int max; > > if(!args) return -1; > if(!(ppm->in = fopen(args, "r"))) return -1; access should be done through URLProtocol or libavformat or something else direct fopen() is highly unflexible (for testing its of course ok but a final read image filter shouldnt do it like this) [...] > static int request_frame(AVFilterLink *link) > { > PPMContext *ppm = link->src->priv; > AVFilterPicRef *out; > > int y; > uint8_t *row; > > if(!ppm->pic) { > ppm->pic = avfilter_get_video_buffer(link, > AV_PERM_WRITE | AV_PERM_PRESERVE | AV_PERM_REUSE); > > row = ppm->pic->data[0]; > for(y = 0; y < ppm->h; y ++) { > fread(row, 3, ppm->w, ppm->in); > row += ppm->pic->linesize[0]; > } > > fclose(ppm->in); > ppm->in = NULL; > } else ppm->pic->pts += 30; ^^ this should be user configureable [...] > AVFilter vsrc_ppm = this (and others) should have some prefix to avoid name clashes [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The worst form of inequality is to try to make unequal things equal. -- Aristotle

3 10

[soc]: r1425 - libavfilter/avfiltergraph.c
by vitor 30 Oct '07

30 Oct '07

Author: vitor Date: Tue Oct 30 22:15:19 2007 New Revision: 1425 Log: Silence warning caused by my last commit Modified: libavfilter/avfiltergraph.c Modified: libavfilter/avfiltergraph.c ============================================================================== --- libavfilter/avfiltergraph.c (original) +++ libavfilter/avfiltergraph.c Tue Oct 30 22:15:19 2007 @@ -446,7 +446,7 @@ static AVFilterContext *create_filter_wi av_log(NULL, AV_LOG_INFO, "creating filter \"%s\" with args \"%s\"\n", name, args ? args : "(none)"); - if(ret = avfilter_open(avfilter_get_by_name(name), NULL)) { + if((ret = avfilter_open(avfilter_get_by_name(name), NULL))) { if(avfilter_init_filter(ret, args, opaque)) { av_log(NULL, AV_LOG_ERROR, "error initializing filter!\n"); avfilter_destroy(ret);

1 0

[soc]: r1424 - in libavfilter: avfiltergraph.c diffs/03_ffplay_filters.diff
by vitor 30 Oct '07

30 Oct '07

Author: vitor Date: Tue Oct 30 22:11:34 2007 New Revision: 1424 Log: Minor simplification Modified: libavfilter/avfiltergraph.c libavfilter/diffs/03_ffplay_filters.diff Modified: libavfilter/avfiltergraph.c ============================================================================== --- libavfilter/avfiltergraph.c (original) +++ libavfilter/avfiltergraph.c Tue Oct 30 22:11:34 2007 @@ -430,7 +430,6 @@ int avfilter_graph_config_links(AVFilter static AVFilterContext *create_filter_with_args(const char *filt, void *opaque) { - AVFilter *filterdef; AVFilterContext *ret; char *filter = av_strdup(filt); /* copy - don't mangle the input string */ char *name, *args; @@ -447,8 +446,7 @@ static AVFilterContext *create_filter_wi av_log(NULL, AV_LOG_INFO, "creating filter \"%s\" with args \"%s\"\n", name, args ? args : "(none)"); - if((filterdef = avfilter_get_by_name(name)) && - (ret = avfilter_open(filterdef, NULL))) { + if(ret = avfilter_open(avfilter_get_by_name(name), NULL)) { if(avfilter_init_filter(ret, args, opaque)) { av_log(NULL, AV_LOG_ERROR, "error initializing filter!\n"); avfilter_destroy(ret); Modified: libavfilter/diffs/03_ffplay_filters.diff ============================================================================== --- libavfilter/diffs/03_ffplay_filters.diff (original) +++ libavfilter/diffs/03_ffplay_filters.diff Tue Oct 30 22:11:34 2007 @@ -1,6 +1,6 @@ --- ffplay.c.old2 2007-10-16 20:27:42.000000000 +0200 +++ ffplay.c 2007-10-16 20:27:51.000000000 +0200 -@@ -25,6 +25,10 @@ +@@ -26,6 +26,10 @@ #include "swscale.h" #include "avstring.h" @@ -11,7 +11,7 @@ #include "version.h" #include "cmdutils.h" -@@ -84,6 +88,10 @@ +@@ -85,6 +89,10 @@ int width, height; /* source height & width */ enum PixelFormat pix_fmt; int allocated; @@ -22,7 +22,7 @@ } VideoPicture; typedef struct SubPicture { -@@ -166,6 +174,10 @@ +@@ -167,6 +175,10 @@ // QETimer *video_timer; char filename[1024]; int width, height, xleft, ytop; @@ -33,7 +33,7 @@ } VideoState; void show_help(void); -@@ -205,6 +217,9 @@ +@@ -206,6 +218,9 @@ static int error_resilience = FF_ER_CAREFUL; static int error_concealment = 3; static int decoder_reorder_pts= 0; @@ -43,7 +43,7 @@ /* current context */ static int is_full_screen; -@@ -651,14 +666,22 @@ +@@ -652,14 +667,22 @@ vp = &is->pictq[is->pictq_rindex]; if (vp->bmp) { @@ -68,7 +68,7 @@ /* if an active format is indicated, then it overrides the mpeg format */ #if 0 -@@ -861,9 +884,15 @@ +@@ -862,9 +885,15 @@ } else if(!is_full_screen && screen_width){ w = screen_width; h = screen_height; @@ -84,7 +84,7 @@ } else { w = 640; h = 480; -@@ -1169,9 +1198,19 @@ +@@ -1170,9 +1199,19 @@ if (vp->bmp) SDL_FreeYUVOverlay(vp->bmp); @@ -104,7 +104,7 @@ #if 0 /* XXX: use generic function */ -@@ -1208,7 +1247,7 @@ +@@ -1209,7 +1248,7 @@ { VideoPicture *vp; int dst_pix_fmt; @@ -113,7 +113,7 @@ static struct SwsContext *img_convert_ctx; /* wait until we have space to put a new picture */ -@@ -1226,8 +1265,13 @@ +@@ -1227,8 +1266,13 @@ /* alloc or resize hardware picture buffer */ if (!vp->bmp || @@ -127,7 +127,7 @@ SDL_Event event; vp->allocated = 0; -@@ -1251,6 +1295,12 @@ +@@ -1252,6 +1296,12 @@ /* if the frame is not skipped, then display it */ if (vp->bmp) { @@ -140,7 +140,7 @@ /* get a pointer on the bitmap */ SDL_LockYUVOverlay (vp->bmp); -@@ -1262,6 +1312,19 @@ +@@ -1263,6 +1313,19 @@ pict.linesize[0] = vp->bmp->pitches[0]; pict.linesize[1] = vp->bmp->pitches[2]; pict.linesize[2] = vp->bmp->pitches[1]; @@ -160,7 +160,7 @@ img_convert_ctx = sws_getCachedContext(img_convert_ctx, vp->width, vp->height, vp->pix_fmt, vp->width, vp->height, dst_pix_fmt, sws_flags, NULL, NULL, NULL); -@@ -1271,6 +1334,7 @@ +@@ -1272,6 +1335,7 @@ } sws_scale(img_convert_ctx, src_frame->data, src_frame->linesize, 0, vp->height, pict.data, pict.linesize); @@ -168,7 +168,7 @@ /* update the bitmap content */ SDL_UnlockYUVOverlay(vp->bmp); -@@ -1378,6 +1442,134 @@ +@@ -1379,6 +1443,134 @@ return 0; } @@ -303,14 +303,13 @@ static int video_thread(void *arg) { VideoState *is = arg; -@@ -1386,10 +1578,47 @@ +@@ -1387,10 +1579,45 @@ double pts; int ret; + +#if ENABLE_AVFILTER + AVFilterContext *filt_src = NULL, *filt_out = NULL, *filt_graph = NULL; -+ AVFilter *filterdef; + + avfilter_init(); + //avfilter_register(&input_filter); @@ -323,8 +322,7 @@ + if(avfilter_init_filter(filt_out, NULL, frame)) goto the_end; + + if(vfilters) { -+ if(!(filterdef = avfilter_get_by_name("graph")) || -+ !(filt_graph = avfilter_open(filterdef, NULL))) ++ if(!(filt_graph = avfilter_open(avfilter_get_by_name("graph"), NULL))) + goto the_end; + if(avfilter_init_filter(filt_graph, vfilters, NULL)) + goto the_end; @@ -351,7 +349,7 @@ if (ret < 0) goto the_end; pts = pts_int; -@@ -1402,6 +1631,10 @@ +@@ -1403,6 +1630,10 @@ stream_pause(cur_stream); } the_end: @@ -362,7 +360,7 @@ av_free(frame); return 0; } -@@ -2131,6 +2364,12 @@ +@@ -2132,6 +2363,12 @@ /* free all pictures */ for(i=0;i<VIDEO_PICTURE_QUEUE_SIZE; i++) { vp = &is->pictq[i]; @@ -375,7 +373,7 @@ if (vp->bmp) { SDL_FreeYUVOverlay(vp->bmp); vp->bmp = NULL; -@@ -2487,6 +2726,9 @@ +@@ -2488,6 +2725,9 @@ #endif { "sync", HAS_ARG | OPT_EXPERT, {(void*)opt_sync}, "set audio-video sync. type (type=audio/video/ext)", "type" }, { "threads", HAS_ARG | OPT_EXPERT, {(void*)opt_thread_count}, "thread count", "count" },

1 0

[soc]: r1423 - in libavfilter: README diffs/04_ffmpeg_filters.diff
by vitor 29 Oct '07

29 Oct '07

Author: vitor Date: Mon Oct 29 17:07:34 2007 New Revision: 1423 Log: First (possibly buggy) try of ffmpeg integration. At this point, the integration is quite simple. It is not possible yet to use more than one input and one output stream for filtering. Added: libavfilter/diffs/04_ffmpeg_filters.diff Modified: libavfilter/README Modified: libavfilter/README ============================================================================== --- libavfilter/README (original) +++ libavfilter/README Mon Oct 29 17:07:34 2007 @@ -18,8 +18,7 @@ wiki. See http://wiki.multimedia.cx/ind RUNNING IT: -Currently, only ffplay has been modified to make use of libavfilter. The -command line syntax for specifying filters is similar to that of mplayer. +The command line syntax for specifying filters is similar to that of mplayer. For example, to vertically flip a video, you would do: ./ffplay -vfilters vflip input_video.avi Added: libavfilter/diffs/04_ffmpeg_filters.diff ============================================================================== --- (empty file) +++ libavfilter/diffs/04_ffmpeg_filters.diff Mon Oct 29 17:07:34 2007 @@ -0,0 +1,356 @@ +Index: ffmpeg.c +=================================================================== +--- ffmpeg.c (revision 10755) ++++ ffmpeg.c (working copy) +@@ -34,6 +34,10 @@ + #include "fifo.h" + #include "avstring.h" + ++#if ENABLE_AVFILTER ++# include "avfilter.h" ++#endif ++ + #if !defined(HAVE_GETRUSAGE) && defined(HAVE_GETPROCESSTIMES) + #include <windows.h> + #endif +@@ -142,6 +146,7 @@ + static int loop_input = 0; + static int loop_output = AVFMT_NOOUTPUTLOOP; + static int qp_hist = 0; ++static char *vfilters = NULL; + + static int intra_only = 0; + static int audio_sample_rate = 44100; +@@ -277,6 +282,11 @@ + is not defined */ + int64_t pts; /* current pts */ + int is_start; /* is 1 at the start and after a discontinuity */ ++#if ENABLE_AVFILTER ++ AVFilterContext *out_video_filter; ++ AVFilterContext *input_video_filter; ++ AVFrame *filter_frame; ++#endif + } AVInputStream; + + typedef struct AVInputFile { +@@ -292,6 +302,241 @@ + static struct termios oldtty; + #endif + ++#if ENABLE_AVFILTER ++typedef struct { ++ AVInputStream *ist; ++} FilterInPriv; ++ ++static int input_init(AVFilterContext *ctx, const char *args, void *opaque) ++{ ++ FilterInPriv *priv = ctx->priv; ++ ++ if(!opaque) return -1; ++ ++ priv->ist = opaque; ++ priv->ist->filter_frame = avcodec_alloc_frame(); ++ ++ return 0; ++} ++ ++static void input_uninit(AVFilterContext *ctx) ++{ ++ FilterInPriv *priv = ctx->priv; ++ av_free(priv->ist->filter_frame); ++} ++ ++static int input_request_frame(AVFilterLink *link) ++{ ++ FilterInPriv *priv = link->src->priv; ++ AVFilterPicRef *picref; ++ ++ picref = avfilter_get_video_buffer(link, AV_PERM_WRITE); ++ ++ memcpy(picref->data, priv->ist->filter_frame->data, ++ sizeof(priv->ist->filter_frame->data)); ++ ++ memcpy(picref->linesize, priv->ist->filter_frame->linesize, ++ sizeof(priv->ist->filter_frame->linesize)); ++ ++ picref->pts = priv->ist->pts; ++ picref->pixel_aspect = priv->ist->st->codec->sample_aspect_ratio; ++ avfilter_start_frame(link, avfilter_ref_pic(picref, ~0)); ++ avfilter_draw_slice(link, 0, picref->h); ++ avfilter_end_frame(link); ++ avfilter_unref_pic(picref); ++ ++ return 0; ++} ++ ++static int *input_query_formats(AVFilterLink *link) ++{ ++ FilterInPriv *priv = link->src->priv; ++ return avfilter_make_format_list(1, priv->ist->st->codec->pix_fmt); ++} ++ ++static int input_config_props(AVFilterLink *link) ++{ ++ FilterInPriv *priv = link->src->priv; ++ AVCodecContext *c = priv->ist->st->codec; ++ ++ link->w = c->width; ++ link->h = c->height; ++ ++ return 0; ++} ++ ++static AVFilter input_filter = ++{ ++ .name = "ffmpeg_input", ++ .author = "Vitor Sessak", ++ ++ .priv_size = sizeof(FilterInPriv), ++ ++ .init = input_init, ++ .uninit = input_uninit, ++ ++ .inputs = (AVFilterPad[]) {{ .name = NULL }}, ++ .outputs = (AVFilterPad[]) {{ .name = "default", ++ .type = AV_PAD_VIDEO, ++ .request_frame = input_request_frame, ++ .query_formats = input_query_formats, ++ .config_props = input_config_props, }, ++ { .name = NULL }}, ++}; ++ ++typedef struct { ++ int pix_fmt; ++} FilterOutPriv; ++ ++ ++static int output_init(AVFilterContext *ctx, const char *args, void *opaque) ++{ ++ FilterOutPriv *priv = ctx->priv; ++ ++ if(!opaque) return -1; ++ ++ priv->pix_fmt = *((int *)opaque); ++ ++ return 0; ++} ++ ++static void output_end_frame(AVFilterLink *link) ++{ ++} ++ ++static int *output_query_formats(AVFilterLink *link) ++{ ++ FilterOutPriv *priv = link->dst->priv; ++ return avfilter_make_format_list(1, priv->pix_fmt); ++} ++ ++static int get_filtered_video_pic(AVFilterContext *ctx, AVPicture *pic2, ++ uint64_t *pts) ++{ ++ AVFilterPicRef *pic; ++ ++ if(avfilter_request_frame(ctx->inputs[0])) ++ return -1; ++ if(!(pic = ctx->inputs[0]->cur_pic)) ++ return -1; ++ ctx->inputs[0]->cur_pic = NULL; ++ ++ *pts = pic->pts; ++ ++ memcpy(pic2->data, pic->data, sizeof(pic->data)); ++ memcpy(pic2->linesize, pic->linesize, sizeof(pic->linesize)); ++ ++ return 1; ++} ++ ++static AVFilter output_filter = ++{ ++ .name = "ffmpeg_output", ++ .author = "Vitor Sessak", ++ ++ .priv_size = sizeof(FilterOutPriv), ++ .init = output_init, ++ ++ .inputs = (AVFilterPad[]) {{ .name = "default", ++ .type = AV_PAD_VIDEO, ++ .end_frame = output_end_frame, ++ .query_formats = output_query_formats, ++ .min_perms = AV_PERM_READ, }, ++ { .name = NULL }}, ++ .outputs = (AVFilterPad[]) {{ .name = NULL }}, ++}; ++ ++static int configure_filters(AVInputStream *ist, AVOutputStream *ost) ++{ ++ AVFilterContext *curr_filter; ++ AVFilterContext *filt_graph; ++ AVCodecContext *codec = ost->st->codec; ++ AVCodecContext *icodec = ist->st->codec; ++ ++ avfilter_init(); ++ ++ if(!(ist->input_video_filter = avfilter_open(&input_filter, "src"))) ++ return -1; ++ if(!(ist->out_video_filter = avfilter_open(&output_filter, "out"))) ++ return -1; ++ ++ if(avfilter_init_filter(ist->input_video_filter, NULL, ist)) ++ return -1; ++ if(avfilter_init_filter(ist->out_video_filter, NULL, &codec->pix_fmt)) ++ return -1; ++ ++ curr_filter = ist->input_video_filter; ++ ++ if(ost->video_crop) { ++ char crop_args[255]; ++ AVFilterContext *filt_crop; ++ snprintf(crop_args, 255, "%d:%d:%d:%d", ost->topBand, ost->topBand, codec->height - frame_topBand, codec->width- frame_bottomBand); ++ filt_crop = avfilter_open(avfilter_get_by_name("crop"), NULL); ++ if (!filt_crop) ++ return -1; ++ if (avfilter_init_filter(filt_crop, crop_args, NULL)) ++ return -1; ++ if (avfilter_link(curr_filter, 0, filt_crop, 0)) ++ return -1; ++ if (avfilter_config_link(curr_filter->outputs[0])) ++ return -1; ++ curr_filter = filt_crop; ++ } ++ ++ if((codec->width != ++ icodec->width - (frame_leftBand + frame_rightBand) + ++ (frame_padleft + frame_padright)) || ++ (codec->height != icodec->height - (frame_topBand + frame_bottomBand) + ++ (frame_padtop + frame_padbottom))) { ++ char crop_args[255]; ++ AVFilterContext *filt_scale; ++ snprintf(crop_args, 255, "%d:%d", codec->width - ++ (frame_padleft + frame_padright), ++ codec->height - (frame_padtop + frame_padbottom)); ++ filt_scale = avfilter_open(avfilter_get_by_name("scale"), NULL); ++ if (!filt_scale) ++ return -1; ++ if (avfilter_init_filter(filt_scale, crop_args, NULL)) ++ return -1; ++ if (avfilter_link(curr_filter, 0, filt_scale, 0)) ++ return -1; ++ if (avfilter_config_link(curr_filter->outputs[0])) ++ return -1; ++ curr_filter = filt_scale; ++ } ++ ++ ++ ++ if(vfilters) { ++ if(!(filt_graph = avfilter_open(avfilter_get_by_name("graph"), NULL))) ++ return -1; ++ if(avfilter_init_filter(filt_graph, vfilters, NULL)) ++ return -1; ++ if(avfilter_link(curr_filter, 0, filt_graph, 0)) ++ return -1; ++ if(avfilter_link(filt_graph, 0, ist->out_video_filter, 0)) ++ return -1; ++ if(avfilter_config_link(curr_filter->outputs[0])) ++ return -1; ++ if(avfilter_graph_config_links(filt_graph)) ++ return -1; ++ if(avfilter_config_link(filt_graph->outputs[0])) ++ return -1; ++ } else { ++ if(avfilter_link(curr_filter, 0, ist->out_video_filter, 0) < 0) ++ return -1; ++ if(avfilter_config_link(curr_filter->outputs[0])) ++ return -1; ++ ++ } ++ ++ codec->width = ist->out_video_filter->inputs[0]->w; ++ codec->height = ist->out_video_filter->inputs[0]->h; ++ ++ return 0; ++} ++#endif /* ENABLE_AVFILTER */ ++ + static void term_exit(void) + { + #ifdef HAVE_TERMIOS_H +@@ -616,6 +861,13 @@ + frame_hook_process(picture2, dec->pix_fmt, dec->width, dec->height, + 1000000 * ist->pts / AV_TIME_BASE); + ++#if ENABLE_AVFILTER ++ memcpy(ist->filter_frame->data, picture2->data, sizeof(picture2->data)); ++ memcpy(ist->filter_frame->linesize, picture2->linesize, sizeof(picture2->linesize)); ++ ++ if (get_filtered_video_pic(ist->out_video_filter, picture2, &ist->pts) < 0) ++ fprintf(stderr, "Error: get_filtered_video_pic failed!\n"); ++#endif + if (picture != picture2) + *picture = *picture2; + *bufp = buf; +@@ -726,6 +978,9 @@ + if (nb_frames <= 0) + return; + ++#if ENABLE_AVFILTER ++ formatted_picture = in_picture; ++#else + if (ost->video_crop) { + if (av_picture_crop((AVPicture *)&picture_crop_temp, (AVPicture *)in_picture, dec->pix_fmt, ost->topBand, ost->leftBand) < 0) { + av_log(NULL, AV_LOG_ERROR, "error cropping picture\n"); +@@ -735,6 +990,7 @@ + } else { + formatted_picture = in_picture; + } ++#endif + + final_picture = formatted_picture; + padding_src = formatted_picture; +@@ -750,12 +1006,14 @@ + } + } + ++#if !ENABLE_AVFILTER + if (ost->video_resample) { + padding_src = NULL; + final_picture = &ost->pict_tmp; + sws_scale(ost->img_resample_ctx, formatted_picture->data, formatted_picture->linesize, + 0, ost->resample_height, resampling_dst->data, resampling_dst->linesize); + } ++#endif + + if (ost->video_pad) { + av_picture_pad((AVPicture*)final_picture, (AVPicture *)padding_src, +@@ -1654,10 +1912,21 @@ + fprintf(stderr, "Cannot get resampling context\n"); + exit(1); + } ++#if ENABLE_AVFILTER ++ ost->resample_height = icodec->height; ++#else + ost->resample_height = icodec->height - (frame_topBand + frame_bottomBand); ++#endif + } + ost->encoding_needed = 1; + ist->decoding_needed = 1; ++ ++#if ENABLE_AVFILTER ++ if (configure_filters(ist, ost)) { ++ fprintf(stderr, "Error opening filters!\n"); ++ exit(1); ++ } ++#endif + break; + case CODEC_TYPE_SUBTITLE: + ost->encoding_needed = 1; +@@ -3674,6 +3943,9 @@ + #ifdef CONFIG_VHOOK + { "vhook", HAS_ARG | OPT_EXPERT | OPT_VIDEO, {(void*)add_frame_hooker}, "insert video processing module", "module" }, + #endif ++#if ENABLE_AVFILTER ++ { "vfilters", OPT_STRING | HAS_ARG, {(void*)&vfilters}, "video filters", "filter list" }, ++#endif + { "intra_matrix", HAS_ARG | OPT_EXPERT | OPT_VIDEO, {(void*)opt_intra_matrix}, "specify intra matrix coeffs", "matrix" }, + { "inter_matrix", HAS_ARG | OPT_EXPERT | OPT_VIDEO, {(void*)opt_inter_matrix}, "specify inter matrix coeffs", "matrix" }, + { "top", HAS_ARG | OPT_EXPERT | OPT_VIDEO, {(void*)opt_top_field_first}, "top=1/bottom=0/auto=-1 field first", "" },

1 0

[soc]: r1422 - libavfilter/avfilter.c
by vitor 29 Oct '07

29 Oct '07

Author: vitor Date: Mon Oct 29 17:00:27 2007 New Revision: 1422 Log: Check for error so that avfilter_open(av_filter_get_by_name(...etc...)) fails gracefully. Modified: libavfilter/avfilter.c Modified: libavfilter/avfilter.c ============================================================================== --- libavfilter/avfilter.c (original) +++ libavfilter/avfilter.c Mon Oct 29 17:00:27 2007 @@ -363,7 +363,12 @@ static const char *filter_name(void *p) AVFilterContext *avfilter_open(AVFilter *filter, char *inst_name) { - AVFilterContext *ret = av_malloc(sizeof(AVFilterContext)); + AVFilterContext *ret; + + if (!filter) + return 0; + + ret = av_malloc(sizeof(AVFilterContext)); ret->av_class = av_mallocz(sizeof(AVClass)); ret->av_class->item_name = filter_name;

1 0

[soc]: r1421 - eac3/eac3dec.c
by bwolowiec 29 Oct '07

29 Oct '07

Author: bwolowiec Date: Sun Oct 28 12:24:34 2007 New Revision: 1421 Log: Small changes, simplify Modified: eac3/eac3dec.c Modified: eac3/eac3dec.c ============================================================================== --- eac3/eac3dec.c (original) +++ eac3/eac3dec.c Sun Oct 28 12:24:34 2007 @@ -124,11 +124,7 @@ static void spectral_extension(EAC3Conte for (bnd = 0; bnd < s->nspxbnds; bnd++){ bandsize = s->spxbndsztab[bnd]; nratio = ((spxmant + 0.5*bandsize) / spxbandtable[s->spxendf]) - noffset[ch]; - if (nratio < 0.0) - nratio = 0.0; - else - if (nratio > 1.0) - nratio = 1.0; + nratio = FFMAX(FFMIN(nratio, 1.0), 0.0); nblendfact[ch][bnd] = sqrt(nratio); sblendfact[ch][bnd] = sqrt(1 - nratio); spxmant += bandsize; @@ -161,7 +157,7 @@ static void spectral_extension(EAC3Conte /* apply notch at all other wrap points */ for (bnd = 1; bnd < s->nspxbnds; bnd++){ if (wrapflag[bnd]) { - filtbin = filtbin - 5; + filtbin -= 5; for (bin = 0; bin < 3; bin++){ s->transform_coeffs[ch][filtbin] *= ff_eac3_spxattentab[s->spxattencod[ch]][bin]; filtbin++; @@ -328,6 +324,9 @@ static void get_eac3_transform_coeffs_ch if (s->chahtinu[ch] != 0) { idct_transform_coeffs_ch(s, ch, blk); } + + memset(s->transform_coeffs[ch]+s->endmant[ch], 0, + sizeof(s->transform_coeffs[0])-s->endmant[ch]); } static int parse_bsi(GetBitContext *gbc, EAC3Context *s){ @@ -368,7 +367,7 @@ static int parse_bsi(GetBitContext *gbc, s->bsid = get_bits(gbc, 5); if (s->bsid < 11 || s->bsid > 16) { - av_log(s->avctx, AV_LOG_ERROR, "bsid should be between 11 and 16\n"); + av_log(s->avctx, AV_LOG_ERROR, "bsid is not within 11 and 16\n"); return -1; } @@ -572,7 +571,7 @@ static int parse_audfrm(GetBitContext *g s->ncplblks += s->cplinu[blk]; } } else { - memset(s->cplinu, 0, sizeof(int) * ff_eac3_blocks[s->numblkscod]); + memset(s->cplinu, 0, sizeof(*s->cplinu) * ff_eac3_blocks[s->numblkscod]); s->ncplblks = 0; } @@ -623,8 +622,7 @@ static int parse_audfrm(GetBitContext *g s->chahtinu[ch] = (nchregs == 1) && get_bits1(gbc); } } else { - for (ch = 0; ch <= s->num_channels; ch++) - s->chahtinu[ch] = 0; + memset(s->chahtinu, 0, sizeof(s->chahtinu)); } /* Audio frame SNR offset data */ if (!s->snroffststr) { @@ -1095,18 +1093,13 @@ static int parse_audblk(GetBitContext *g } if (s->snroffststr) { - av_log(s->avctx, AV_LOG_INFO, "NOT TESTED\n"); if (!blk || get_bits1(gbc)) { int csnroffst = (get_bits(gbc, 6) - 15) << 4; - if (s->snroffststr == 1) { - int snroffst = (csnroffst + get_bits(gbc, 4)) << 2; - for (ch = !s->cplinu[blk]; ch <= s->num_channels; ch++) - s->snroffst[ch] = snroffst; - } else { - if (s->snroffststr == 2) { - for (ch = !s->cplinu[blk]; ch <= s->num_channels; ch++) - s->snroffst[ch] = (csnroffst + get_bits(gbc, 4)) << 2; - } + int snroffst = 0; + for (i = !s->cplinu[blk]; ch <= s->num_channels; ch++){ + if (ch == !s->cplinu[blk] || s->snroffststr == 2) + snroffst = (csnroffst + get_bits(gbc, 4)) << 2; + s->snroffst[ch] = snroffst; } } } @@ -1171,40 +1164,32 @@ static int parse_audblk(GetBitContext *g /* run bit allocation */ for (ch = !s->cplinu[blk]; ch <= s->num_channels; ch++) { - int start=0, end=0; - start = s->strtmant[ch]; - end = s->endmant[ch]; - - ff_ac3_bit_alloc_calc_psd((int8_t *)s->dexps[ch], start, end, - s->psd[ch], s->bndpsd[ch]); + ff_ac3_bit_alloc_calc_psd((int8_t *)s->dexps[ch], s->strtmant[ch], + s->endmant[ch], s->psd[ch], s->bndpsd[ch]); s->bit_alloc_params.fscod = s->fscod; s->bit_alloc_params.halfratecod = 0; ff_ac3_bit_alloc_calc_mask(&s->bit_alloc_params, - s->bndpsd[ch], start, end, s->fgain[ch], + s->bndpsd[ch], s->strtmant[ch], s->endmant[ch], s->fgain[ch], (ch == s->lfe_channel), s->deltbae[ch], s->deltnseg[ch], s->deltoffst[ch], s->deltlen[ch], s->deltba[ch], s->mask[ch]); if (s->chahtinu[ch] == 0) - ff_ac3_bit_alloc_calc_bap(s->mask[ch], s->psd[ch], start, end, - s->snroffst[ch], s->bit_alloc_params.floor, ff_ac3_baptab, + ff_ac3_bit_alloc_calc_bap(s->mask[ch], s->psd[ch], s->strtmant[ch], + s->endmant[ch], s->snroffst[ch], s->bit_alloc_params.floor, ff_ac3_baptab, s->bap[ch]); else if (s->chahtinu[ch] == 1) - ff_ac3_bit_alloc_calc_bap(s->mask[ch], s->psd[ch], start, end, + ff_ac3_bit_alloc_calc_bap(s->mask[ch], s->psd[ch], s->strtmant[ch], s->endmant[ch], s->snroffst[ch], s->bit_alloc_params.floor, ff_ac3_hebaptab, s->hebap[ch]); } got_cplchan = 0; - // TODO only for debug - for (ch = 0; ch <= s->num_channels; ch++) - memset(s->transform_coeffs[ch], 0, 256*sizeof(float)); - /* Quantized mantissa values */ for (ch = 1; ch <= s->num_channels; ch++) { get_eac3_transform_coeffs_ch(gbc, s, blk, ch, &m); @@ -1227,8 +1212,7 @@ static int parse_audblk(GetBitContext *g /** * Performs Inverse MDCT transform */ -static void do_imdct(EAC3Context *ctx) -{ +static void do_imdct(EAC3Context *ctx){ int ch; for (ch = 1; ch <= ctx->nfchans + ctx->lfeon; ch++) { @@ -1276,7 +1260,7 @@ static int eac3_decode_frame(AVCodecCont avctx->sample_rate = ff_ac3_freqs[c->fscod]; } - avctx->bit_rate = (c->frmsiz * (avctx->sample_rate) * 16 / ( ff_eac3_blocks[c->numblkscod] * 256)) / 1000; + avctx->bit_rate = (c->frmsiz * avctx->sample_rate * 16 / ( ff_eac3_blocks[c->numblkscod] * 256)) / 1000; /* channel config */ if (!avctx->request_channels) {

3 2

[soc]: r1420 - libavfilter/diffs/00_build.diff
by vitor 24 Oct '07

24 Oct '07

Author: vitor Date: Wed Oct 24 20:16:59 2007 New Revision: 1420 Log: Fix compilation. Thanks for Jeremy Mordkoff for the bugfix. Modified: libavfilter/diffs/00_build.diff Modified: libavfilter/diffs/00_build.diff ============================================================================== --- libavfilter/diffs/00_build.diff (original) +++ libavfilter/diffs/00_build.diff Wed Oct 24 20:16:59 2007 @@ -59,13 +59,12 @@ Index: Makefile all: lib $(PROGS) $(ALL_TARGETS-yes) lib: -@@ -76,7 +82,10 @@ - endif +@@ -77,6 +83,9 @@ ifeq ($(CONFIG_SWSCALER),yes) $(MAKE) -C libswscale all + endif +ifeq ($(CONFIG_AVFILTER),yes) + $(MAKE) -C libavfilter all - endif +endif ffmpeg_g$(EXESUF): ffmpeg.o cmdutils.o .libs

1 0

avfilter Makefile
by Jeremy Mordkoff 24 Oct '07

24 Oct '07

After applying the diffs, I got a top level Makefile with this: ifeq ($(CONFIG_SWSCALER),yes) $(MAKE) -C libswscale all ifeq ($(CONFIG_AVFILTER),yes) $(MAKE) -C libavfilter all endif endif and it would not link because it never built libavfilter. I changed it to ifeq ($(CONFIG_SWSCALER),yes) $(MAKE) -C libswscale all endif ifeq ($(CONFIG_AVFILTER),yes) $(MAKE) -C libavfilter all endif and it builds and runs Was this dependency on swscaler intentional? JLM ____________________ Zazzle Technologies, Inc 238 Littleton Road, Suite 200 Westford, MA 01886 978-467-1395 x233

3 2

[soc]: r1419 - qcelp/qcelpdec.c
by reynaldo 22 Oct '07

22 Oct '07

Author: reynaldo Date: Mon Oct 22 22:12:32 2007 New Revision: 1419 Log: TRIVIAL - Avoid redoing is_ifq check if unneded Modified: qcelp/qcelpdec.c Modified: qcelp/qcelpdec.c ============================================================================== --- qcelp/qcelpdec.c (original) +++ qcelp/qcelpdec.c Mon Oct 22 22:12:32 2007 @@ -803,7 +803,7 @@ static int qcelp_decode_frame(AVCodecCon { if(n>3) // Random seed cbseed |= (uint16_t)q->frame->data[ order[n].index ]<<(n-4); - if(n<16 && !q->frame->data[ order[n].index ]) is_ifq = 0; + if(n<16 && is_ifq && !q->frame->data[ order[n].index ]) is_ifq = 0; } }

1 0

[soc]: r1418 - qcelp/qcelpdec.c
by reynaldo 21 Oct '07

21 Oct '07

Author: reynaldo Date: Sun Oct 21 20:36:03 2007 New Revision: 1418 Log: Fix RATE_OCTAVE random seed computation and check for IFQ in the first 16 bits. Both changes from Michael's review. Modified: qcelp/qcelpdec.c Modified: qcelp/qcelpdec.c ============================================================================== --- qcelp/qcelpdec.c (original) +++ qcelp/qcelpdec.c Sun Oct 21 20:36:03 2007 @@ -708,9 +708,9 @@ static int qcelp_decode_frame(AVCodecCon { QCELPContext *q = avctx->priv_data; const QCELPBitmap *order = NULL; - int16_t *outbuffer = data, cbseed; + int16_t *outbuffer = data; int i, n, is_ifq = 0, is_codecframe_fmt = 0; - uint16_t first16 = 0; + uint16_t first16 = 0, cbseed = 0; float qtzd_lspf[10], gain[16], cdn_vector[160], ppf_vector[160], lpc[10]; float interpolated_lspf[10]; int g0[16], index[16]; @@ -753,6 +753,14 @@ static int qcelp_decode_frame(AVCodecCon q->frame->rate = RATE_OCTAVE; q->frame->bits = qcelp_bits_per_rate[RATE_OCTAVE]; order = QCELP_REFERENCE_FRAME + QCELP_8THRPKT_REFERENCE_POS; + + /* + * We assume its IFQ unless we find at least one '0' + * in the first 16 bits, this check is performed as part of + * the reordering loop that follows + */ + + is_ifq = 1; break; case 1: is_codecframe_fmt=1; @@ -791,12 +799,11 @@ static int qcelp_decode_frame(AVCodecCon q->frame->data[ order[n].index ] |= get_bits1(&q->gb)<<order[n].bitpos; - if(n<20) + if(q->frame->rate == RATE_OCTAVE) { - if(n>3) // This is the random seed for rate 1/8 frames - cbseed |= q->frame->data[ order[n].index ]>>n; - if(n<16) // This is for a rate 1/8 only sanity check - first16 |= q->frame->data[ order[n].index ]>>n; + if(n>3) // Random seed + cbseed |= (uint16_t)q->frame->data[ order[n].index ]<<(n-4); + if(n<16 && !q->frame->data[ order[n].index ]) is_ifq = 0; } }

1 0