[FFmpeg-devel] [PATCH] libavfilter-soc?: implement fish filter
Stefano Sabatini
stefano.sabatini-lala
Mon Jun 1 19:08:27 CEST 2009
On date Monday 2009-06-01 17:42:11 +0200, Michael Niedermayer encoded:
> On Mon, Jun 01, 2009 at 04:11:02PM +0200, Stefano Sabatini wrote:
> > On date Monday 2009-06-01 14:04:05 +0200, Stefano Sabatini encoded:
> > > On date Monday 2009-06-01 13:56:14 +0200, Stefano Sabatini encoded:
> > > > On date Monday 2009-06-01 11:51:56 +0200, Michael Niedermayer encoded:
> > > > > On Sun, May 31, 2009 at 05:12:56PM +0200, Stefano Sabatini wrote:
> > > > > > On date Saturday 2009-05-30 22:04:52 +0200, Michael Niedermayer encoded:
> > > > > [...]
> > > > > > > > +
> > > > > > > > +typedef struct {
> > > > > > > > + const AVClass *class;
> > > > > > > > +
> > > > > > > > + int w, h;
> > > > > > >
> > > > > > > > + char *h_range, *s_range, *v_range;
> > > > > > >
> > > > > > > They are ultimately not strings
> > > > > >
> > > > > > Yes, but then how should I put them in the context when using
> > > > > > av_set_options_string()? (BTW this is the same "trick" used to set an
> > > > > > expression).
> > > > >
> > > > > the ranges can be set to expressions that get evaluated for each frame?
> > > >
> > > > I believe so and that would be cool, that could be:
> > > > expr='between(h, 100, 200) * between(v, 100, 255)'
> > > > or
> > > > expr='between(h, mod(N), N+100) * between(v, 100, 255)'
> > > >
> > > > I'm supposing speed is not an issue for this filter, right?
> > >
> > > After one minute of thinking, having h_first, h_second, v_first,
> > > v_second, ...
> > >
> > > would require just a per-frame evaluation, that's better than to
> > > evaluate an expression for each pixel.
> >
> > Patch updated, I'm not yet satisfied with how the S/V ranges are
> > dealt, what about to normalize S/V ranges this way:
> > 100:70 -> 70:70
> > -5:5 -> 0:5
> > 250:260 -> 250:255
> > ?
> [...]
> > +typedef struct {
> > + const AVClass *class;
> > +
> > + int w, h;
> > +
> > + double var_values[VARS_NB];
> > +
> > + char *h_first_expr, *h_second_expr,
> > + *s_min_expr, *s_max_expr,
> > + *v_min_expr, *v_max_expr;
> > +
>
> > + AVEvalExpr *h_first_evalexpr, *h_second_evalexpr,
> > + *s_min_evalexpr, *s_max_evalexpr,
> > + *v_min_evalexpr, *v_max_evalexpr;
>
> doesnt need to be in the context
>
>
>
> > +
> > + int hsub, vsub;
> > +
>
> > + char *zap_color_str; ///< if specified, not matched pixels will be set to zap_color
>
> same issue as ive mentioned in the previous review
>
> to evaluate an expression per frame, the corresponding char* expression
> OR the AVEvalExpr must be kept but NOT both
>
> to parse a single int [4] once you do not need the string
So I have to repeat again the question:
then how should I put them in the context when using
av_set_options_string()?
There's no way to set a color directly using av_set_string(), same for
an expression, that's why I need to store the string in the context
and *then* convert it to the target struct, I can't see how to avoid
that but extending opt.c.
> your code is a mess ...
>
> iam not sure what to do here.
> The original fish filter did not do any of that, also i think i should
> remind you that porting code and any other change unrelated to porting must
> be in seperate patches.
That would be more work for small gain, I'm not sure I'm willing to do
that.
> This is even more important once it becomes obvious that some of these
> unrelated changes are considered messy by some other developer
Regards.
--
FFmpeg = Freak & Fundamental Mere Peaceful Erotic Geek
More information about the ffmpeg-devel
mailing list