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

From: "Tomas Härdin" <tjoppen@acc.umu.se>
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: Wed, 16 Mar 2022 21:06:35 +0100
Message-ID: <4ddc8d7856a4013833a413ac86c4cab09c610f6e.camel@acc.umu.se> (raw)
In-Reply-To: <c8c2be49-e31f-5193-bf67-d8773d3f93c@passwd.hu>

ons 2022-03-16 klockan 20:38 +0100 skrev Marton Balint:
> 
> 
> On Wed, 16 Mar 2022, Tomas Härdin wrote:
> 
> > mån 2022-03-14 klockan 21:44 +0100 skrev Marton Balint:
> > > 
> > > 
> > > 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.
> > 
> > I don't see why not. But upping to 32-bit is easy anyways.
> 
> I tried, but body partition count is the same for the last body
> partition 
> and for the footer partition, both having different index tables...
> 
> So I still find it more starightforward to use index start position 
> instead of some magic to find out the proper partition count, is it
> fine 
> with you?

I mean it shouldn't matter so long as its unique. So sure

/Tomas

_______________________________________________
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".