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

[haim alon]

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.

Thanks!

Attached an updated patch,

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