[FFmpeg-devel] [PATCH 1/4] mxfdec: Index table based seeking

[Tomas Härdin]

On Wed, 2011-09-28 at 21:59 +0200, Georg Lippitsch wrote:
> Am 27.09.2011, 11:08 Uhr, schrieb Tomas Härdin <tomas.hardin at codemill.se>:
> 
> > I took the liberty of refactoring and fixing a few bugs in your patch,
> > in addition to rebasing my partition parsing code on top of it and
> > adding some patches to support the V210 sample at
> > http://titan.codemill.se/~tomhar/samples/multiindex/
> 
> Here is a patch that makes the XDCAM HD files from your sample-directory
> seekable.
> 
> These files were indeed weird. They had every index entry doubled, and  
> therefore the nb_index_entries value was twice the index_duration. Even  
> mxfdump only parsed the indices for half of the file.

That's Avid for you.. IIRC there's actually 2*n+1 entries - the last
entry contains the length of the essence.

> I therefore added some code to remove double entries. Also, the sizes of  
> the index entries were not calculated if nb_delta_entries was 0, which  
> I've added now.

Yeah, I think they like the 64k limit of the local set coding. Neither
do I TBH.

> ...
> +    current = 0;
>      for (i = 0; i < segment->nb_index_entries; i++) {
> +        int flag_entry;
> +        uint64_t stream_offset_entry;
>          avio_rb16(pb);  /* TemporalOffset and KeyFrameOffset */
> -        segment->flag_entries[i] = avio_r8(pb);
> -        segment->stream_offset_entries[i] = avio_rb64(pb);
> -        if (segment->slice_count) {
> -            AV_MALLOC_ARR_OR_ENOMEM(segment->slice_offset_entries[i], segment->slice_count);
> -
> -            for (j = 0; j < segment->slice_count; j++) {
> -                segment->slice_offset_entries[i][j] = avio_rb32(pb);
> +        flag_entry = avio_r8(pb);
> +        stream_offset_entry = avio_rb64(pb);
> +        if (current == 0 || segment->stream_offset_entries[current-1] != stream_offset_entry)

Might want a comment above this "if" explaining why this is needed.
Something like /* handle Avid index tables with duplicate entries */

> +        {
> +            segment->flag_entries[current] = flag_entry;
> +            segment->stream_offset_entries[current] = stream_offset_entry;
> +            if (segment->slice_count) {
> +                AV_MALLOC_ARR_OR_ENOMEM(segment->slice_offset_entries[current],
> +                                        segment->slice_count);
> +                
> +                for (j = 0; j < segment->slice_count; j++) {
> +                    segment->slice_offset_entries[current][j] = avio_rb32(pb);
> +                }
> +            }
> +            current++;
> +        } else {
> +            if (segment->slice_count) {
> +                for (j = 0; j < segment->slice_count; j++)
> +                    avio_rb32(pb);

avio_skip(pb, 4*segment->slice_count);

>              }
>          }
>  
>          avio_skip(pb, length - 11 - 4 * segment->slice_count);
>      }
> +    segment->nb_unique_index_entries = current;

If my earlier memory was right, then we might want to do something like:

/* account for Avid's index tables having an extra entry for the size */
segment->nb_unique_index_entries = current ==
segment->nb_index_entries ? current : current - 1;

>      return 0;
>  }
>  
> @@ -982,7 +999,7 @@ static int mxf_parse_index(MXFContext *mxf, int i, AVStream *st)
>          st->index_entries = av_realloc(st->index_entries, st->index_entries_allocated_size);
>          for (k = 0; k < duration; k++) {
>              AVIndexEntry *e = &st->index_entries[st->nb_index_entries];
> -            if (k < tableseg->nb_index_entries) {
> +            if (k < tableseg->nb_unique_index_entries) {
>                  e->pos = tableseg->stream_offset_entries[k];
>                  if (n_delta < tableseg->nb_delta_entries) {
>                      if (n_delta < tableseg->nb_delta_entries - 1) {
> @@ -1002,6 +1019,9 @@ static int mxf_parse_index(MXFContext *mxf, int i, AVStream *st)
>                      if (tableseg->slice[n_delta] > 0)
>                          e->pos += tableseg->slice_offset_entries[k][tableseg->slice[n_delta]-1];
>                      e->pos += tableseg->element_delta[n_delta];
> +                } else if (k < duration - 1) {
> +                    e->size = tableseg->stream_offset_entries[k+1] -
> +                        tableseg->stream_offset_entries[k];
>                  }
>                  e->flags = !(tableseg->flag_entries[k] & 0x30) ? AVINDEX_KEYFRAME : 0;
>              } else {
> @@ -1481,8 +1501,9 @@ static int mxf_read_close(AVFormatContext *s)
>              break;
>          case IndexTableSegment:
>              seg = (MXFIndexTableSegment *)mxf->metadata_sets[i];
> -            for (j = 0; j < seg->nb_index_entries; j++)
> -                av_freep(&seg->slice_offset_entries[j]);
> +            if (seg->slice_count)
> +                for (j = 0; j < seg->nb_index_entries; j++)
> +                    av_freep(&seg->slice_offset_entries[j]);

Unrelated? Also, aren't all of these NULL if !slice_count and thus av_freep() is OK?

/Tomas