[FFmpeg-cvslog] r19773 - in trunk/libavformat: seek.c seek.h
Ivan Schreter
schreter
Sun Sep 6 16:49:51 CEST 2009
Michael Niedermayer wrote:
> On Sat, Sep 05, 2009 at 09:31:01PM +0200, schreter wrote:
>
>> Author: schreter
>> Date: Sat Sep 5 21:31:01 2009
>> New Revision: 19773
>>
>> Log:
>> cosmetic changes (indentation, doxygen comments, braces, put structures for API to header, ...)
>>
> [...]> /**
>
>> - * Compare two timestamps exactly, taking into account their respective time bases.
>> + * Compare two timestamps exactly, taking their respective time bases into account.
>> *
>> - * @param ts_a timestamp A.
>> - * @param tb_a time base for timestamp A.
>> - * @param ts_b timestamp B.
>> - * @param tb_b time base for timestamp A.
>> - * @return -1. 0 or 1 if timestamp A is less than, equal or greater than timestamp B.
>> + * @param ts_a timestamp A
>> + * @param tb_a time base for timestamp A
>> + * @param ts_b timestamp B
>> + * @param tb_b time base for timestamp A
>> + * @return -1, 0 or 1 if timestamp A is less than, equal or greater than timestamp B
>> */
>> static int compare_ts(int64_t ts_a, AVRational tb_a, int64_t ts_b, AVRational tb_b)
>> {
>> @@ -95,9 +63,9 @@ static int compare_ts(int64_t ts_a, AVRa
>> if (ts_a == INT64_MIN)
>> return ts_a < ts_b ? -1 : 0;
>> if (ts_a == INT64_MAX)
>> - return ts_a > ts_b ? 1 : 0;
>> + return ts_a > ts_b ? 1 : 0;
>> if (ts_b == INT64_MIN)
>> - return ts_a > ts_b ? 1 : 0;
>> + return ts_a > ts_b ? 1 : 0;
>> if (ts_b == INT64_MAX)
>> return ts_a < ts_b ? -1 : 0;
>>
>> @@ -105,7 +73,7 @@ static int compare_ts(int64_t ts_a, AVRa
>> b = ts_b * tb_b.num * tb_a.den;
>>
>> res = a - b;
>> - if (res == 0)
>> + if (!res)
>> return 0;
>> else
>> return (res >> 63) | 1;
>>
>
> not related to your cosmetic change but this code is complete nonsense
>
>
Since you quoted timestamp comparison routine, I suppose you are talking
about it. Can you please explain why it is complete nonsense? It's
actually exactly what you wanted - exact comparison of timestamps.
I can argument why it works. Consider ts_a and ts_b we want to compare
with respective timebases tb_a and tb_b. Thus, we compare:
ts_a * N(tb_a) / D(tb_a) =?= ts_b * N(tb_b) / D(tb_b)
Multiply both sides with D(tb_a) * D(tb_b), which is positive, so
comparison result is the same. Thus:
ts_a * N(tb_a) * D(tb_b) =?= ts_b * N(tb_b) * D(tb_a)
Comparing these two values _is_ exact and correct. The only problem is
handling special cases with INT64_MIN/INT64_MAX, since those would
produce undefined result on multiplication, so it has to be done first.
If you are referring to the result computation, we are comparing a and
b. Thus, if a is less than b, obviously their difference is less than 0.
If a is greater than b, the difference is greater than 0. If they are
equal, difference is zero. One could do two ifs, but it works with one.
If difference is zero, then timestamps are equal. Otherwise, we use a
trick. Bit 64 is set to 1 for negative numbers, 0 for positive.
Arithmetic shift right copies high-order bit to other bits. I.e., the
result is -1 or 0. OR-ing with 1 makes it -1 or 1, thus the result is
well-defined.
End of the proof.
But you brought me to another point: should the two timestamps be
unsynchronized (begin at different absolute time), respective start
times have to be subtracted before timestamp comparison. This is
currently missing.
> please disable or revert all your recent seeking changes, this code needs
> quite a bit of bugfixing and i dont see it happening.
>
I only have 1-2 hours per week to work on FFmpeg. I do what I can, but
first I need to understand which bugs are there in order to fix them. My
samples do work. As you wish, I will disable the new seeking call in
mpegts.c by putting in the original routine and selecting old/new
version by an #ifdef (and put back the old seek regression reference),
so I can continue improving seeking code and people who wish to use it
can compile it in by specifying appropriate #ifdef.
> Speaking of that your h264 timestamping code also needs work that does not
> seem to be done ...
>
Yes. I implemented handling timestamps if the stream specifies
appropriate SEI messages. To say it bluntly, anyone is welcome to
continue the work to handle streams which don't specify them. I don't
have time for that and also no need for that.
I want to invest my very limited time to make AVCHD work correctly with
FFmpeg, so I can edit it directly. For that, I need seeking to work
reliably, that's why I work on it in the first place. Current MPEG-TS
implementation without my patches simply doesn't support seeking
correctly (nor does MPEG-PS, BTW).
Regards,
Ivan
More information about the ffmpeg-cvslog
mailing list