Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
From: Paul B Mahol <onemda@gmail.com>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] drawtext filter
Date: Fri, 27 Jan 2023 18:31:08 +0100
Message-ID: <CAPYw7P4+jady6OKD8R5LE0JcdQBqc68ddyA4eMjfLaTW3YP3WQ@mail.gmail.com> (raw)
In-Reply-To: <47ee718c-bce0-cffa-9c6a-2889a6547017@tiscali.it>

On 1/27/23, Francesco Carusi <klimklim@tiscali.it> wrote:
> On 26/01/2023 17:37, Paul B Mahol wrote:
>> On 1/26/23, Francesco Carusi <klimklim@tiscali.it> wrote:
>>> On 26/01/2023 14:21, Paul B Mahol wrote:
>>>> On 1/26/23, Francesco Carusi <klimklim@tiscali.it> wrote:
>>>>> The drawtext reinit command is also used in the docs as an example for
>>>>> the sendcmd filter, so I thought it was fine to use commands in that
>>>>> way. In my opinion it is also a convenient way to modify multiple
>>>>> options at the same time.
>>>>> Should the command match the name of a filter option instead?
>>>>>
>>>> Please do not top post.
>>>>
>>>> It is much better to use already existing options for commands that is
>>>> more intuitive to users. Also multiple options can be set at runtime,
>>>> there is no such limitation.
>>> ok, I'm going to remove the "change" command and add commands that match
>>> the options that it included.
>> Thanks, feel free to ask questions on #ffmpeg-devel irc channel.
> I'm attaching the updated patch, I also updated the document at
> https://github.com/yethie/FFmpeg/blob/master/drawtext/CHANGES.md
> Thanks

Amazing, I like demos!

Could improve code style of newly added/changed lines?
For example opening { put on separate line. So code style is in sync
with rest of codebase.

The commands stuff does not need to use strcmp to detect if option
value have been changed, you could avoid strcmp by just caching old
value prior to calling function the picks new values, and after that
just compare old with new and then if it differs call needed code.

>>
>>>>> On 26/01/2023 11:50, Paul B Mahol wrote:
>>>>>> On 1/26/23, Francesco Carusi <klimklim@tiscali.it> wrote:
>>>>>>> Hi, I'm new to contributing to ffmpeg!
>>>>>>>
>>>>>>> I modified the drawtext filter to improve text rendering and add
>>>>>>> some
>>>>>>> features. You can find a high level description of the changes at
>>>>>>> this
>>>>>>> link:
>>>>>>>
>>>>>>> https://github.com/yethie/FFmpeg/blob/master/drawtext/CHANGES.md
>>>>>>>
>>>>>>> I'm also attaching the patch file.
>>>>>>> I looked for the filter maintainer to discuss about the changes I
>>>>>>> made
>>>>>>> but it looks like there isn't any, am I correct?
>>>>>>>
>>>>>>> Please let me know if this is the right way to submit my
>>>>>>> contribution.
>>>>>> Why filter can not support normal commands for options? Like most/all
>>>>>> other filters that have support for changing options values at
>>>>>> runtime.
>>>>>>
>>>>>> The reinit and yours added change option(s) are very
>>>>>> strange/inconvenient things to do.
>>>>>> _______________________________________________
>>>>>> ffmpeg-devel mailing list
>>>>>> ffmpeg-devel@ffmpeg.org
>>>>>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>>>>>
>>>>>> To unsubscribe, visit link above, or email
>>>>>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>>>>> _______________________________________________
>>>>> ffmpeg-devel mailing list
>>>>> ffmpeg-devel@ffmpeg.org
>>>>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>>>>
>>>>> To unsubscribe, visit link above, or email
>>>>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>>>>>
>>>> _______________________________________________
>>>> ffmpeg-devel mailing list
>>>> ffmpeg-devel@ffmpeg.org
>>>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>>>
>>>> To unsubscribe, visit link above, or email
>>>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>>> _______________________________________________
>>> ffmpeg-devel mailing list
>>> ffmpeg-devel@ffmpeg.org
>>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>>
>>> To unsubscribe, visit link above, or email
>>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>>>
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

  reply	other threads:[~2023-01-27 17:31 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-26 10:40 Francesco Carusi
2023-01-26 10:50 ` Paul B Mahol
2023-01-26 13:17   ` Francesco Carusi
2023-01-26 13:21     ` Paul B Mahol
2023-01-26 16:25       ` Francesco Carusi
2023-01-26 16:37         ` Paul B Mahol
2023-01-27 17:18           ` Francesco Carusi
2023-01-27 17:31             ` Paul B Mahol [this message]
2023-01-28 14:45               ` Francesco Carusi
2023-01-28 15:32                 ` Paul B Mahol
2023-01-30 10:48                   ` Francesco Carusi
2023-01-30 12:08                     ` Anton Khirnov
2023-01-30 12:19                     ` Paul B Mahol
2023-02-03 14:18                       ` Francesco Carusi
2023-03-14 15:58                         ` Francesco Carusi
2023-03-14 16:07                           ` Paul B Mahol
2023-03-16 16:52                         ` Anton Khirnov
2023-03-20  7:41                           ` Francesco Carusi
2023-03-20  8:50                             ` Paul B Mahol
2023-05-26 12:19                               ` Francesco Carusi
2023-06-02 17:12                                 ` Paul B Mahol
2023-09-11 10:01                                 ` Paul B Mahol

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAPYw7P4+jady6OKD8R5LE0JcdQBqc68ddyA4eMjfLaTW3YP3WQ@mail.gmail.com \
    --to=onemda@gmail.com \
    --cc=ffmpeg-devel@ffmpeg.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git