[FFmpeg-devel] [PATCH] rmdec.c: support for multirate files

Ronald S. Bultje rsbultje
Tue Dec 30 15:26:57 CET 2008


Hi,

On Tue, Dec 30, 2008 at 9:06 AM, Reimar D?ffinger
<Reimar.Doeffinger at stud.uni-karlsruhe.de> wrote:
> On Tue, Dec 30, 2008 at 08:42:58AM -0500, Ronald S. Bultje wrote:
>> -
>> -            len -= 12;
>> +            if (state >> 16 == 1)
>> +                url_fskip(pb, 1);
>> +            len -= 12 + (state >> 16 & 0x1);
>
> len -= 12;
> if (state >> 16 == 1) {
>    url_fskip(pb, 1);
>    len--;
> }
>
> Is IMHO simpler and more correct (what if some future change would allow
> (state >> 16) == 3 ? Your code would certainly break, my suggestion is at
> least somewhat correct.)

Changed, see $attached.

> Though "if (state > 0xFFFF)" might be even better...

I'm not sure what the field means... I tried to log on to the helix
site to look it up but I'm not authorized to view the .rm file format
documentation [1]. As long as we don't know what it means, either
could be correct. It could be a flags field, in which case state&1 is
correct, it could be an ID field, in which case state>0xFFFF is
correct, or it could be a skip-len field, in which case we shouldn't
skip 1 byte, but state>>16 bytes. Or it could just be a
synchronization field, in which case ==1 is the best way to read it.

Regardless, we currently only allow 0x0000<size:16> and
0x0001<size:16>, so the field (state>>16) can only be 0 or 1 at this
point in the code, so we're really talking theory here. :-).

Ronald

[1] https://rarvcode-formprot.helixcommunity.org/
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: x
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20081230/77939991/attachment.txt>



More information about the ffmpeg-devel mailing list