[FFmpeg-devel] [PATCH] avfilter/image2: Add source file path and basename to each packet side data.

[Marton Balint]

On Mon, 16 Dec 2019, Alexandre Heitor Schmidt wrote:

> This is the second time I'm submiting this patch, now modified to be
> less intrusive, as sugested by Marton Balint, and hopefully without git
> send-email messing up with the diff by replacing '@' within
> documentation entries.
>
> The patch modifies image2 filter to make available two special metadata
> entries called source_path and source_file, which represents,
> respectively, the complete path to the source image for the current
> frame and the basename i.e. the file name related to the current frame.
> These can then be used by filters like drawtext and others.
>
> doc/filters: The documentation for drawtext was also updated and an usage
> example was added.
>
> Fixes #2874.
>
> Signed-off-by: Alexandre Heitor Schmidt <alexandre.schmidt at gmail.com>
> ---
> doc/filters.texi      | 15 +++++++++++++--
> libavformat/img2.h    |  2 ++
> libavformat/img2dec.c | 28 ++++++++++++++++++++++++++++
> 3 files changed, 43 insertions(+), 2 deletions(-)
>
> diff --git a/doc/filters.texi b/doc/filters.texi
> index c543203ce3..15498aee0d 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -9723,8 +9723,12 @@ Available metadata can be identified by inspecting entries
> starting with TAG included within each frame section
> printed by running @code{ffprobe -show_frames}.
> 
> -String metadata generated in filters leading to
> -the drawtext filter are also available.
> +String metadata generated in filters leading to the drawtext filter are also
> +available. For example, image2 filter generates two special metadata fields
> +called @var{source_path} and @var{source_basename} for each input frame
> +generated from a single image. @var{source_path} contains the entire path
> +to input filename, while @var{source_basename} contains the equivalent
> +to @code{basename(source_path)}.

This does not really belong here, you are documenting the image2 
*demuxer*'s behaviour of setting source_basename and source_path metadata 
fields.

The only thing that might be relevant here is that the metadata can not
only come from filters but from demuxers as well.

Also since the metadata names are demuxer specific probably it is 
better to use hierarchical names, like "lavf.image2dec.source_path" and 
"lavf.image2dec.source_basename", similar to how the filters set their 
custom metadata fields.

> 
> @item n, frame_num
> The frame number, starting from 0.
> @@ -9872,6 +9876,13 @@ drawtext=fontfile=FreeSans.ttf:text=DOG:fontsize=24:x=10:y=20+24-max_glyph_a,
> drawtext=fontfile=FreeSans.ttf:text=cow:fontsize=24:x=80:y=20+24-max_glyph_a
> @end example
> 
> + at item
> +Plot special @var{source_basename} metadata, extracted from each input frame, or
> +the string "NA" if the metadata is not defined.
> + at example
> +drawtext="fontsize=20:fontcolor=white:fontfile=FreeSans.ttf:text='%{metadata\:source_basename\:NA}':x=10:y=10"
> + at end example
> +

The example can stay as is, that is fine.

> @end itemize
> 
> For more information about libfreetype, check:
> diff --git a/libavformat/img2.h b/libavformat/img2.h
> index 0e5b374a6b..8208c1f58b 100644
> --- a/libavformat/img2.h
> +++ b/libavformat/img2.h
> @@ -74,5 +74,7 @@ extern const AVOption ff_img_options[];
> 
> int ff_img_read_header(AVFormatContext *s1);
> 
> +int add_filename_as_pkt_side_data(char *filename, AVPacket *pkt);
> +

No need to make it part of the header.

> int ff_img_read_packet(AVFormatContext *s1, AVPacket *pkt);
> #endif
> diff --git a/libavformat/img2dec.c b/libavformat/img2dec.c
> index f8b4a655a5..3fb7aa7967 100644
> --- a/libavformat/img2dec.c
> +++ b/libavformat/img2dec.c
> @@ -374,6 +374,32 @@ int ff_img_read_header(AVFormatContext *s1)
>     return 0;
> }
> 
> +/**
> + * Add this frame's source path and basename to packet's sidedata
> + * as a dictionary, so it can be used by filters like 'drawtext'.
> + */
> +int add_filename_as_pkt_side_data(char *filename, AVPacket *pkt) {

This can be made static.

> +    uint8_t* metadata;
> +    int metadata_len;
> +    AVDictionary *d = NULL;
> +    char *packed_metadata = NULL;
> +
> +    av_dict_set(&d, "source_path", filename, 0);
> +    av_dict_set(&d, "source_basename", av_basename(filename), 0);
> +
> +    packed_metadata = av_packet_pack_dictionary(d, &metadata_len);
> +    if (!packed_metadata)
> +        return AVERROR(ENOMEM);
> +    if (!(metadata = av_packet_new_side_data(pkt, AV_PKT_DATA_STRINGS_METADATA, metadata_len))) {
> +        av_freep(&packed_metadata);
> +        return AVERROR(ENOMEM);
> +    }
> +    memcpy(metadata, packed_metadata, metadata_len);
> +    av_freep(&packed_metadata);
> +
> +    return 0;
> +}
> +
> int ff_img_read_packet(AVFormatContext *s1, AVPacket *pkt)
> {
>     VideoDemuxData *s = s1->priv_data;
> @@ -485,6 +511,8 @@ int ff_img_read_packet(AVFormatContext *s1, AVPacket *pkt)
>
>     if (s->is_pipe)
>         pkt->pos = avio_tell(f[0]);
> +    else if ( (res = add_filename_as_pkt_side_data(filename, pkt)) != 0 )
> +        goto fail;

Cosmetics/nitpicking, a separate if and the usual way of checking for 
errors is more readable IMHO:

if (!s->is_pipe) {
     res = add_filename_as_pkt_side_data(filename, pkt);
     if (res < 0)
         goto fail;
}

Thanks,
Marton