[FFmpeg-devel] [PATCH] libavfilter-soc?: implement fish filter

[Michael Niedermayer]

On Mon, Jun 01, 2009 at 07:08:27PM +0200, Stefano Sabatini wrote:
> 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()?

you added that, if it cant be used (cleanly) then dont use it.


> 
> There's no way to set a color directly using av_set_string(), same for

and?

Question is not "how to use X" but "Is X the right tool at all"
and the awnser seems AFIAKS to be a "no"


> 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.

So because you want to use a pencil on a nail you suggest
to extend the pencil with a heavy piece of iron, well why not
use a hammer to begin with?

Theres sscanf() and you can use the color API directly [in a seperate patch
once fish is ported]



>  
> > 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.

if you arent willing to port the fish filter to libavfilter then
ok, no problem

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I have never wished to cater to the crowd; for what I know they do not
approve, and what they approve I do not know. -- Epicurus
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090601/13ba9c17/attachment.pgp>