[FFmpeg-devel] [PATCH v4 3/3] avformat/mxfenc: prefer to use the configured metadta

[Marton Balint]

On Fri, 15 Jan 2021, Tomas Härdin wrote:

> lör 2021-01-09 klockan 13:07 +0800 skrev lance.lmwang at gmail.com:
>> From: Limin Wang <lance.lmwang at gmail.com>
>> 
>> The metadata company_name, product_name, product_version from input
>> file will be deleted to avoid overwriting information
>> Please to test with below command:
>> ./ffmpeg -i ../fate-suite/mxf/Sony-00001.mxf -c:v copy -c:a copy out.mxf
>> and
>> ./ffmpeg -i ../fate-suite/mxf/Sony-00001.mxf -c:v copy -c:a copy \
>>         -metadata company_name="xxx" \
>>         -metadata product_name="xxx" \
>>         -metadata product_version="xxx" \
>>         out.mxf
>> 
>> Signed-off-by: Limin Wang <lance.lmwang at gmail.com>
>> ---
>>  fftools/ffmpeg_opt.c |  3 +++
>>  libavformat/mxfenc.c | 12 ++++++++----
>>  2 files changed, 11 insertions(+), 4 deletions(-)
>> 
>> diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
>> index c295514..493763b 100644
>> --- a/fftools/ffmpeg_opt.c
>> +++ b/fftools/ffmpeg_opt.c
>> @@ -2650,6 +2650,9 @@ loop_end:
>>          if(o->recording_time != INT64_MAX)
>>              av_dict_set(&oc->metadata, "duration", NULL, 0);
>>          av_dict_set(&oc->metadata, "creation_time", NULL, 0);
>> +        av_dict_set(&oc->metadata, "company_name", NULL, 0);
>> +        av_dict_set(&oc->metadata, "product_name", NULL, 0);
>> +        av_dict_set(&oc->metadata, "product_version", NULL, 0);
>>      }
>>      if (!o->metadata_streams_manual)
>>          for (i = of->ost_index; i < nb_output_streams; i++) {
>> diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c
>> index d8678c9..5244211 100644
>> --- a/libavformat/mxfenc.c
>> +++ b/libavformat/mxfenc.c
>> @@ -722,16 +722,20 @@ static void mxf_write_identification(AVFormatContext *s)
>>  {
>>      MXFContext *mxf = s->priv_data;
>>      AVIOContext *pb = s->pb;
>> -    const char *company = "FFmpeg";
>> -    const char *product = s->oformat != &ff_mxf_opatom_muxer ? "OP1a Muxer" : "OPAtom Muxer";
>> -    const char *version;
>> +    AVDictionaryEntry *com_entry =  av_dict_get(s->metadata, "company_name", NULL, 0);
>> +    AVDictionaryEntry *product_entry =  av_dict_get(s->metadata, "product_name", NULL, 0);
>> +    AVDictionaryEntry *version_entry =  av_dict_get(s->metadata, "product_version", NULL, 0);
>> +    const char *company = com_entry ? com_entry->value : "FFmpeg";
>> +    const char *product = product_entry ? product_entry->value : s->oformat != &ff_mxf_opatom_muxer ? "OP1a Muxer" : "OPAtom Muxer";
>> +    const char *version = NULL;
>> +    const char *product_version = version_entry ? version_entry->value : AV_STRINGIFY(LIBAVFORMAT_VERSION);
>>      int length;
>>
>>      mxf_write_metadata_key(pb, 0x013000);
>>      PRINT_KEY(s, "identification key", pb->buf_ptr - 16);
>>
>>      version = s->flags & AVFMT_FLAG_BITEXACT ?
>> -        "0.0.0" : AV_STRINGIFY(LIBAVFORMAT_VERSION);
>> +        "0.0.0" : product_version;
>
> Again, why? If you have a company that maintains a fork of FFmpeg then
> compile that info in here instead. Compare with FFmbc which always puts
> "FFmbc" as CompanyName.

And how can a product based on libavformat set the company name, product 
name and product version? It seems a valid use case for me that these are 
overridable. Also note that this product version is only the "user 
friendly" version string, for the numeric version still 
LIBAVFORMAT_VERSION values are used.

Regards,
Marton