[FFmpeg-devel] [PATCH] avformat/avformat.h: Add av_stream_remove_side_data.

[Jacob Trimble]

On Mon, Jul 9, 2018 at 9:57 AM Jacob Trimble <modmaker at google.com> wrote:
>
> On Tue, Jul 3, 2018 at 5:59 PM Michael Niedermayer
> <michael at niedermayer.cc> wrote:
> >
> > On Tue, Jul 03, 2018 at 12:14:19PM -0700, Jacob Trimble wrote:
> > > On Mon, Jul 2, 2018 at 6:07 PM Michael Niedermayer
> > > <michael at niedermayer.cc> wrote:
> > > >
> > > > On Mon, Jun 25, 2018 at 04:03:32PM -0700, Jacob Trimble wrote:
> > > > > Signed-off-by: Jacob Trimble <modmaker at google.com>
> > > > > ---
> > > > >  libavformat/avformat.h |  8 ++++++++
> > > > >  libavformat/utils.c    | 11 +++++++++++
> > > > >  2 files changed, 19 insertions(+)
> > > >
> > > >
> > > >
> > > >
> > > >
> > > > >
> > > > > diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> > > > > index fdaffa5bf4..434c88837e 100644
> > > > > --- a/libavformat/avformat.h
> > > > > +++ b/libavformat/avformat.h
> > > > > @@ -2167,6 +2167,14 @@ AVStream *avformat_new_stream(AVFormatContext *s, const AVCodec *c);
> > > > >  int av_stream_add_side_data(AVStream *st, enum AVPacketSideDataType type,
> > > > >                              uint8_t *data, size_t size);
> > > > >
> > > > > +/**
> > > > > + * Removes any existing side data of the given type.
> > > > > + *
> > > > > + * @param st stream
> > > > > + * @param type side information type
> > > > > + */
> > > > > +void av_stream_remove_side_data(AVStream *st, enum AVPacketSideDataType type);
> > > >
> > > > What would use this and why ?
> > > > The commit message does not explain this
> > > >
> > > > If side data is changing it probably should be put in AVPackets or AVFrames
> > > > not the stream.
> > > >
> > >
> > > I am using this to removing the side data that contains the
> > > AVEncryptionInitInfo objects once I handle them.  Since an MP4 file
> > > can contain multiple pssh atoms, there can be multiple
> > > AVEncryptionInitInfo structs.  To make it easier for me, I want to
> > > remove the side data that contain them once I have handled them.  This
> > > means that if the AVStream contains the side data, it is because of
> > > new init info I haven't seen.  Since the pssh atoms are more "global"
> > > it makes more sense to put them in the AVStream.
> >
> > I dont fully understand but
> > If you intend to remove things while reading the "header" of a mp4 file
> > these things probably should not be in side data to begin with but be
> > internal to the demuxer.
> >
> > otherwise, after the header or outside the demuxer removal seems a "no go"
> > but i may misunderstand what you intend to do. Please explain if iam
> > totally off here with how i interpret this
> >
> > One simple API good vs. bad test btw should be to consider that theres
> > a demuxer connected to a muxer.
> > If this does not work to produce a equivalent file the API is not good
> > for example if you change side data in the AVStream in the middle between
> > outputing packets i would not expect the muxer to see this and thus not
> > be able to reproduce this in the stored file.
> >
> > Also if you mess with the demxuers side data from outside, not only
> > will this result in undefined behavior it also might be that you still
> > need it when for example seeking back to the start
> >
> > again maybe i totally misunderstand what you intend here
> >
>
> I would expect the muxer to do what I am doing, it would remove the
> side data when it handles the data so it doesn't have to keep a copy
> of all the init data it has seen.
>
> For example, consider converting fragmented MP4 into a different
> fragmented MP4.  The pssh atoms can appear inside the fragments, so
> the muxer should see the new pssh atoms and add them to the
> current/next fragment while muxing.  The best way I see is to check if
> the side data exists, handle it, and remove the side data.  The
> alternative would be to convert the side data to the
> AVEncryptionInitInfo struct at every step, then compare each element
> against a copy the muxer has.  This is extremely slow and requires
> storing the init data several different ways.
>
> Another alternative would be to put the side data on the frames, but
> this doesn't seem right either.  The init info is "header" data, so it
> seems weird to put it on a random frame, and putting the data on every
> frame would be more duplication and require the muxer/app to compare
> them for every frame to detect new init info.
>

Does that make sense?  Is this something that could be merged?

> >
> > [...]
> > --
> > Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> >
> > The worst form of inequality is to try to make unequal things equal.
> > -- Aristotle
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel at ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel