Pinging for comments / review...
On Tue, Apr 4, 2017 at 3:45 PM, Brett Harrison
wrote:
> Resurrecting this patch.
>
> On Thu, Sep 15, 2016 at 3:20 AM, Michael Niedermayer <
> mich...@niedermayer.cc> wrote:
>
>> On Fri, Sep 09, 2016 at 05:26:03PM -0700, Brett Harrison wrote:
>> > Here are the cha
Resurrecting this patch.
On Thu, Sep 15, 2016 at 3:20 AM, Michael Niedermayer wrote:
> On Fri, Sep 09, 2016 at 05:26:03PM -0700, Brett Harrison wrote:
> > Here are the changes requested
> [...]
> > +static av_cold int parse_fontsize(AVFilterContext *ctx)
> > +{
> > +DrawTextContext *s = ctx-
On Fri, Sep 09, 2016 at 05:26:03PM -0700, Brett Harrison wrote:
> Here are the changes requested
[...]
> +static av_cold int parse_fontsize(AVFilterContext *ctx)
> +{
> +DrawTextContext *s = ctx->priv;
> +int err;
> +
> +if (s->fontsize_expr == NULL)
> +return AVERROR(EINVAL);
>
Any feedback on the most recent patch?
On Fri, Sep 9, 2016 at 5:26 PM, Brett Harrison
wrote:
> Here are the changes requested
>
> On Thu, Sep 8, 2016 at 8:50 AM, Michael Niedermayer <
> mich...@niedermayer.cc> wrote:
>
>> On Tue, Sep 06, 2016 at 10:27:24AM -0700, Brett Harrison wrote:
>> > This
Here are the changes requested
On Thu, Sep 8, 2016 at 8:50 AM, Michael Niedermayer
wrote:
> On Tue, Sep 06, 2016 at 10:27:24AM -0700, Brett Harrison wrote:
> > This patch addresses your concerns.
> >
> > On Fri, Sep 2, 2016 at 5:05 PM, Michael Niedermayer
>
> > wrote:
> >
> > > On Fri, Sep 02,
On Tue, Sep 06, 2016 at 10:27:24AM -0700, Brett Harrison wrote:
> This patch addresses your concerns.
>
> On Fri, Sep 2, 2016 at 5:05 PM, Michael Niedermayer
> wrote:
>
> > On Fri, Sep 02, 2016 at 03:31:21PM -0700, Brett Harrison wrote:
> > > Addressed the following concerns.
> > >
> > > ===
> >
This patch addresses your concerns.
On Fri, Sep 2, 2016 at 5:05 PM, Michael Niedermayer
wrote:
> On Fri, Sep 02, 2016 at 03:31:21PM -0700, Brett Harrison wrote:
> > Addressed the following concerns.
> >
> > ===
> >
> > >libavfilter/vf_drawtext.c: In function ‘update_fontsize’:
> > >libavfilter/v
On Fri, Sep 02, 2016 at 03:31:21PM -0700, Brett Harrison wrote:
> Addressed the following concerns.
>
> ===
>
> >libavfilter/vf_drawtext.c: In function ‘update_fontsize’:
> >libavfilter/vf_drawtext.c:422:5: warning: ISO C90 forbids mixed declarations
> >and code [->Wdeclaration-after-statement]
Addressed the following concerns.
===
>libavfilter/vf_drawtext.c: In function ‘update_fontsize’:
>libavfilter/vf_drawtext.c:422:5: warning: ISO C90 forbids mixed declarations
>and code [->Wdeclaration-after-statement]
Fixed this.
>also patch breaks:
>./ffmpeg -i m.mpg -vf
>>drawtext=fontfile
Le septidi 17 fructidor, an CCXXIV, Moritz Barsnick a écrit :
> *Assuming* he means "assign update_fontsize()'s return value to ret,
> and check whether ret is != 0", which would correspond to the more
> verbose
> if ((ret = update_fontsize(ctx)) != 0) {
>
> is it sufficient to say:
> if (ret
On Fri, Sep 02, 2016 at 14:20:37 +0200, Nicolas George wrote:
> You mean the parentheses? Well, if you are not sure, then the parentheses
> are necessary. We are not playing golf, we want code that is readable and
> robust.
>
> For reference, comparisons have precedence over assignments. That mean
Le septidi 17 fructidor, an CCXXIV, Moritz Barsnick a écrit :
> Sorry, probably makes sense, equal to
> if ((ret = update_fontsize(ctx)) != 0) {
>
> I'm still not *really* sure whether the second set of brackets is
> required to have "if" check the assigned ret (I thought not). I'll let
>
On Fri, Sep 02, 2016 at 14:10:41 +0200, Moritz Barsnick wrote:
> > +if ((ret = update_fontsize(ctx))) {
> You were meaning to write
>if (ret = update_fontsize(ctx)) {
> or
>if ((ret = update_fontsize(ctx)) < 0) {
> ?? (Too many brackets the way you did it.)
Sorry, probably make
On Thu, Sep 01, 2016 at 18:17:11 -0700, Brett Harrison wrote:
> Most recent patch. I was evaluating fontsize too early before when using
> the 'n' variable in equations.
> +av_log(ctx, AV_LOG_ERROR, "Font not open\n");
I was wondering: Was does this message tell the user?
> +if ((re
On Thu, Sep 01, 2016 at 06:17:11PM -0700, Brett Harrison wrote:
> Most recent patch. I was evaluating fontsize too early before when using
> the 'n' variable in equations.
>
> On Thu, Sep 1, 2016 at 3:44 PM, Brett Harrison
> wrote:
>
> > Any feedback on this newest patch?
> >
> > On Tue, Aug 30
Most recent patch. I was evaluating fontsize too early before when using
the 'n' variable in equations.
On Thu, Sep 1, 2016 at 3:44 PM, Brett Harrison
wrote:
> Any feedback on this newest patch?
>
> On Tue, Aug 30, 2016 at 2:17 PM, Brett Harrison <
> brett.harri...@zyamusic.com> wrote:
>
>> Sin
Any feedback on this newest patch?
On Tue, Aug 30, 2016 at 2:17 PM, Brett Harrison wrote:
> Since there are differing opinions on how the default fontsize should be
> established this patch adds my changes while preserving the current
> behavior when fontsize is not specified.
>
> On Tue, Aug 30
Hi,
On Fri, Aug 26, 2016 at 10:37 PM, Brett Harrison
wrote:
> Allows expr evaluation in the fontsize parameter for drawtext.
Thanks for making this. I regularly use drawtext to create
watermarked/timecoded access copies in a moving image archive and I
have to use workarounds in scripts in order
Since there are differing opinions on how the default fontsize should be
established this patch adds my changes while preserving the current
behavior when fontsize is not specified.
On Tue, Aug 30, 2016 at 2:43 AM, Nicolas George wrote:
> Le tridi 13 fructidor, an CCXXIV, Brett Harrison a écrit
Le tridi 13 fructidor, an CCXXIV, Brett Harrison a écrit :
> Before I fix the patch, can you clarify the intended functionality?
>
> The docs say that 16 is the default fontsize, however if
> CONFIG_LIBFONTCONFIG is configured and ffmpeg if called with:
>
> -vf drawtext=text=abc:fontcolor=white
>
On Mon, Aug 29, 2016 at 02:23:44PM -0700, Brett Harrison wrote:
> Before I fix the patch, can you clarify the intended functionality?
>
> The docs say that 16 is the default fontsize, however if
> CONFIG_LIBFONTCONFIG is configured and ffmpeg if called with:
>
> -vf drawtext=text=abc:fontcolor=wh
Before I fix the patch, can you clarify the intended functionality?
The docs say that 16 is the default fontsize, however if
CONFIG_LIBFONTCONFIG is configured and ffmpeg if called with:
-vf drawtext=text=abc:fontcolor=white
on my system the font used will be /opt/X11/share/fonts/TTF/Vera.ttf (t
On Sat, Aug 27, 2016 at 04:30:05PM -0700, Brett Harrison wrote:
> Fixed patch based on comments.
>
> This time attaching patch file.
>
> On Sat, Aug 27, 2016 at 6:21 AM, Moritz Barsnick wrote:
>
> > On Fri, Aug 26, 2016 at 14:37:42 -0700, Brett Harrison wrote:
> >
> > > +if (diff != 0) {
>
Fixed patch based on comments.
This time attaching patch file.
On Sat, Aug 27, 2016 at 6:21 AM, Moritz Barsnick wrote:
> On Fri, Aug 26, 2016 at 14:37:42 -0700, Brett Harrison wrote:
>
> > +if (diff != 0) {
> > + return diff > 0 ? 1 : diff < 0 ? -1 : 0;
> > +}
>
> If diff != 0, it
On Fri, Aug 26, 2016 at 14:37:42 -0700, Brett Harrison wrote:
> +if (diff != 0) {
> + return diff > 0 ? 1 : diff < 0 ? -1 : 0;
> +}
If diff != 0, it can only be >0 or <0, nothing else:
if (diff != 0)
return diff > 0 ? 1 : -1;
(And you can drop the curly brackets.)
>
On Fri, Aug 26, 2016 at 02:37:42PM -0700, Brett Harrison wrote:
> Allows expr evaluation in the fontsize parameter for drawtext.
>
> examples (fontsize 1/3 of video height):
>
> ffmpeg -i http://i.giphy.com/kwAi4WrChkSfm.gif -vf
> "drawtext=fontfile=/Library/Fonts/Verdana
> Bold.ttf:text='HI':fo
Allows expr evaluation in the fontsize parameter for drawtext.
examples (fontsize 1/3 of video height):
ffmpeg -i http://i.giphy.com/kwAi4WrChkSfm.gif -vf
"drawtext=fontfile=/Library/Fonts/Verdana
Bold.ttf:text='HI':fontcolor=yellow:x=(w-tw)/2:y=(h-th)/1.3:fontsize=h/3"
out.gif
ffmpeg -i http:
27 matches
Mail list logo
