[FFmpeg-devel] [PATCH] lavfi test pattern generator

[Stefano Sabatini]

On date Saturday 2011-06-18 20:20:09 +0200, Nicolas George encoded:
> Le decadi 30 prairial, an CCXIX, Stefano Sabatini a écrit :
> > +    char *size;                 ///< video frame size
> > +    char *rate;                 ///< video frame rate
> > +    char *duration;             ///< total duration of the generated video
> 
> Isn't it a bother to have these fields in the context while their only use
> is to be immediately parsed into other fields?
> 
> > +    { "size",     "Set video size, accept a string describing frame size, such as 640x480 or hd720.", OFFSET(size), FF_OPT_TYPE_STRING, {.str = "320x240"}, 0, 0 },
> 
> Nit: describing *the* frame size, maybe. And possibly set *the* video size.

Uhm, here I did like in the rest of the documentation, not sure what
is more correct but I prefer to keep consistency here.

> > +    if ((ret = (av_set_options_string(test, args, "=", ":"))) < 0) {
> 
> With current Git, this segfaults with '-vf test', but works with '-vf
> test=', because av_set_options_string assumes that args is not null.
> 
> A potential fix would be to add near the beginning of av_set_options_string:
> 
>     if (!opts)
> 	return 0;
> 
> I do not know if you have something in that sense in your working tree.

Sent a separate patch for that.
 
> > + * Fill rectangle with value val.
> 
> Fill *a* rectangle.

Fixed.
 
> The rests looks like worthy enhancement on top of my old patch. Thanks for
> bringing it back.

BTW, do you have suggestions for a better name for the source? In case
we want to add more test sources (e.g. like that used in the vsynth
tests).
-- 
FFmpeg = Furious and Frenzy Mythic Patchable Educated Gymnast
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0005-lavfi-add-test-source.patch
Type: text/x-diff
Size: 13644 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20110619/848ff55b/attachment.bin>