[FFmpeg-devel] [PATCH] RTMP seek support

[Howard Chu]

Michael Niedermayer wrote:
> On Thu, Apr 01, 2010 at 01:11:24PM -0700, Howard Chu wrote:
>> Michael Niedermayer wrote:
>>> On Wed, Mar 31, 2010 at 05:24:49PM -0700, Howard Chu wrote:
>>>> Howard Chu wrote:
>>>>> Michael Niedermayer wrote:
>>>>>> removial is planed but the new API might be changed if we find the need
>>>>>> to change it still ...
>>>>
>>>> It seems to me that one thing that ought to be changed is that rescaling
>>>> the timestamp should still be done by the caller, not in read_seek2.
>>>> Right
>>>> now each implementation of read_seek2() has to duplicate this code.
>>>
>>> if rescaling is done outside then exact seeking becomes impossible
>>> because rescaling implicates rounding
>>
>> But that's always true, regardless. If you sepcify a seek timestamp in
>> nanoseconds and the stream only supports microseconds, it is going to have
>> to round anyway.
>
> there are normally several streams, and their timebases normally differ.

Does that mean a high level seek request is expected to result in several 
low-level seek operations, one for each individual stream?

>>>> Index: libavformat/flvdec.c
>>>> ===================================================================
>>>> --- libavformat/flvdec.c	(revision 22713)
>>>> +++ libavformat/flvdec.c	(working copy)
>>>> @@ -442,6 +442,38 @@
>>>>        return ret;
>>>>    }
>>>>
>>>> +static int flv_read_seek(AVFormatContext *s, int stream_index,
>>>> +    int64_t ts, int flags)
>>>> +{
>>>> +    return av_url_read_fseek(s->pb, stream_index, ts, flags);
>>>> +}
>>>> +
>>>> +static int flv_read_seek2(AVFormatContext *s, int stream_index,
>>>> +    int64_t min_ts, int64_t ts, int64_t max_ts, int flags)
>>>> +{
>>>> +    int ret = AVERROR_NOTSUPP;
>>>> +
>>>> +    if (url_is_streamed(s->pb)) {
>>>> +        if (stream_index<   0) {
>>>> +            AVStream *st;
>>>> +
>>>> +            stream_index = av_find_default_stream_index(s);
>>>> +            if (stream_index<   0)
>>>> +                return -1;
>>>> +
>>>> +            st = s->streams[stream_index];
>>>> +            // timestamp for default must be expressed in AV_TIME_BASE
>>>> units
>>>> +            ts = av_rescale(ts, st->time_base.den,
>>>> +                                AV_TIME_BASE *
>>>> (int64_t)st->time_base.num);
>>>> +        }
>>>> +        ret = av_url_read_fseek(s->pb, stream_index, ts, flags);
>>>> +    }
>>>> +
>>>> +    if (ret == AVERROR_NOTSUPP)
>>>> +        ret = av_seek_frame(s, stream_index, ts, flags | (ts - min_ts>
>>>> (uint64_t)(max_ts - ts) ? AVSEEK_FLAG_BACKWARD : 0));
>>>> +    return ret;
>>>> +}
>>>
>>> now i see what you meant by that this will need to be changed again
>>> either way this does not implement the new API correctly
>>
>> It's simply based on the behavior of avformat_seek_file(). I just aimed at
>> producing the same result for non-streams, while giving the protocol
>> handler read_seek a chance to run.
>
> the behavior of above in case av_url_read_fseek() is used seems to
> differ quite a bit from what the new api docs expect

What docs? find|grep in ffmpeg/doc doesn't have any hits on read_seek2.

The end result is identical to the avformat_seek_file() case where 
read_seek2() does not exist, so how can the behavior be different? If that's 
true then avformat_seek_file() is also broken.

>>> also you mix AVERROR_NOTSUPP with AVERROR(ENOSYS)
>>
>> They are identical. AVERROR_NOTSUPP is defined to AVERROR(ENOSYS) in
>> libavutil/error.h.
>
> currently yes, and maybe always so but i prefer code that does not
> depend on such assumtations

OK, I'll just use AVERROR(ENOSYS).

-- 
   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/