[Ffmpeg-devel] Matroska Patch

[Steve Lhomme]

Michael Niedermayer wrote:
> Hi
> 
> On Sat, Mar 25, 2006 at 05:15:34PM +0100, Steve Lhomme wrote:
>> Diego Biurrun wrote:
>>> On Thu, Mar 23, 2006 at 10:55:48AM +0100, Diego Biurrun wrote:
>>>> On Thu, Mar 23, 2006 at 10:29:09AM +0100, Steve Lhomme wrote:
>>>>> Diego Biurrun wrote:
>>>>>> This case is different IMO.  The use of 'time' as variable name is
>>>>>> problematic.  You have to have a copy of the C standard lying around to
>>>>>> check which uses are allowed and which aren't to avoid shooting yourself
>>>>>> in the foot.  In this situation avoiding time as variable name outright
>>>>>> looks like the clean solution to me.
>>>>>>
>>>>>> @Steve: Variable renaming should go in a separate patch in any case.
>>>>> Here you go.
>>>> Michael, if you don't object, I'm going to apply this during the next
>>>> days.
>>> Applied.
>>
>> Thanks Diego.
>>
>> So here is the more important patch that makes the block_time signed (as 
>> in the specs).
> 
> rejected, this is wrong its int16 not 64 this change will not do anything
> excpet maybe hiding a gcc warning, it will never become <0

int64_t time_scale;
block_time = ((data[0] << 8) | data[1]) * matroska->time_scale;

Now ((data[0] << 8) | data[1]) could be cast to int16_t first. I don't 
know if it's done automatically or not, given time_scale is signed. So 
it would be better to make sure it is.

Steve