[Ffmpeg-devel] Matroska Patch

[Steve Lhomme]

Michael Niedermayer wrote:
>> +            if (track->default_duration)
>> +                av_reduce(&st->codec->time_base.num, &st->codec->time_base.den, track->default_duration, 1000000000, 30000);
> 
> not acceptable

Because ? The idea here is to be able to get 30000/1001 for matching 
default_duration. The rest of the time the more basic time_base (25/1) 
is found. That's at least the theory. In practice it doesn't work well...

>> -                uint64_t time;
>> +                int64_t block_time;
>>                  uint32_t *lace_size = NULL;
>>                  int n, track, flags, laces = 0;
>>                  uint64_t num;
>> @@ -2372,8 +2434,8 @@
>>                      break;
>>                  }
>>  
>> -                /* time (relative to cluster time) */
>> -                time = ((data[0] << 8) | data[1]) * matroska->time_scale;
>> +                /* block_time (relative to cluster time) */
>> +                block_time = ((data[0] << 8) | data[1]) * matroska->time_scale;
>>                  data += 2;
>>                  size -= 2;
>>                  flags = *data;
>> @@ -2470,10 +2532,10 @@
>>                              break;
>>                          }
>>                          if (cluster_time != (uint64_t)-1) {
>> -                            if (time < 0 && (-time) > cluster_time)
>> +                            if (block_time < 0 && (-block_time) > cluster_time)
>>                                  timecode = cluster_time;
>>                              else
>> -                                timecode = cluster_time + time;
>> +                                timecode = cluster_time + block_time;
> 
> cosmetical changes are not accpetable in functional patches


Are you seriously suggesting to send 2 patches (in what order) with the 
cosmetic and the code change ? For some reasons MSVC doesn't like a 
variable called time. It must be defined as a macro somewhere. And 
changing a name can't be such a bad change.

Steve