Can I help in any way in advancing this patch?

On 03/02/2023 15:18, Francesco Carusi wrote:


On 30/01/2023 13:19, Paul B Mahol wrote:
On 1/30/23, Francesco Carusi <klimk...@tiscali.it> wrote:
On 28/01/2023 16:32, Paul B Mahol wrote:
On 1/28/23, Francesco Carusi <klimk...@tiscali.it> wrote:
On 27/01/2023 18:31, Paul B Mahol wrote:
On 1/27/23, Francesco Carusi <klimk...@tiscali.it> wrote:
On 26/01/2023 17:37, Paul B Mahol wrote:
On 1/26/23, Francesco Carusi <klimk...@tiscali.it> wrote:
On 26/01/2023 14:21, Paul B Mahol wrote:
On 1/26/23, Francesco Carusi <klimk...@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.
Sure, I'll put opening { on a new line for functions, not for control
statements, like in the rest of the code. Is it fine?
Yes. Thanks.

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.
I'll cache the numeric values. I think that caching string values is not the preferred solution because in addition to the strcmp needed to check the value, it would also need a strdup to cache the previous value, even
when the command does not involve those options. Does it sound
reasonable?
Yes.
I'm attaching the patch that includes the changes we discussed.
space between 'for' and '('

Do not keep old code in comments if its no longer relevant or working.

Ok I added spaces between control statements (if, for, while) and '(', and also cleaned up comments. Following Anton Khirnov suggestion I tried to split the changes into multiple commits. However the first one is quite bit since it contains a major change in how the filter works and cannot be split further.
Patches attached.

On 26/01/2023 11:50, Paul B Mahol wrote:
On 1/26/23, Francesco Carusi <klimk...@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-requ...@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-requ...@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-requ...@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-requ...@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-requ...@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-requ...@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-requ...@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-requ...@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-requ...@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-requ...@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-requ...@ffmpeg.org with subject "unsubscribe".

Reply via email to