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

[Baptiste Coudurier]

Hi,

On 09/09/2009 06:28 AM, haim alon wrote:
> Hi,
>
> On Wed, Sep 9, 2009 at 3:22 PM, Reimar D?ffinger
> <Reimar.Doeffinger at gmx.de>wrote:
>
>> On Wed, Sep 09, 2009 at 02:00:47PM +0300, haim alon wrote:
>>>> No, at least av_strlcpy reads at most "size-1" bytes, 4 in this case.
>>>> The problem is with the return value of it:
>>>> return len + strlen(src) - 1;
>>>> So my suggestion to replace strncpy with av_strlcpy actually completely
>>>> broke things, (av_)strlcpy can only be used for correctly 0-terminated
>>>> strings, and I fear that this is used wrongly in some places in
>>>> FFmpeg, e.g. rtmppkt.c looks suspicious.
>>>>
>>>>
>>> Why there is a problem with the return value?
>>> In this case, it copies 4 characters from the source uint32_t and then
>> add
>>> NULL terminator.
>> And then it will call strlen on the input value to calculate the return
>> value and overread until it either hits a 0 by chance or crashes.
>>
>>> By the way, I've tried using AV_WB32 instead but it swapped the
>> characters.
>>> Maybe AV_WL32 should be used.
>> Since get_le32 was used, yes it should be AV_WL32.
>> But the log message will still print it in the wrong order on big-endian
>> systems.
>> IMO the whole casting mess should be removed and type would better be
>> uint8_t type[5] ={0};
>> get_buffer(pb, type, 4);
>>
>>
> Looks to me like a good solution.
>
>
>>> +        // check that char is legal - not NULL
>>> +        if (next_comp_brand_ptr[0]&&  next_comp_brand_ptr[1]&&
>> next_comp_brand_ptr[2]&&  next_comp_brand_ptr[3]) {
>>
>> Not sure if this should be checked more tightly, e.g. for ASCII, or for
>> printable or...
>>
>
> I think isprint() is better than just compare to NULL.

IMHO get rid of the cruft and export everything even if it's not printable.

"Each brand is a printable four-character code, registered with ISO, 
that identifies a precise specification."

It should not happen, if it happens, well, file is broken and we will 
add cruft to handle this later.

[...]

-- 
Baptiste COUDURIER
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
FFmpeg maintainer                                  http://www.ffmpeg.org