[FFmpeg-devel] [PATCH] id3v2 unsynchronisation support

[Reimar Döffinger]

On Sat, Jul 24, 2010 at 09:58:10PM +1000, Alexander Kojevnikov wrote:
> On 24 July 2010 18:48, Reimar D?ffinger <Reimar.Doeffinger at gmx.de> wrote:
> >> -static void read_ttag(AVFormatContext *s, int taglen, const char *key)
> >> +static int get_byte_resync (ByteIOContext *s, int *len)
> >> +{
> >> + ? ?int val;
> >> +
> >> + ? ?if (--*len < 0)
> >> + ? ? ? ?return 0;
> >> + ? ?val = get_byte (s);
> >> + ? ?if (val == 0xff && !peek_byte (s)) {
> >> + ? ? ? ?if (--*len < 0)
> >> + ? ? ? ? ? ?return 0;
> >> + ? ? ? ?get_byte (s);
> >> + ? ?}
> >> + ? ?return val;
> >> +}
> >
> > I don't think 0 should be returned when 0xff is the last
> > in the tag.
> > Also I don't think len should be decreased when nothing is read.
> > And the spaces after the function name are inconsistent with all
> > other code in FFmpeg.
> >
> > if (*len <= 0)
> > ? ?return 0;
> > val = get_byte(s);
> > *len--;
> > if (val == 0xff && *len > 0 && !peek_byte(s)) {
> > ? ?get_byte(s);
> > ? ?*len--;
> > }
> > return val;
> 
> Done, thanks for the snippet.
> 
> On a side note, I'm seeing bogus "value computed is not used" warnings
> with this code (gcc 4.5.0), should something be done about them?

They aren't bogus, it must be (*len)--, not *len--;
Or *len -= 1;
Or whatever other method.

> +    int byte;

This can now be moved into the blocks where it is used.

> -        while (taglen-- && q - dst < dstlen - 7) {
> +        byte = 0;

Since there is no need to initialize it to 0.