On Thu, Jun 17, 2010 at 8:45 AM, Michael Niedermayer <[email protected]> wrote: > On Wed, Jun 16, 2010 at 10:50:06PM +0200, Michael Chinen wrote: >> On Wed, Jun 16, 2010 at 10:07 PM, Michael Niedermayer <[email protected]> >> wrote: >> > On Wed, Jun 09, 2010 at 03:42:09PM +0200, mchinen wrote: >> >> Author: mchinen >> >> Date: Wed Jun 9 15:42:08 2010 >> >> New Revision: 5826 >> >> >> >> Log: >> >> creating seek2010 dir for my soc proj and adding current patch >> > [...] >> > >> >> +Index: libavformat/avformat.h >> >> +=================================================================== >> >> +--- libavformat/avformat.h (revision 23548) >> >> ++++ libavformat/avformat.h (working copy) >> >> +@@ -390,6 +390,21 @@ >> >> + int min_distance; /**< Minimum distance between this and >> >> the previous keyframe, used to avoid unneeded searching. */ >> >> + } AVIndexEntry; >> >> + >> >> ++#define AV_SEEKTABLE_BUILDING 0x0001 >> >> ++#define AV_SEEKTABLE_CBR 0x0002 >> >> ++#define AV_SEEKTABLE_FINISHED 0x0004 >> >> ++#define AV_SEEKTABLE_COPIED 0x0008 >> > >> > missing documentation >> > >> > >> > [...] >> > >> >> +@@ -531,6 +546,9 @@ >> >> + * Number of frames that have been demuxed during >> >> av_find_stream_info() >> >> + */ >> >> + int codec_info_nb_frames; >> >> ++ >> >> ++ /* new av_seek_frame() support */ >> >> ++ AVSeekTable seek_table; >> >> + } AVStream; >> > >> > we alraedy have a table for seeking, that is AVStream.index_entries >> > why do you add a second table? >> This one is a complete index table that will be saved/loaded/built on >> a different thread during decoding. I made a new one mostly to make >> sure I didn't cause regression bugs. Also currently lots of demuxers >> change the state of that index table during normal read/parse. Once >> the complete table is built, the idea is to stop using the old >> index_entries one and use the complete table for seeking. Is this >> okay? > > lets assume you have a seperate demuxer for buildng the index, then this one > already has its own AVStream.index_entries which it also should already fill > during demuxing. > only thing left is moving it over to AVStream.index_entries of the main > demuxer once its done. > am i missing something? > > this does appear simpler and simpler to implement to me
I've thought more about this and I think there are tradeoffs either way: With using a seperate demuxer and moving over: -have to go through all demuxer specific parse code that modifies the index and change it to not do this after moving the table over -some demuxers (avidec.c) appear to use the last index in index_entries to find the last timestamp in read_packet. There may be a few places where demuxers expect the index_entries to be something that isn't the complete index table. -implementing the moving over in the parallel use case will require more than mutexing the entries array since we will also have invalid indexes returned by av_index_search_timestamp. With using AVSeekTable struct inside AVStream: -more memory is used -AVStream has one more thing in it (although either way we will need the flags variable in AVSeekTable to exist somewhere). -seek functions need to be modified to use this structure instead of index_entries. (this is already done in utils.c and some demuxers, although they need to be optimized) I agree that it would be nicer from an interface standpoint, not to introduce a new data structure, but it doesn't seem as simple to do it that way. You guys definitely know the code better than I do. Let me know what I'm missing. Michael _______________________________________________ FFmpeg-soc mailing list [email protected] https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-soc
