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

Michael Niedermayer michael at niedermayer.cc
Wed Aug 22 00:43:04 EEST 2018


On Mon, Aug 20, 2018 at 11:41:32AM -0700, Jacob Trimble wrote:
> 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?

how does this work ?
in what you describe IIUC there would be a demuxer connected to a muxer
its AVPackets would be passed from one to the other, possibly though
a FIFO adding arbitrary delay.
now if the demxuer updates its stream side data, what will transfer
this to the muxer ? they have seperate contexts.
And how will this occur at the right time in relation to the packets
also consider that the packets from 2 streams may be ordered differently
especially if the demxuer doesnt produce packets that are correctly
interleaved.
i may have misunderstood what you intended, please correct me if that is
so

also if the muxer removes stream side data elements from its context
nothing removes them from the demuxers context where new ones would
be added. I am not sure all user apps would handle this the same


thanks

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I am the wisest man alive, for I know one thing, and that is that I know
nothing. -- Socrates
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20180821/7ace2883/attachment.sig>


More information about the ffmpeg-devel mailing list