[FFmpeg-soc] [soc]: r5826 - in seek2010: . seek2010.patch

[Michael Niedermayer]

On Fri, Jun 18, 2010 at 01:14:57PM +0200, Michael Chinen wrote:
> On Fri, Jun 18, 2010 at 4:44 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
> > On Fri, Jun 18, 2010 at 02:16:43AM +0200, Michael Chinen wrote:
> >> On Thu, Jun 17, 2010 at 8:45 AM, Michael Niedermayer <michaelni at gmx.at> 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 <michaelni at gmx.at> 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
> >
> > if a demuxer did that, that would implicate that it keeps changing the
> > index after it read through the file, this makes no sense and sounds
> > like a bug.
> I thought it can happen in some cases.  AVStream.max_index_entries
> imposes a size limit based on on the index_entries array.  The default
> value is 2^20 bytes, which may or may not hold all index entries.
> When the limit is reached ff_reduce_index throws away old ones.  This
> is done in av_read_frame_internal for formats that use a generic
> index.  But we can just check the flag to see if we've built a
> complete table and should ignore this limit.

whatever is done there is no reason to make a in background build index behave
differently from th main in relation to max_index_entries
the point of this variable is to limit memory consumtion


> 
> >
> >> -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.
> >
> > avidec alraedy reads the whole index for normal files. background index
> > building would only be used for truncated avi files
> 
> 
> >
> >
> >> -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.
> >
> > you wont even need a mutex
> > just put the index at some point that is accessable to the main demuxer
> > when its finished building
> > and the main demuxer then checks if thats non NULL and moves it over
> > that way the normal index is only accessed by the main demuxer
> > this can be implemented without a mutex
> Thanks, that's a good solution.  I think i'll just put the check/move
> in the seek function since that's the first point in code where we can
> use the new table.
> index_entries is also a public structure - is it mostly safe to say
> that clients haven't been using it or there is some doc saying not to?
> (as this would break under the move)

any client that writes to it is broken, a client reading it cant really
expect it to contain anythng beyond what is documented

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I wish the Xiph folks would stop pretending they've got something they
do not.  Somehow I fear this will remain a wish. -- Måns Rullgård
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-soc/attachments/20100619/fd78d1fd/attachment.pgp>