[FFmpeg-devel] Suggestion for a centralized language-tag facility in libavformat

[Michael Niedermayer]

On Mon, Apr 20, 2009 at 05:20:49PM +0200, cyril comparon wrote:
> I addressed all the issues you raised, expect this one:
> 
> >> +
> >> + ? ?av_strlcpy(searchedEntry.str, lang, 4);
> >
> > this and searchedEntry are redundant
> 
> Since I bsearch over a list of LangEntries, I need to define a
> LangEntry key and thus copy the searched string into its embedded
> char[4]. 

Why do you think you need that?

also if you have some kind of script that created that table, this may
be usefull to us too in the future


[...]
> +static int langTable_compare(const void *lhs, const void *rhs)
> +{
> +    return strcmp(((const LangEntry *)lhs)->str, ((const LangEntry *)rhs)->str);
> +}
> +
> +const char *av_convertLangTo(const char *lang, enum AV_LangCodespace targetCodespace)
> +{
> +    int i;
> +    LangEntry searchedEntry;
> +    const LangEntry *entry = NULL;
> +    const int NB_CODESPACES = sizeof(langTableCounts)/sizeof(*langTableCounts);
> +
> +    if (targetCodespace < 0 || targetCodespace >= NB_CODESPACES)
> +        return NULL;
> +
> +    av_strlcpy(searchedEntry.str, lang, 4);
> +    for (i=0; !entry && i<NB_CODESPACES; i++)

> +        entry = (const LangEntry *)bsearch(&searchedEntry,

the cast looks useless


[...]
> @@ -403,6 +405,19 @@
>  //                av_log(s, AV_LOG_ERROR, "flags: 0x%x stream id %d, bitrate %d\n", flags, stream_id, bitrate);
>                  asf->stream_bitrates[stream_id]= bitrate;
>              }
> +        } else if (!guidcmp(&g, &ff_asf_language_guid)) {
> +            int j;
> +            int stream_count = get_le16(pb);

> +            if (stream_count < 0)
> +                stream_count = 0;

unneeded


> +            for(j = 0; j < stream_count; j++) {
> +                char lang[1024];
> +                uint8_t lang_len;
> +                lang_len = get_byte(pb);
> +                get_str16_nolen(pb, lang_len, lang, sizeof(lang));
> +                if (j < 128)
> +                    asf->stream_languages[j] = av_strdup(lang);
> +            }
>          } else if (!guidcmp(&g, &ff_asf_extended_content_header)) {
>              int desc_count, i;
>  

> @@ -455,7 +470,7 @@
>              get_le32(pb); // max-object-size
>              get_le32(pb); // flags (reliable,seekable,no_cleanpoints?,resend-live-cleanpoints, rest of bits reserved)
>              stream_num = get_le16(pb); // stream-num
> -            get_le16(pb); // stream-language-id-index
> +            asf->streams[stream_num].stream_language_index = get_le16(pb); // stream-language-id-index
>              get_le64(pb); // avg frametime in 100ns units
>              stream_ct = get_le16(pb); //stream-name-count
>              payload_ext_ct = get_le16(pb); //payload-extension-system-count
> @@ -535,6 +550,19 @@
>                            &st->sample_aspect_ratio.den,
>                            dar[i].num, dar[i].den, INT_MAX);
>  //av_log(s, AV_LOG_ERROR, "dar %d:%d sar=%d:%d\n", dar[i].num, dar[i].den, st->sample_aspect_ratio.num, st->sample_aspect_ratio.den);
> +
> +            // copy and convert language codes to the frontend
> +            uint16_t stream_language_index;
> +            const char *rfc1766;
> +            stream_language_index = asf->streams[i].stream_language_index;
> +            rfc1766 = asf->stream_languages[stream_language_index];
> +            if (rfc1766 && strlen(rfc1766) > 1) {

this is still reading from outside the array


[...]
> @@ -442,6 +443,54 @@
>          end_header(pb, hpos);
>      }
>  
> +    /* stream extension headers */
> +    for(n=0;n<s->nb_streams;n++) {
> +        hpos = put_header(pb, &ff_asf_ext_stream_header);
> +        put_le64(pb, 0); // starttime
> +        put_le64(pb, 0); // endtime
> +        AVStream *st = s->streams[n];
> +        put_le32(pb, st->codec->bit_rate); // leak-datarate
> +        put_le32(pb, 0); // bucket-datasize
> +        put_le32(pb, 0); // init-bucket-fullness
> +        put_le32(pb, 0); // alt-leak-datarate
> +        put_le32(pb, 0); // alt-bucket-datasize
> +        put_le32(pb, 0); // alt-init-bucket-fullness
> +        put_le32(pb, 0); // max-object-size
> +        put_le32(pb, 0); // flags (reliable,seekable,no_cleanpoints?,resend-live-cleanpoints, rest of bits reserved)
> +        put_le16(pb, asf->streams[n].num); // stream-num
> +        put_le16(pb, n); // stream-language-id-index
> +        put_le64(pb, 0); // avg frametime in 100ns units
> +        put_le16(pb, 0); //stream-name-count
> +        put_le16(pb, 0); //payload-extension-system-count

is it allowed to set all these fields to 0 ?


> +        end_header(pb, hpos);
> +    }
> +
> +    /* language list */

> +    hpos = put_header(pb, &ff_asf_language_guid);
> +    {
> +        int16_t nbStreamsWithSupportedLang = 0;

please put the variable at the top of the funtion


> +        for(n=0;n<s->nb_streams;n++) {

> +            AVMetadataTag *lang_tag;
> +            lang_tag = av_metadata_get(s->streams[n]->metadata, "language", NULL, 0);

declaration and init can be merged


> +            if (lang_tag && av_convertLangTo(lang_tag->value, AV_LANG_ISO639_1))
> +                nbStreamsWithSupportedLang++;
> +        }
> +        put_le16(pb, nbStreamsWithSupportedLang);
> +    }
> +    for(n=0;n<s->nb_streams;n++) {
> +        AVMetadataTag *lang_tag;
> +        lang_tag = av_metadata_get(s->streams[n]->metadata, "language", NULL, 0);
> +        if (lang_tag) {
> +            const char *language;
> +            language = av_convertLangTo(lang_tag->value, AV_LANG_ISO639_1);
> +            if (language) {
> +                put_byte(pb, 2*(1+strlen(language)));
> +                put_str16_nolen(pb, language);
> +            }
> +        }
> +    }

as alraedy said this stores the langs but it does not seem to associate
it to the correct streams, am i missing something?

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If you think the mosad wants you dead since a long time then you are either
wrong or dead since a long time.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090421/52806c89/attachment.pgp>