[FFmpeg-devel] [RFC] Seeking API

[Baptiste Coudurier]

Michael Niedermayer wrote:
> On Thu, Jan 22, 2009 at 04:10:53PM -0800, Baptiste Coudurier wrote:
>> Michael Niedermayer wrote:
> [...]
>>>> Also we could handle FRAMENUM | NEXT_KEYFRAME or PREV_KEYFRAME with the
>>>> help of min_distance value, which becomes complicated for the user.
>>>>
>>>> Needing a stream_index is ok if documented IMHO.
>>> I dont mind adding FRAMENUM & stream_index but this will make the API
>>> more complex. see:
>>>
>>> /**
>>>  * Seek to timestamp ts.
>>>  * Seeking will be done so that the point from which all active streams
>>>  * can be presented successfully will be closest to ts and within min/max_ts.
>>>  *
>>>  * if flags contain AVSEEK_FLAG_BYTE then all timestamps are in byte and
>>>  * are the file position (this may not be supported by all demuxers).
>>>  * if flags contain AVSEEK_FLAG_FRAME then all timestamps are in frames
>>>  * in the stream with stream_index (this may not be supported by all demuxers).
>>>  * else all timestamps are in units of the stream selected by stream_index or
>>>  * if its -1 AV_TIME_BASE units.
>>>  * if flags contain AVSEEK_FLAG_ANY then non keyframes are treated as
>>>  * keyframes (this may not be supported by all demuxers).
>>>  *
>>>  * @param stream_index index of the stream which is used as timebase reference.
>>>  * @param min_ts smallest acceptable timestamp
>>>  * @param ts target timestamp
>>>  * @param max_ts largest acceptable timestamp
>>>  * @param flags flags
>>>  * @returns >=0 on success, error code otherwise
>>>  */
>>> int av_seek_frame2(AVFormatContext *s, int stream_index, int64_t min_ts, int64_t ts, int64_t max_ts, int flags);
>> I like it, are min/max_ts allowed to be 0/-1/AV_NOPTS_VALUE ? What
>> happens in this case ? No limits ?
> 
> i see no immedeate problem with allowing the full signed 64bit range
> the no limit case would then be the INT64_MIN/MAX

Humm, according to libavformat users, treating negative timestamps as 0
really improve usability, so should we enforce uint64_t ?

I did it for mov demuxer, mxf does it already, a patch a just been
posted for avi. If seeking is based on pts, it makes sense IMHO.

Should we offer a way to indicate the user how much backward we return ?
I believe this might be useful.

>>> and this still skiped one issue, namely that AVSEEK_FLAG_FRAME will with this
>>> API not seek to the wanted frame unless the active set selected by
>>> AVStream.discard is just that single stream.
>>> We can add yet another flag to override this of course ...
>> Could we always allow to specify a discarded stream and take it into
>> account nonetheless ? What harm could it cause ?
> 
> I think this becomes too complex, noone will remember all these
> special cases ...

You might be right. First goal should be usability.

> "the stream you use as timebase reference will be considerd active even if
> it is marked as not active"
> A flag is IMHO significantly cleaner than such sideeffects, because
> writing FLAG_ONLY_SPECIFIED_STREAM_ACTIVE makes it explict what is intended

Default behaviour should be the most intuitive and used, so
AVSEEK_FLAG_IGNORE_DISCARDED_STREAMS ? or
AVSEEK_FLAG_USE/CONSIDER_DISCARDED_STREAMS ?

Also better use the term "discarded" since this is the field used in
API, I think.

>> Should we let the possibility to reeanable a stream ? In this case
>> populating index for all streams is useful.
> 
> I dont think theres anything that disallows reenabling a stream thus yes
> it is allowed

Noted.

>> Atm, it seems entries are added even if stream is discarded, at least
>> mov and mpeg ps.
> 
>> I'd say -1 with FLAG_FRAME could mean first video track for ease of use,
>> it's not really importand though.
> 
> i think we have already too many special cases

Well, ok.

>>>>>> Not sure if this is much related to seeking:
>>>>>> I'd like containers to export the AVIndex feature usage and add pts to
>>>>>> AVIndexEntry, also a way to generate the full AVIndexEntry and have the
>>>>>> possibility to export it as file to be loaded later.
>>>>>>
>>>>>> Also user needs to specify if he wants to seek by dts or pts and
>>>>>> therefore what timestamp value refers to.
>>>>> Is there any use case in which the user wants to seek by dts?
>>>> Not sure, currently, mpeg ps demuxer add dts to index entries, same for mov.
>>>> So either you add pts to index entries but with reordering, binary
>>>> search is difficult.
>>> seeking in many demuxers will need to be reworked if we want to improve
>>> things.
>> Definitely, what is the plan to keep compatibility with av_seek_frame ?
> 
> no plan ;)
> id say we just add the new functions, switch to using them and then drop the
> old ones.
> Of course we might run into some issues that have to be dealt with as they
> occur ...
> but then as said thats no real plan just stating the obvious

Ok, so we need to add read_seek2 to AVInputFormat.

>>> also dts&pts in mpeg-ps only have to be stored once every 0.5 sec or so
>>> which is a bigger issue than reordering.
>> Right, this should be handled also.
>>
> 
>>>> Or you add dts and export first_dts value, so it is possible to seek to
>>>> first dts.
>>>>
>>>> In all case "timestamp" value needs clarification.
>>> all timestamps where intended to be presentation timestamps
>>> i felt that:
>>>     "Seeking will be done so that the point from which all active streams
>>>     can be presented successfully will be closest to ts and within min/max_ts."
>>> was makeing that clear
>>> do you have some suggestion to improve this?
>>> i could rename ts to pts ...
>> Using pts would make it clear and avoid any confusion for future
>> demuxers, I believe.
>>
> 
>> So should demuxers be changed to add pts in index ?
> 
>> av_index_search_timestamps should also be changed then.
> 
> I think a safer approuch would be to add a new function and drop
> av_index_search_timestamps() in the case the dts based search becomes
> unused

Ok.

>> Should we add pts to AVIndexEntry, to have dts and pts so simplify
>> search, or is it unnecessary ?
> 
> i dont know yet ...
> Or to say it differently, we can add it when it becomes needed/usefull ...

Ok.

I suddently have an idea, should we call the new function
"avformat_seek_file", "avformat_seek" ?

So plan seems to be:

- add avformat_seek_file proto
- add read_seek2 (maybe a better name can be found) to AVInputFormat
- choose a reference implementation for a format using ideally all
features of the new API, and implement read_seek2
- implement avformat_seek_file

Comments welcome.

-- 
Baptiste COUDURIER                              GnuPG Key Id: 0x5C1ABAAA
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
checking for life_signs in -lkenny... no
FFmpeg maintainer                                  http://www.ffmpeg.org