[FFmpeg-devel] [PATCH v8 09/13] avfilter/textmod: Add textmod, censor and show_speaker filters

Soft Works softworkz at hotmail.com
Wed Sep 22 06:36:07 EEST 2021



> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of Andreas
> Rheinhardt
> Sent: Wednesday, 22 September 2021 05:08
> To: ffmpeg-devel at ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v8 09/13] avfilter/textmod: Add textmod,
> censor and show_speaker filters
> 

[..]
 
> This function is horrible (and I already wanted to deprecate it, but for
> this I would need to fix all current users of it): It automatically
> frees the array on error and it returns void, thereby encouraging a
> programming style in which errors are not checked. (You are checking
> them.) The automatic free on error means that there are basically only
> two usecases for it (unless one just ignores any errors): To store
> pointers to objects that don't need to be freed at all (like static
> objects) or to store non-ownership pointers, like pointers to substrings
> of a string. Now that you started strduping the strings you are no
> longer of this type. In other words, there will be leaks on error.
> 
> Anyway, I don't see why you went the route of using individually
> allocated strings.

[..]

> Leak on error, as above.
> (Notice that you do not need to allocate the replace_list entries
> individually; all you need to do is strdup s->find before it is split
> into little pieces. Then you can get the offset of the nth replace
> string in the duplicate of s->find by s->find_list[n] - s->find.)
> Finally, when dealing with the replace list you already know in advance
> how many entries there will be, so you don't have to reallocate.

The reason why I changed to individual allocation is that eventually 
I want to be able to:

- Split by multiple separators, e.g. '\r', '\n' in addition to the 
  chosen separator
- Remove empty entries (when two separators follow after each other)
- Trim leading and trailing whitespace from each entry

Doing all these things with pointers to the full string seems to be 
too much of a hazzle and error prone.

> > + *
> > + *  ffmpeg -ss 00:04:30 -i
> "U:\TestMedia\subtitle_burnin\subtitlevideo.mkv" -filter_complex
> "show_speaker=style='{\\c&HDD0000&\\be1\\\i1\\bord10}:line_break=1',[0:v]over
> lay_textsubs" -map 0 -c:v libx265 output.mkv
> > + */
> 
> These examples belong into filters.texi.

They are in filters.texi.
I left them here as well for easier reference, but I can remove these.

Thanks again,
softworkz


More information about the ffmpeg-devel mailing list