Re: [FFmpeg-devel] [PATCH 2/2] avformat/mxfenc: do not write index tables with the same InstanceUID

From: Marton Balint <cus@passwd.hu>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH 2/2] avformat/mxfenc: do not write index tables with the same InstanceUID
Date: Mon, 14 Mar 2022 21:44:29 +0100 (CET)
Message-ID: <6a996093-83fe-53c9-c0c0-6cbf9c5ad1e@passwd.hu> (raw)
In-Reply-To: <718be8d98b74fd2f62ef9fc0c4350ee60ce46b47.camel@acc.umu.se>

On Mon, 14 Mar 2022, Tomas Härdin wrote:

> mån 2022-03-14 klockan 20:54 +0100 skrev Marton Balint:
>> 
>> 
>> On Mon, 14 Mar 2022, Tomas Härdin wrote:
>> 
>> > mån 2022-03-14 klockan 19:49 +0100 skrev Marton Balint:
>> > > Only index tables repeating previous index tables should use the
>> > > same
>> > > InstaceUID. Use the index start position when generating the
>> > > InstanceUID to fix
>> > > this.
>> > > 
>> > > Signed-off-by: Marton Balint <cus@passwd.hu>
>> > > ---
>> > >  libavformat/mxfenc.c | 2 +-
>> > >  1 file changed, 1 insertion(+), 1 deletion(-)
>> > > 
>> > > diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c
>> > > index ba8e7babfb..5b972eadaa 100644
>> > > --- a/libavformat/mxfenc.c
>> > > +++ b/libavformat/mxfenc.c
>> > > @@ -1757,7 +1757,7 @@ static void
>> > > mxf_write_index_table_segment(AVFormatContext *s)
>> > >  
>> > >      // instance id
>> > >      mxf_write_local_tag(s, 16, 0x3C0A);
>> > > -    mxf_write_uuid(pb, IndexTableSegment, 0);
>> > > +    mxf_write_uuid(pb, IndexTableSegment, mxf-
>> > > > last_indexed_edit_unit);
>> > 
>> > Two things: yes, it is good that this fixes the same InstanceUID
>> > being
>> > reused. But more importantly, we should not be writing files with
>> > over
>> > 65536 partitions!
>> 
>> last_indexed_edit_unit is frame based not partition based, so it can 
>> overflow 65536 realtively easily, that is why I submitted patch 1.
>
> Right. But we could use the partition number instead.

Well, we could use mxf->body_partitions_count but it is not trivial to see 
that it will work for all cases. For simple indexes, we rewrite the index 
table in the footer when writing the mxf header, opatom may follow another 
layout, so it just felt less error-prone to use actually the start offset 
of the index.

>
>> 
>> > 
>> > This has been bugging me for quite some time. Honestly I don't know
>> > why
>> > the decision was taken initially to write indices every 10 seconds.
>> > In
>> > any use-case where seeks are moderately expensive working with
>> > files
>> > produced by mxfenc is a nightmare. Prime example being HTTP.
>> 
>> The 10 second body partition limit is coming from some specification 
>> (XDCAM HD?), so this is kind of intentional.
>> 
>> > 
>> > If we do still need to keep writing partitions this way, can we
>> > repeat
>> > the IndexTableSegments in the footer so the entire file doesn't
>> > have to
>> > be scanned?
>> 
>> Yeah, that is what smart tools like bmxtools are doing.
>
> If XDCAM requires this amount of partitions then yeah, probably write
> the index tables twice. That way a smart reader should be able to
> figure out that it doesn't need to read more than the header, RIP and
> footer.

Sure, but this can be another patch.

Thanks,
Marton
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".