[FFmpeg-devel] [PATCH] avfilter: add dynoverlay filter.

[Paul B Mahol]

On 9/23/16, Priebe, Jason <jpriebe at cbcnewmedia.com> wrote:
> Thanks for the detailed review of the code, Paul.  You caught some dumb
> mistakes that I should have picked up on before I sent the submission.
>
>> dynoverlay sounds very powerful, but this filter funcionality is very
>> limited.
>
> True, it is limited, but it does something that no other filter does (I am
> interested in your named pipe idea, though -- see note at the end).
>
>> What's up with code identation?
>
> I don't know -- I didn't realize there was a problem.  I'm just using vi;
> maybe
> it formatted stuff in a funny way.  Or maybe you don't like my braces on new
> lines?
>
>> > +If the named PNG file does not exist, the filter will do nothing.
>>
>> I do not like this design. Supporting only PNG with RGBA, working only
>> in YUV420P.
>
> I built the filter to satisfy a specific need; I thought that maybe others
> who needed other formats could add in the support for those formats later.
> I thought I saw some filters in ffmpeg that only support specific formats,
> so I didn't realize that was a problem.
>
>> > + at item check_interval
>> > +(optional) The interval (in ms) between checks for updates to the
>> > overlay
>> > file. For
>> > +efficiency, the filter does not check the filesystem on every frame.
>> > You
>> > can make
>> > +it check more frequently (less efficient, but more responsive to
>> > changes in
>> > the
>> > +overlay PNG) by specifying a lower number. Or you can make it check
>> > less
>> > frequently
>> > +(more efficient, but less responsive to changes in the overlay PNG) by
>> > specifying
>> > +a higher number.
>> > +
>> > +Default value is @code{250}.
>> > + at end table
>>
>> This approach is bad to provide such functionality.
>
> Why?  Yes, it's a lot of system calls, but it's still performant, so is
> it fundamentally wrong?
>
>> > + -vf format=pix_fmts=yuv420p \
>> > + -vf dynoverlay=overlayfile=/var/tmp/overlay.png:check_interval=100 \
>>
>> You can not give more than one -vf's, only one will ever be used.
>
> My mistake - I was trying to take a much more complicated example and
> distill it
> down and anonymize it.
>
>> > +#include <unistd.h>
>> > ... many headers omitted ...
>> > +#include "libavutil/lfg.h"
>>
>> Are all those headers really needed?
>
> Probably not; I could try to pare it down.
>
>> > + if (ctx->overlay_frame)
>> > + {
>> > + // TODO - make sure we have freed everything
>> > + av_freep(&ctx->overlay_frame->data[0]);
>> > + }
>> > + ctx->overlay_frame = NULL;
>>
>> This is wrong.
>
> Thanks for catching this; I should use av_frame_free(), right?
>
>> > + if (ctx->overlay_frame)
>> > + {
>>
>> Check is not needed.
>>
>> > + av_frame_free (&ctx->overlay_frame);
>> > + }
>> > + ctx->overlay_frame = NULL;
>>
>> Not needed.
>
> Does this mean that it is safe to call av_frame_free() with NULL ?
>
>> I think much better approach is to use named pipes, probably as
>> separate video filter source.
>
> I would love to see an example of this technique.  Based on what I read
> online
> (http://ffmpeg.gusari.org/viewtopic.php?f=11&t=2774 for example), I didn't
> think this technique worked.  And even if it could work, wouldn't I need a
> process
> that is constantly generating frame data from PNGs to feed it to ffmpeg,
> which
> then has to read it 30 times a second (when it only changes every 2-3
> *minutes*).

Named pipe approach would implement video source which would read images
from named pipe. It would read from named pipe until it decodes single frame
and then would use that frame as input to next filter, for example
overlay filter.
If it encounters EOF in named pipe it would not abort but would instead keep
sending last frame it got, for example completely transparent frame.
If it suddenly get more data from pipe it would update its internal
frame and output
it as input to next filter in chain.

So command would look like this:

imagepipe=named_pipe:rate=30[o],[0:v][o]overlay=x=0:y=0 ...

And than in another terminal, you would use commands like this:

cat random_image.format > named_pipe