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

[Paul B Mahol]

On 9/23/16, Priebe, Jason  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.
>
>> > +@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}.
>> > +@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 
>> > ... 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(>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 (>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=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
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[wm4]

On Thu, 22 Sep 2016 18:45:43 +
"Priebe, Jason"  wrote:

> This patch adds a new filter that allows you to drive dynamic graphic overlays
> on a live encoding by creating/updating/deleting a specified 32-bit PNG.
> This is very different from the overlay filter, because it lets you change
> the overlay in real time during a live stream.  It doesn't allow you to 
> overlay
> video on top of video, but you can overlay still images over video, which is
> useful for things like lower-thirds and fullscreen graphics.

This seems like a very odd approach, and the filter seems to have very
limited usefulness.

Why can't this be a bit more general? Like providing an API to set
overlay images, and letting ffmpeg.c use it to e.g. load from a file in
intervals. Rather than hardcoding yet another CLU use-case in the libs.

We already have a long list of filters that seem to hardcode such
things.

Most importantly, it definitely shouldn't duplicate any blending code.

> +
> +static unsigned long long get_current_time_ms (void)
> +{
> + unsigned long long ms_since_epoch;
> + struct timespec spec;
> +
> + clock_gettime(CLOCK_REALTIME, );

Unportable.

> +
> + ms_since_epoch =
> + (unsigned long long)(spec.tv_sec) * 1000 +
> + (unsigned long long)(spec.tv_nsec) / 1.0e6;
> +
> + return ms_since_epoch;
> +}
> +
> +static int load_overlay (AVFilterContext *fctx)
> +{
> + DynOverlayContext *ctx = fctx->priv;
> +
> + AVFrame *rgba_frame;
> +
> + struct stat attrib;
> + int ret;
> +
> + if ((ret = stat(ctx->overlayfile, )) != 0)

Not sure if portable.

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[Priebe, Jason]

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.

> > +@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}.
> > +@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 
> > ... 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(>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 (>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=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*).

That seems extremely inefficient for overlays that are not changing frequently.

This is why I was trying to read the PNG once, hold it in memory until it 
changes
or is removed.  Maybe it would be more elegant if I wasn't polling the 
filesystem;
I could send signals to ffmpeg instead.  But I don't see a clear advantage to 
that
technique, and there are some disadvantages.  It's extremely simple for an 
external
application to write to a specified file.  But a model based on signalling would
require that the process also know the PID of the ffmpeg process.

I guess my question for you is whether this filter has any value to the larger
community.  If not, I'll just maintain it myself as a patch that I can apply to
my own builds.

Jason Priebe
CBC New Media
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[Paul B Mahol]

On 9/22/16, Priebe, Jason  wrote:
> This patch adds a new filter that allows you to drive dynamic graphic
> overlays
> on a live encoding by creating/updating/deleting a specified 32-bit PNG.
> This is very different from the overlay filter, because it lets you change
> the overlay in real time during a live stream.  It doesn't allow you to
> overlay
> video on top of video, but you can overlay still images over video, which is
> useful for things like lower-thirds and fullscreen graphics.
>
> It is efficient in its handling of PNG files, as it only decodes the PNG
> data
> when it changes.  It is not optimized in its handling of the compositing,
> since
> it composites the entire image on every frame, even if the majority of the
> overlay is fully transparent.  Even with that caveat, it only takes about
> 2% of overall CPU while compositing 1920x1080 images on HD video on a
> Core i7-6700K.
>
> I'm pretty sure that I'm allocating my frames/buffers correctly and that I'm
> freeing them as expected.  But if I've missed something, please let me know.
>
> I did my best with the FATE test.  I understand the concept of "generate
> video, perform the filter, calculate the MD5 of the output, compare to
> expected MD5".  But I didn't really see how I was supposed to visually
> inspect
> the output video before committing the test.
>
> I modified the fate-run.sh script to output the ffmpeg command it was
> running
> so I could capture the output video and make sure it contained what I
> expected.
> So I think my test is good.
>
> It's a very basic test -- just makes sure it can read the PNG and overlay
> it.
> I don't do anything fancy like remove or update the PNG during the encoding,
> although that would be a more complete test of what the filter is designed
> to do.
>
> The test included requires a PNG overlay image, dynoverlay.png.  It can be
> downloaded from http://imgur.com/a/6PIkT
>
>
>
> ---
> Changelog | 1 +
> doc/filters.texi | 54 ++
> libavfilter/Makefile | 1 +
> libavfilter/allfilters.c | 1 +
> libavfilter/version.h | 2 +-
> libavfilter/vf_dynoverlay.c | 439
> 

dynoverlay sounds very powerful, but this filter funcionality is very limited.

> tests/fate/filter-video.mak | 3 +
> 7 files changed, 500 insertions(+), 1 deletion(-)
> create mode 100644 libavfilter/vf_dynoverlay.c
>

What's up with code identation?

> diff --git a/Changelog b/Changelog
> index 2d0a449..5b620b4 100644
> --- a/Changelog
> +++ b/Changelog
> @@ -31,6 +31,7 @@ version :
> - MediaCodec HEVC decoding
> - TrueHD encoder
> - Meridian Lossless Packing (MLP) encoder
> +- dynoverlay filter
> version 3.1:
> diff --git a/doc/filters.texi b/doc/filters.texi
> index 070e57d..e67e29a 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -7080,6 +7080,60 @@ For more information about fontconfig, check:
> For more information about libfribidi, check:
> @url{http://fribidi.org/}.
> +@section dynoverlay
> +
> +Uses a PNG with alpha to dynamically add, update, and remove overlays
> +during live streams.
> +
> +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.


> +
> +When the filter first detects the presence of the PNG, it will load it
> +into a memory and overlay it on all frames until the PNG is either
> +updated or removed. If the PNG is updated, the filter will read it into
> +memory again. If the PNG is removed, the filter will clear the memory
> +and stop overlaying the image.
> +
> +Note that this filter only works with YUV420P video.
> +
> +The filter accepts the following options:
> +
> +@table @option
> +@item overlayfile
> +(required) The name of the PNG that will contain the overlays. Note that
> the file
> +may or may not exist when ffmpeg is launched. It can be created, updated,
> +and removed from the filesystem, and the filter will respond accordingly.
> +
> +@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}.
> +@end table

This approach is bad to provide such functionality.

> +
> +@subsection Examples
> +
> +@itemize
> +@item
> +Add an overlay to video captured from a DeckLink Mini card; check for
> updates to
> +the overlay PNG every 100ms
> +@example
> +ffmpeg -probesize 1k -r 3/1001 \
> + -f decklink -i 'DeckLink Mini Recorder (1)@@11' -y \
> + -vf format=pix_fmts=yuv420p \
> + -vf dynoverlay=overlayfile=/var/tmp/overlay.png:check_interval=100 \

You can not give more than one