[FFmpeg-devel] [PATCH][RFC] QCP demuxer

[Kenan Gillet]

On May 15, 2009, at 4:56 PM, Michael Niedermayer wrote:

> On Fri, May 15, 2009 at 03:36:36PM -0700, Kenan Gillet wrote:
>> On Fri, May 15, 2009 at 2:16 PM, Michael Niedermayer <michaelni at gmx.at 
>> > wrote:
>>> On Fri, May 15, 2009 at 10:46:12PM +0200, Reimar D?ffinger wrote:
>>>> On Fri, May 15, 2009 at 11:21:21AM -0700, Kenan Gillet wrote:
>>>>> Any other solution i missed?
>>>>
>>>> e.g. 8-entry LUT and & 7
>>>
>>> this does not appear particularly robust in light of a damaged file
>>>
>>> [...]
>>
>> A file with its rate-map-table corrupted but using a fixed rate
>> would still be  playable, as one with a corrupted but unused entry.
>> So being permissive would be nice, plus, some invalid data would
>> be detected later if a corrupted entry is used.
>>
>
>> FIY, I have a sample, a trashed file, with a corrupted rate-map- 
>> table,
>> which still played correctly with the solution attached which make it
>> round 3.
>
> the probability of a 8 entry table to be corrupted while the rest of  
> the
> file is not is rather small, that is compared to "implementation  
> error"
> in that sense, are you sure your implementation is correct?

I check the code against the RFC, and i believe it is correct.
The rate-map-table is only used if the  var-rate-flag is non zero.
If zero, the file then contains packets of fixed length and the table
is not used.

After further investigation, the trashed file was generating an empty
wav file, so the changes were not making any difference in the output.


> and what makes you think it is corrupted?

I used the trasher tools.
BTW, is there any rule of thumb for a good count/maxburst/filesize ratio
when using the trasher tool?

>
> [...]
>
>> +            int ret, mode = get_byte(pb);
>> +            int pkt_size  = c->fixed_packet_size ? c- 
>> >fixed_packet_size - 1
>> +                                                 : c- 
>> >rates_per_mode[mode];
>
> this still has the same problem of crashing almost instantly
> and being very fragile even if not crashing

what about:

int pkt_size, ret, mode = get_byte(pb);

if (c->fixed_packet_size) {
     pkt_size = c->fixed_packet_size - 1;
} else if (mode <= QCP_MAX_MODE) {
     pkt_size = c->rates_per_mode[mode];
} else
     pkt_size = -1


note that the byte_mode is present either if the packet size is fixed  
or variable.


Kenan