[FFmpeg-devel] [PATCH] Store Major brand, Minor version and compatible brands of a mov file using the metadata API

[haim alon]

Hi,

On Tue, Sep 8, 2009 at 8:34 PM, Baptiste Coudurier <
baptiste.coudurier at gmail.com> wrote:

> On 09/08/2009 04:55 AM, haim alon wrote:
>
>> Hi,
>>
>> On Tue, Sep 8, 2009 at 11:54 AM, Diego Biurrun<diego at biurrun.de>  wrote:
>>
>>  On Mon, Sep 07, 2009 at 11:55:25AM +0300, haim alon wrote:
>>>
>>>> --- libavformat/mov.c (revision 19787)
>>>> +++ libavformat/mov.c (working copy)
>>>> @@ -485,15 +485,41 @@
>>>>
>>>> +    for (i=0; i<  numCompBrand; i++) { /*compatible brands*/
>>>>
>>> nit: i = 0
>>>
>>>
>>>  Done.
>>
>> Following Reimar's remark,  I also added a special check for the
>> compatible
>> name string - if there is a NULL character, this brand is skipped and a
>> warning message is logged.
>>
>> Attached an updated patch.
>>
>
> Please make the patch coding style and variable naming scheme matching the
> one used in the same file.
>
> compat_brand, minor_ver, etc...
>
>
Done


>
>  Thanks,
>> Haim.
>>
>>
>> ------------------------------------------------------------------------
>>
>> Index: libavformat/mov.c
>> ===================================================================
>> --- libavformat/mov.c   (revision 19787)
>> +++ libavformat/mov.c   (working copy)
>> @@ -485,15 +485,51 @@
>>      return 0; /* now go for moov */
>>  }
>>
>> +/* read major brand, minor version and compatible brands and store them
>> as metadata */
>>  static int mov_read_ftyp(MOVContext *c, ByteIOContext *pb, MOVAtom atom)
>>  {
>> +    uint32_t minorVer, nextCompBrand;
>> +    char* nextCompBrandPtr;
>> +    int numCompBrand, origNumCompBrand, i;
>> +    char majorBrandStr[5]; /* 4 characters + null */
>> +    char minorVerStr[11]; /* 32 bit integer ->  10 digits + null */
>> +    char numCompBrandStr[8]; /* 7 digits + null (Max 10 milion brands) */
>> +    char* compBrandsStr;
>> +    char* currCompBrandPtr;
>>      uint32_t type = get_le32(pb);
>>
>>      if (type != MKTAG('q','t',' ',' '))
>>          c->isom = 1;
>> -    av_log(c->fc, AV_LOG_DEBUG, "ISO: File Type Major Brand:
>> %.4s\n",(char *)&type);
>> -    get_be32(pb); /* minor version */
>> -    url_fskip(pb, atom.size - 8);
>> +    av_log(c->fc, AV_LOG_DEBUG, "ISO: File Type Major Brand: %.4s\n",
>> (char *)&type);
>> +    av_strlcpy(majorBrandStr, (char*)&type, 5); /*set major version to
>> majorBrandStr - add NULL terminator */
>>
>
> AV_WB32
>
>
Since we are dealing with string (4 characters) I think using the strlcpy is
more suitable.
This also inserts the NULL terminator at the end of the destination string.


>  +    av_metadata_set(&c->fc->metadata, "MajorBrand", majorBrandStr);
>>
>
> "major_brand"
>
>
Done


>  +    minorVer = get_be32(pb); /*minor version*/
>> +    snprintf(minorVerStr, sizeof(minorVerStr), "%d", minorVer);
>> +    av_metadata_set(&c->fc->metadata, "MinorVersion", minorVerStr);
>>
>
> "minor_version"
>
>
Done


> > [...]
> >
>
>> +    numCompBrand = (atom.size - 8) / 4;
>> +    compBrandsStr = av_malloc(numCompBrand*4 + 1); /* 4 characters for
>> each brand + null terminator */
>> +    currCompBrandPtr = compBrandsStr;
>> +    origNumCompBrand = numCompBrand;
>> +    for (i = 0; i<  origNumCompBrand; i++) { /*compatible brands*/
>> +        nextCompBrand = get_le32(pb);
>> +        nextCompBrandPtr = (char*)&nextCompBrand;
>> +        if (nextCompBrandPtr[0]&&  nextCompBrandPtr[1]&&
>>  nextCompBrandPtr[2]&&  nextCompBrandPtr[3]) // check that char is legal -
>> not NULL
>> +        {
>> +            memcpy(currCompBrandPtr,&nextCompBrand, 4);
>> +            currCompBrandPtr += 4;
>> +        }
>> +        else
>> +        {
>> +            av_log(c->fc, AV_LOG_WARNING, "compatible brand name contains
>> illegal character - skipped.\n");
>> +            numCompBrand -= 1; /* reduce counter since brand is skipped
>> */
>> +        }
>>
>
> Brace placements.
>
>
Done


>  +    }
>> +    *currCompBrandPtr = '\0';
>> +    av_metadata_set(&c->fc->metadata, "CompatibleBrands", compBrandsStr);
>>
>
> compatible_brands
>
>
Done

Attached an updated patch.

Thanks,
Haim.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ffmpeg.compatible.patch
Type: text/x-patch
Size: 2648 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090909/a25bee71/attachment.bin>