[FFmpeg-devel] avformat/flvenc: allow to write qualifying metadata as number

The FLV format can store metadata as numbers which are used and handled
by many streaming platforms.

Now, metadata values can be sent as AMF Number type if
1) tag key starts with "num_"
2) value is scanned and can be represented as a double.

Written tag will have "num_" prefix removed if written as AMF Number.
---
 doc/muxers.texi      |  2 ++
 libavformat/flvenc.c | 33 ++++++++++++++++++++++++++++-----
 2 files changed, 30 insertions(+), 5 deletions(-)

On 2023-02-11 04:32 pm, Gyan Doshi wrote:
> The FLV format can store metadata as numbers which are used and handled
> by many streaming platforms.
>
> Now, metadata values can be sent as AMF Number type if
> 1) tag key starts with "num_"
> 2) value is scanned and can be represented as a double.
>
> Written tag will have "num_" prefix removed if written as AMF Number.

Comments?

> ---
>   doc/muxers.texi      |  2 ++
>   libavformat/flvenc.c | 33 ++++++++++++++++++++++++++++-----
>   2 files changed, 30 insertions(+), 5 deletions(-)
>
> diff --git a/doc/muxers.texi b/doc/muxers.texi
> index ed5341be39..c9bafeec19 100644
> --- a/doc/muxers.texi
> +++ b/doc/muxers.texi
> @@ -509,6 +509,8 @@ ffmpeg -re -i ... -c:v libx264 -c:a aac -f fifo -fifo_format flv -map 0:v -map 0
>   @section flv
>   
>   Adobe Flash Video Format muxer.
> +This muxer will store user metadata whose keys start with @code{num_} and whose value is identified as a
> +pure value of type double as an AMF Number. All other user metadata is stored as a String.
>   
>   This muxer accepts the following options:
>   
> diff --git a/libavformat/flvenc.c b/libavformat/flvenc.c
> index 81d9b6100d..25d09ef304 100644
> --- a/libavformat/flvenc.c
> +++ b/libavformat/flvenc.c
> @@ -23,6 +23,7 @@
>   #include "libavutil/dict.h"
>   #include "libavutil/intfloat.h"
>   #include "libavutil/avassert.h"
> +#include "libavutil/eval.h"
>   #include "libavutil/mathematics.h"
>   #include "libavcodec/codec_desc.h"
>   #include "libavcodec/mpeg4audio.h"
> @@ -275,8 +276,10 @@ static void write_metadata(AVFormatContext *s, unsigned int ts)
>       AVIOContext *pb = s->pb;
>       FLVContext *flv = s->priv_data;
>       int write_duration_filesize = !(flv->flags & FLV_NO_DURATION_FILESIZE);
> -    int metadata_count = 0;
> +    int metadata_count = 0, is_num;
>       int64_t metadata_count_pos;
> +    double numvalue;
> +    char *key, *tail;
>       const AVDictionaryEntry *tag = NULL;
>   
>       /* write meta_tag */
> @@ -378,10 +381,30 @@ static void write_metadata(AVFormatContext *s, unsigned int ts)
>               av_log(s, AV_LOG_DEBUG, "Ignoring metadata for %s\n", tag->key);
>               continue;
>           }
> -        put_amf_string(pb, tag->key);
> -        avio_w8(pb, AMF_DATA_TYPE_STRING);
> -        put_amf_string(pb, tag->value);
> -        metadata_count++;
> +
> +        is_num = !strncmp(tag->key, "num_", 4) && *(tag->key + 4);
> +        key = is_num ? (tag->key + 4) : tag->key;
> +
> +        if (is_num) {
> +            numvalue = av_strtod(tag->value, &tail);
> +            if (*tail || numvalue == HUGE_VAL) {
> +                av_log(s, AV_LOG_ERROR, "Value %s for key %s cannot be stored as a double-typed number. Will be written as a string\n", tag->value, key);
> +                key = tag->key;
> +                is_num = 0;
> +            }
> +        }
> +
> +        av_log(s, AV_LOG_DEBUG, "Writing tag %s with value %s count: %d\n", key, tag->value, metadata_count);
> +
> +        put_amf_string(pb, key);
> +        if (is_num) {
> +            put_amf_double(pb, numvalue);
> +        } else {
> +            avio_w8(pb, AMF_DATA_TYPE_STRING);
> +            put_amf_string(pb, tag->value);
> +        }
> +
> +            metadata_count++;
>       }
>   
>       if (write_duration_filesize) {

Quoting Gyan Doshi (2023-02-11 12:02:51)
> The FLV format can store metadata as numbers which are used and handled
> by many streaming platforms.
> 
> Now, metadata values can be sent as AMF Number type if
> 1) tag key starts with "num_"
> 2) value is scanned and can be represented as a double.
> 
> Written tag will have "num_" prefix removed if written as AMF Number.

Using the normal metadata dict for structured data seems hacky to me.
Wouldn't it be better to add a private dict-type muxer option for this?

On 2023-02-14 03:35 pm, Anton Khirnov wrote:
> Quoting Gyan Doshi (2023-02-11 12:02:51)
>> The FLV format can store metadata as numbers which are used and handled
>> by many streaming platforms.
>>
>> Now, metadata values can be sent as AMF Number type if
>> 1) tag key starts with "num_"
>> 2) value is scanned and can be represented as a double.
>>
>> Written tag will have "num_" prefix removed if written as AMF Number.
> Using the normal metadata dict for structured data seems hacky to me.
> Wouldn't it be better to add a private dict-type muxer option for this?

The numerical metadata, like other string meta in FLV can change during 
streaming, so it will be
reloaded and refreshed. Once you suggested to shift the loading metadata 
from file + reload options
to fftools, this was the way to identify such metadata, since format 
contexts and AVStream only support
one AVDictionary and entries will have to be commingled

And this isn't really structured data. We are re-encoding how some of it 
is stored. It is still inputted
by the user as strings. The prefix is a cheap hack to clearly identify 
such keys.

(The other option would be add AVDictionary value 'types' and all 
associated signalling and tooling,
which seems useful in the long run but overkill for this.)

Regards,
Gyan

Any further comments?

On 2023-02-14 06:21 pm, Gyan Doshi wrote:
>
>
> On 2023-02-14 03:35 pm, Anton Khirnov wrote:
>> Quoting Gyan Doshi (2023-02-11 12:02:51)
>>> The FLV format can store metadata as numbers which are used and handled
>>> by many streaming platforms.
>>>
>>> Now, metadata values can be sent as AMF Number type if
>>> 1) tag key starts with "num_"
>>> 2) value is scanned and can be represented as a double.
>>>
>>> Written tag will have "num_" prefix removed if written as AMF Number.
>> Using the normal metadata dict for structured data seems hacky to me.
>> Wouldn't it be better to add a private dict-type muxer option for this?
>
> The numerical metadata, like other string meta in FLV can change 
> during streaming, so it will be
> reloaded and refreshed. Once you suggested to shift the loading 
> metadata from file + reload options
> to fftools, this was the way to identify such metadata, since format 
> contexts and AVStream only support
> one AVDictionary and entries will have to be commingled
>
> And this isn't really structured data. We are re-encoding how some of 
> it is stored. It is still inputted
> by the user as strings. The prefix is a cheap hack to clearly identify 
> such keys.
>
> (The other option would be add AVDictionary value 'types' and all 
> associated signalling and tooling,
> which seems useful in the long run but overkill for this.)
>
> Regards,
> Gyan
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Quoting Gyan Doshi (2023-02-19 09:13:55)
> On 2023-02-14 06:21 pm, Gyan Doshi wrote:
> >
> >
> > On 2023-02-14 03:35 pm, Anton Khirnov wrote:
> >> Quoting Gyan Doshi (2023-02-11 12:02:51)
> >>> The FLV format can store metadata as numbers which are used and handled
> >>> by many streaming platforms.
> >>>
> >>> Now, metadata values can be sent as AMF Number type if
> >>> 1) tag key starts with "num_"
> >>> 2) value is scanned and can be represented as a double.
> >>>
> >>> Written tag will have "num_" prefix removed if written as AMF Number.
> >> Using the normal metadata dict for structured data seems hacky to me.
> >> Wouldn't it be better to add a private dict-type muxer option for this?
> >
> > The numerical metadata, like other string meta in FLV can change 
> > during streaming, so it will be
> > reloaded and refreshed. Once you suggested to shift the loading 
> > metadata from file + reload options
> > to fftools, this was the way to identify such metadata, since format 
> > contexts and AVStream only support
> > one AVDictionary and entries will have to be commingled
> >
> > And this isn't really structured data. We are re-encoding how some of 
> > it is stored. It is still inputted
> > by the user as strings. The prefix is a cheap hack to clearly identify 
> > such keys.
> >
> > (The other option would be add AVDictionary value 'types' and all 
> > associated signalling and tooling,
> > which seems useful in the long run but overkill for this.)

We have way too many hacks already, because everbody's looking for a
quick solution and ignores longterm maintainability.
I don't think this patch should go in.

On 2023-02-20 10:10 pm, Anton Khirnov wrote:
> Quoting Gyan Doshi (2023-02-19 09:13:55)
>> On 2023-02-14 06:21 pm, Gyan Doshi wrote:
>>>
>>> On 2023-02-14 03:35 pm, Anton Khirnov wrote:
>>>> Quoting Gyan Doshi (2023-02-11 12:02:51)
>>>>> The FLV format can store metadata as numbers which are used and handled
>>>>> by many streaming platforms.
>>>>>
>>>>> Now, metadata values can be sent as AMF Number type if
>>>>> 1) tag key starts with "num_"
>>>>> 2) value is scanned and can be represented as a double.
>>>>>
>>>>> Written tag will have "num_" prefix removed if written as AMF Number.
>>>> Using the normal metadata dict for structured data seems hacky to me.
>>>> Wouldn't it be better to add a private dict-type muxer option for this?
>>> The numerical metadata, like other string meta in FLV can change
>>> during streaming, so it will be
>>> reloaded and refreshed. Once you suggested to shift the loading
>>> metadata from file + reload options
>>> to fftools, this was the way to identify such metadata, since format
>>> contexts and AVStream only support
>>> one AVDictionary and entries will have to be commingled
>>>
>>> And this isn't really structured data. We are re-encoding how some of
>>> it is stored. It is still inputted
>>> by the user as strings. The prefix is a cheap hack to clearly identify
>>> such keys.
>>>
>>> (The other option would be add AVDictionary value 'types' and all
>>> associated signalling and tooling,
>>> which seems useful in the long run but overkill for this.)
> We have way too many hacks already, because everbody's looking for a
> quick solution and ignores longterm maintainability.
> I don't think this patch should go in.

1) What is the 'slow solution' for this case?

2) What long-term maintenance issues do you see with this patch?
(The FLV format is no longer actively developed. Last spec update is >10 
years.  flvenc only receives bug fixes and API/code harmonization updates
  The user has to know of and deliberately add the prefix, so intent is 
clear. There's no chance of accidentally handling other entries.)

The other narrow-scope option I see is to add a flvenc option, which 
when set, checks every value, and writes it as a AMF Number if scanned 
as a pure number.

Regards,
Gyan