[FFmpeg-devel] [PATCH] matroskaenc: write tags.

[David Conrad]

On Mar 25, 2010, at 1:49 PM, Anton Khirnov wrote:

> On Thu, Mar 25, 2010 at 04:25:34AM -0400, David Conrad wrote:
>> On Mar 20, 2010, at 4:51 AM, Anton Khirnov wrote:
>> 
>>> added a metadata conv table to the muxer + it now adds per-track/chapter
>>> tags to the cluster seekhead, not main.
>>> 
>>> Anton Khirnov
>> 
>> Why can't all tags be stored in one level-one element? At least, I don't see from the spec why this wouldn't work.
>> 
> right, it seems i misunderstood the specs. should be fixed.
> 
>>> +    simpletag = start_ebml_master(s->pb, MATROSKA_ID_SIMPLETAG, 0);
>>> +    put_ebml_string(s->pb, MATROSKA_ID_TAGNAME, t->key);
>>> +    put_ebml_string(s->pb, MATROSKA_ID_TAGLANG, "und");
>>> +    put_ebml_uint(  s->pb, MATROSKA_ID_TAGDEFAULT, 1);
>> 
>> No need to write mandatory elements if we never use anything but the default value.
>> 
> i though mantatory elements were mandatory ;) removed
> 
>>> +    if (av_metadata_get(s->metadata, "", NULL, AV_METADATA_IGNORE_SUFFIX)) {
>>> +        ret = mkv_add_seekhead_entry(mkv->main_seekhead, MATROSKA_ID_TAGS, url_ftell(s->pb));
>>> +        if (ret < 0) return ret;
>>> +
>>> +        tags = start_ebml_master(s->pb, MATROSKA_ID_TAGS, 0);
>>> +
>>> +        while ((t = av_metadata_get(s->metadata, "", t, AV_METADATA_IGNORE_SUFFIX)))
>> 
>> Don't need the extra () here or down further.
> gcc complains if i remove them.

-Wparanthesis I guess. Oh well...

Anyway, after reading the specs again, the key must be in all uppercase.
Also, tag languages should be handled, e.g. from http://samples.mplayerhq.hu/V-codecs/SVQ3/shrek2_tsr_qt_480.mov

[...]

> +static int mkv_write_tags(AVFormatContext *s)
> +{
> +    MatroskaMuxContext *mkv = s->priv_data;
> +    ebml_master        tags = {0};
> +    int i, ret;
> +
> +    if (av_metadata_get(s->metadata, "", NULL, AV_METADATA_IGNORE_SUFFIX)) {
> +        ret = mkv_add_seekhead_entry(mkv->main_seekhead, MATROSKA_ID_TAGS, url_ftell(s->pb));
> +        if (ret < 0) return ret;
> +
> +        tags = start_ebml_master(s->pb, MATROSKA_ID_TAGS, 0);
> +
> +        mkv_write_tag(s->pb, s->metadata, 0, 0);
> +    }
> +
> +    for (i = 0; i < s->nb_streams; i++) {
> +        AVStream *st = s->streams[i];
> +
> +        if (!av_metadata_get(st->metadata, "", 0, AV_METADATA_IGNORE_SUFFIX))
> +            continue;
> +
> +        if (!tags.pos) {
> +            ret = mkv_add_seekhead_entry(mkv->cluster_seekhead, MATROSKA_ID_TAGS, url_ftell(s->pb));
> +            if (ret < 0) return ret;
> +
> +            tags = start_ebml_master(s->pb, MATROSKA_ID_TAGS, 0);
> +        }

Conditionally starting the tags master should be moved into mkv_write_tag rather than be duplicated 3x.