[FFmpeg-devel] [PATCH v3 2/2] libavformat/mxfenc: write package name metadata

[Mark Reid]

On Wed, Mar 4, 2015 at 4:07 AM, <tomas.hardin at codemill.se> wrote:

> On 2015-03-03 05:06, Mark Reid wrote:
>
>> +/*
>> + * Returns the calculated length a local tag containing an utf-8
>> string as utf-16
>> + */
>> +static uint64_t mxf_utf16_local_tag_length(const char *utf8_str)
>> +{
>> +    return utf8_str? 4 + mxf_utf16len(utf8_str) * 2: 0;
>>
>
> Should return zero if mxf_utf16len()*2 > 65535.
>
>  +}
>> +
>> +/*
>> + * Write a local tag containing an utf-8 string as utf-16
>>   */
>>  static void mxf_write_local_tag_utf16(AVIOContext *pb, int tag, const
>> char *value)
>>  {
>> -    int i, size = strlen(value);
>> +    int size = mxf_utf16len(value);
>>      mxf_write_local_tag(pb, size*2, tag);
>>
>
> Same here. Check that size*2 < 65536. I don't want us outputting illegal
> MXFs.
>
>  -    for (i = 0; i < size; i++)
>> -        avio_wb16(pb, value[i]);
>> +    avio_put_str16be(pb, value);
>>  }
>>
>
>
>
I did put a size check in mxf_write_package before writing the tags here:

 -static void mxf_write_package(AVFormatContext *s, enum MXFMetadataSetType
> type)
> +static void mxf_write_package(AVFormatContext *s, enum
> MXFMetadataSetType type, const char *package_name)
>  {
>      MXFContext *mxf = s->priv_data;
>      AVIOContext *pb = s->pb;
>      int i, track_count = s->nb_streams+1;
> +    uint64_t name_size = 0;
> +
> +    if (package_name)
> +        name_size = mxf_utf16_local_tag_length(package_name);
> +
> +    if (name_size >= INT16_MAX){
> +        av_log(s, AV_LOG_WARNING, "package name size %"PRIx64" invalid
> (too large), ignoring\n", name_size);
> +        name_size = 0;
> +    }


 (hmm but looks like it should be UINT16_MAX instead anyway). I'll move the
check to those functions instead and send a new patch.  Thanks for taking
the time to review.