[Ffmpeg-devel] Matroska Patch

[Måns Rullgård]

Steve Lhomme <steve.lhomme at free.fr> writes:

> 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...

If it doesn't work, why are you sending a patch?

>>> -                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 ?

Yes.

> 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.

Then figure out why.  We don't support msvc, and will not accept
arbitrary patches to make it work without an explanation.

-- 
M?ns Rullg?rd
mru at inprovide.com