[FFmpeg-devel] [PATCH 2/3] Add av_load_index_file and av_save_index_file

[Michael Niedermayer]

On Sun, Aug 22, 2010 at 11:01:12AM +0200, Michael Chinen wrote:
> On Thu, Aug 19, 2010 at 12:29 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> > On Thu, Aug 19, 2010 at 11:29:01AM +0200, Michael Chinen wrote:
> >> On Tue, Aug 17, 2010 at 2:07 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> >> [...]
> >> >> +/**
> >> >> + * Save the flags, size, and differences for the index table
> >> >> + */
> >> >> +#define FF_SAVEINDEX_PACKETSIZE 1000
> >> >> +static int av_save_index_stream(ByteIOContext *bc, AVStream *st,
> >> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?AVFormatContext *ic) {
> >> >> + ? ?AVIndexEntry start_ie;
> >> >> + ? ?AVIndexEntry *last_ie, *curr_ie;
> >> >> + ? ?int i;
> >> >> +
> >> >> + ? ?//we may need to save something like codec id to be safe.
> >> >> + ? ?ff_put_v(bc, st->seek_table_flags);
> >> >> + ? ?//we only write the index if we have a finished table.
> >> >> + ? ?if (st->seek_table_flags & AV_SEEKTABLE_FINISHED) {
> >> >> + ? ? ? ?ff_put_v(bc, st->nb_index_entries);
> >> >> +
> >> >> + ? ? ? ?memset(&start_ie, 0, sizeof(AVIndexEntry));
> >> >> + ? ? ? ?last_ie = &start_ie;
> >> >> + ? ? ? ?for (i = 0; i < st->nb_index_entries; i++) {
> >> >> + ? ? ? ? ? ?curr_ie = &st->index_entries[i];
> >> >
> >> >> + ? ? ? ? ? ?ff_put_v(bc, curr_ie->pos - last_ie->pos);
> >> >> + ? ? ? ? ? ?ff_put_v(bc, curr_ie->timestamp - last_ie->timestamp);
> >> >> + ? ? ? ? ? ?ff_put_v(bc, curr_ie->flags - last_ie->flags);
> >> >> + ? ? ? ? ? ?ff_put_v(bc, curr_ie->size - last_ie->size);
> >> >> + ? ? ? ? ? ?ff_put_v(bc, curr_ie->min_distance - last_ie->min_distance);
> >> >
> >> > vertical align please
> >>
> >> done.
> >>
> >> >
> >> >
> >> >> + ? ? ? ? ? ?last_ie = curr_ie;
> >> >
> >> >> + ? ? ? ? ? ?if (i % FF_SAVEINDEX_PACKETSIZE == 0)
> >> >> + ? ? ? ? ? ? ? ?put_flush_packet(bc);
> >> >
> >> > why?
> >>
> >> Unfamiliarity. ?Removed.
> >>
> >> >
> >> >
> >> > [...]
> >> >
> >> >> +static int av_load_index(ByteIOContext *bc, AVFormatContext *ic) {
> >> >> + ? ?int i, ret;
> >> >> + ? ?uint64_t v;
> >> >> + ? ?char read_str[256];
> >> >> +
> >> >> + ? ?get_strz(bc, read_str, 255);
> >> >
> >> > s/255/sizeof()/
> >>
> >> Done.
> >>
> >> Michael
> >
> >> ?avformat.h | ? 12 ++++
> >> ?utils.c ? ?| ?167 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >> ?2 files changed, 179 insertions(+)
> >> a7ff3f38a534c051b0d450ed379f005ad310311d ?0002-Add-av_load_index_file-and-av_save_index_file.patch
> >> From de8ad0accaaa6110430c52e28aef6418024bba2c Mon Sep 17 00:00:00 2001
> >> From: Michael Chinen <mchinen at gmail.com>
> >> Date: Thu, 19 Aug 2010 10:49:03 +0200
> >> Subject: [PATCH 2/3] Add av_load_index_file and av_save_index_file
> >>
> >> 2nd Submission - MN's review:
> >> remove packet flushing
> >> stylistic changes
> >> ---
> >> ?libavformat/avformat.h | ? 12 ++++
> >> ?libavformat/utils.c ? ?| ?167 ++++++++++++++++++++++++++++++++++++++++++++++++
> >> ?2 files changed, 179 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> >> index 6aeb460..5785f3a 100644
> >> --- a/libavformat/avformat.h
> >> +++ b/libavformat/avformat.h
> >> @@ -1243,6 +1243,18 @@ int av_build_index(AVFormatContext *s, int flags);
> >> ?#define AV_BUILD_INDEX_USE_CBR ?0x0002 ///< Detect and use CBR instead of index seeking
> >>
> >> ?/**
> >> + * Saves the index table built with av_build_index to a file.
> >> + * @return 0 if succesful or a negative value on failure.
> >> + */
> >> +int av_save_index_file(AVFormatContext *ic, const char *filename);
> >> +
> >> +/**
> >> + * Loads the index saved with av_save_index.
> >> + * @return 0 if successful or a negative value on failure.
> >> + */
> >> +int av_load_index_file(AVFormatContext *ic, const char *filename);
> >> +
> >> +/**
> >> ? * Ensure the index uses less memory than the maximum specified in
> >> ? * AVFormatContext.max_index_size by discarding entries if it grows
> >> ? * too large.
> >> diff --git a/libavformat/utils.c b/libavformat/utils.c
> >> index cc893f6..5633df6 100644
> >> --- a/libavformat/utils.c
> >> +++ b/libavformat/utils.c
> >> @@ -1822,6 +1822,173 @@ int avformat_seek_file(AVFormatContext *s, int stream_index, int64_t min_ts, int
> >> ? ? ?// try some generic seek like av_seek_frame_generic() but with new ts semantics
> >> ?}
> >>
> >> +/**
> >> + * Save the flags, size, and differences for the index table
> >> + */
> >> +static int av_save_index_stream(ByteIOContext *bc, AVStream *st,
> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?AVFormatContext *ic) {
> >> + ? ?AVIndexEntry start_ie;
> >> + ? ?AVIndexEntry *last_ie, *curr_ie;
> >> + ? ?int i;
> >> +
> >> + ? ?//we may need to save something like codec id to be safe.
> >> + ? ?ff_put_v(bc, st->seek_table_flags);
> >> + ? ?//we only write the index if we have a finished table.
> >> + ? ?if (st->seek_table_flags & AV_SEEKTABLE_FINISHED) {
> >> + ? ? ? ?ff_put_v(bc, st->nb_index_entries);
> >> +
> >> + ? ? ? ?memset(&start_ie, 0, sizeof(AVIndexEntry));
> >> + ? ? ? ?last_ie = &start_ie;
> >> + ? ? ? ?for (i = 0; i < st->nb_index_entries; i++) {
> >> + ? ? ? ? ? ?curr_ie = &st->index_entries[i];
> >> + ? ? ? ? ? ?ff_put_v(bc, curr_ie->pos ? ? ? ? ?- last_ie->pos);
> >> + ? ? ? ? ? ?ff_put_v(bc, curr_ie->timestamp ? ?- last_ie->timestamp);
> >> + ? ? ? ? ? ?ff_put_v(bc, curr_ie->flags ? ? ? ?- last_ie->flags);
> >> + ? ? ? ? ? ?ff_put_v(bc, curr_ie->size ? ? ? ? - last_ie->size);
> >> + ? ? ? ? ? ?ff_put_v(bc, curr_ie->min_distance - last_ie->min_distance);
> >> + ? ? ? ? ? ?last_ie = curr_ie;
> >> + ? ? ? ?}
> >> + ? ? ? ?av_log(ic, AV_LOG_DEBUG,
> >> + ? ? ? ? ? ? ? "successfully wrote index stream with %i entries.\n",
> >> + ? ? ? ? ? ? ? st->nb_index_entries);
> >> + ? ?}
> >> + ? ?put_flush_packet(bc);
> >> +
> >> + ? ?return 0;
> >> +}
> >
> > theres a race condition if 2 applications are run and one tries to load an
> > index that hasnt been finished yet.
> > this risk can be avoided by seeking back once its all finished and setting
> > some flag at the begin.
> > this could still leave a issue with 2 applications trying to write the same
> > index at the same time, this needs O_EXCL to be passed to open() and thus
> > requires a bit of api extension to pass this through
> 
> Okay, I did this the byte setting lock for now.

that still leaves the second race when 2 applications write the index
and some apps will likely write it automatically so this is a real issue


[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I have often repented speaking, but never of holding my tongue.
-- Xenocrates
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100825/0259bc9b/attachment.pgp>