[FFmpeg-devel] [PATCH] RTMP seek support

[Howard Chu]

Michael Niedermayer wrote:
> On Sat, Apr 10, 2010 at 04:27:00PM -0700, Howard Chu wrote:
>> Michael Niedermayer wrote:
>>> On Sat, Apr 10, 2010 at 09:26:31AM -0700, Howard Chu wrote:
>>>> Michael Niedermayer wrote:
>>>>> On Wed, Apr 07, 2010 at 08:48:44AM -0700, Howard Chu wrote:
>>>>>> Michael Niedermayer wrote:
>>>>>>> so will you fix it? or will you knowingly round the wrong direction
>>>>>>> because there are other unrelated problems?
>>>>>>
>>>>>> I've been trying to fix it. But while you're telling me that it's
>>>>>> wrong,
>>>>>> no
>>>>>> one is telling me how to do it right.
>>>>>
>>>>> av_rescale_rnd() in this case
>>>>
>>>> One more go-round.
>>> [...]
>>>
>>>> Index: libavformat/librtmp.c
>>>> ===================================================================
>>>> --- libavformat/librtmp.c	(revision 22813)
>>>> +++ libavformat/librtmp.c	(working copy)
>>>> @@ -124,10 +141,12 @@
>>>>        RTMP *r = s->priv_data;
>>>>
>>>>        if (flags&   AVSEEK_FLAG_BYTE)
>>>> -        return AVERROR_NOTSUPP;
>>>> +        return AVERROR(ENOSYS);
>>>>
>>>>        /* seeks are in milliseconds */
>>>> -    timestamp = av_rescale(timestamp, AV_TIME_BASE, 1000);
>>>> +    if (stream_index<   0)
>>>> +        timestamp = av_rescale_rnd(timestamp, 1000, AV_TIME_BASE,
>>>> AV_ROUND_DOWN);
>>>
>>> the rounding direction depends on the the flags for the old seeking
>>> API
>>
>> I thought about that, but I disagree. The stream is playing forward
>> regardless, so any difference between rounding up or rounding down will
>> disappear by the time the next frame plays.
>
> The old as well as new APIs clearly specify what a demuxer should do.
> unless you suggest to change a deprecated API implementing it to the
> best that is possible is what should be done

Ok.

Should certainly think about it again in the new API. There's no information 
available for a caller to decide what min_ts/max_ts intervals to use. E.g., 
typically the minimum seek quantum will be the distance between two keyframes, 
and that won't necessarily be a constant in any given stream. Looks like the 
caller would have to go to a lot of trouble to get this info.

>> If you round up while seeking forward, it's possible to overshoot the
>> target and completely miss it,
>
> no, i dont think its possible in this case.
> If it where then no rounding could be done because rounding down is wrong
> that would mean the code would need to work with the exact timestamp

Even if you have the exact timestamp it may be useless, if you're trying to 
seek to a non-keyframe. I don't know what apps you're thinking of that depend 
on this accuracy, certainly most users watching their media players don't 
care. But there will be plenty of cases where rounding up can overshoot, if 
the underlying stream is forced to seek to the next keyframe.

>> thus necessitating another seek attempt. If
>> you round down, there's no chance to overshoot, and you definitely will not
>> miss the target.

>>> this should be a seperate patch
>>
>> I specifically asked about this 9 days ago and nobody addressed it. This
>> review process would be a lot less unpleasant and wasteful if you folks
>> didn't wait so long to address these issues.
>
> i can refrain from commenting issues i see, i cant comment issues/questions
> i miss, lack the motivation to deal with or lack the time to deal with.
> Suggestion to improve this are welcome, unconstructive complaints that our
> review process isnt ideal are less welcome

I have no suggestion for how to fix lack of motivation or lack of time. For 
the others, obviously you have to read all of the comments in each email. 
Otherwise there's no point in writing the emails in the first place, might as 
well just ship diffs around with no accompanying text at all.

-- 
   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: dif2.txt
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100410/d9d6f45b/attachment.txt>