[FFmpeg-devel] [PATCH] avformat/mov: Store trak > udta metadata on each stream

[Martin Storsjö]

On Tue, 4 Feb 2025, Martin Storsjö wrote:

> Hi Rémi,
>
> On Wed, 15 Jan 2025, Rémi Bernon wrote:
>
>> Some files keep extra metadata such as 'name' fields within udta, and
>> it is useful for Wine to access them with the "export_all" option so
>> they can then be exposed to Windows applications.
>> 
>> Signed-off-by: Rémi Bernon <rbernon at codeweavers.com>
>> ---
>> libavformat/mov.c | 10 ++++++++--
>> 1 file changed, 8 insertions(+), 2 deletions(-)
>> 
>> diff --git a/libavformat/mov.c b/libavformat/mov.c
>> index c016ce8e41..f067ca4905 100644
>> --- a/libavformat/mov.c
>> +++ b/libavformat/mov.c
>> @@ -354,6 +354,12 @@ static int mov_read_udta_string(MOVContext *c, 
>> AVIOContext *pb, MOVAtom atom)
>>     int (*parse)(MOVContext*, AVIOContext*, unsigned, const char*) = NULL;
>>     int raw = 0;
>>     int num = 0;
>> +    AVDictionary **metadata;
>> +
>> +    if (c->trak_index >= 0 && c->trak_index < c->fc->nb_streams)
>> +        metadata = &c->fc->streams[c->trak_index]->metadata;
>> +    else
>> +        metadata = &c->fc->metadata;
>>
>>     switch (atom.type) {
>>     case MKTAG( '@','P','R','M'): key = "premiere_version"; raw = 1; break;
>> @@ -572,10 +578,10 @@ retry:
>>             str[str_size] = 0;
>>         }
>>         c->fc->event_flags |= AVFMT_EVENT_FLAG_METADATA_UPDATED;
>> -        av_dict_set(&c->fc->metadata, key, str, 0);
>> +        av_dict_set(metadata, key, str, 0);
>>         if (*language && strcmp(language, "und")) {
>>             snprintf(key2, sizeof(key2), "%s-%s", key, language);
>> -            av_dict_set(&c->fc->metadata, key2, str, 0);
>> +            av_dict_set(metadata, key2, str, 0);
>>         }
>>         if (!strcmp(key, "encoder")) {
>>             int major, minor, micro;
>> -- 
>> 2.45.2
>
> So instead of storing metadata on the demuxer level, it is stored on the 
> stream level, to avoid clobbering metadata if multiple streams provide 
> metadata with the same name.
>
> I guess that sounds reasonable. However, wouldn't this be a notable change 
> for consumers that currently expect to see such metadata on the demuxer 
> level?
>
> I guess we don't have any firm guarantees about such things, and if the 
> metadata specifically is for a track, it is the more correct thing to do 
> anyway.
>
> I don't have all the usual cases around mov/mp4 metadata fresh in mind 
> though. CCing James to hear his opinion on this.

I discussed this with James on irc, and he pointed out that udta can exist 
both on the top level and within the track boxes. And this patch does the 
right thing wrt that, as trak_index is >= 0 while parsing a trak, 
otherwise it's < 0.

So this patch seems correct here, so I'll go ahead and push it.

// Martin