[FFmpeg-devel] [PATCH 4/4] mov: simplify mov_read_chapters() by using avio_get_str16be

[Anton Khirnov]

On Sun, Jan 23, 2011 at 07:59:03PM +0100, Reimar D?ffinger wrote:
> On Sun, Jan 23, 2011 at 12:38:30PM +0100, Anton Khirnov wrote:
> > @@ -2303,43 +2302,33 @@ static void mov_read_chapters(AVFormatContext *s)
> >      for (i = 0; i < st->nb_index_entries; i++) {
> >          AVIndexEntry *sample = &st->index_entries[i];
> >          int64_t end = i+1 < st->nb_index_entries ? st->index_entries[i+1].timestamp : st->duration;
> > +        uint8_t title[512];
> 
> Well, it does add a limitation on the title length (and use more stack).
Fixed.
(maybe it'd be better to use two mallocs for different encodings, i'm
not sure)
> 
> > +        if ((ch = get_be16(sc->pb)) == 0xfeff)
> 
> I'd prefer to not squeeze so much in one line, particularly when it starts looking
> like lisp with all those ()
> 
split
> > +            AV_WB16(title, ch);
> > +            get_buffer(sc->pb, title + sizeof(ch), sizeof(title) - sizeof(ch));
> 
> IMO the sizeof is more obfuscating than helpful here, the sequence
> AV_WB16(ptr, val)
> ptr + 2
> is IMO more obvious than
> AV_WB16(ptr, val)
> ptr + sizeof(val)
deobfuscated
Also changed get_buffer to get_strz to ensure NULL-termination.

-- 
Anton Khirnov
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-mov-simplify-mov_read_chapters-by-using-avio_get_str.patch
Type: text/x-diff
Size: 2954 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20110123/5ea6b5c7/attachment.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20110123/5ea6b5c7/attachment.pgp>