[FFmpeg-devel] [PATCH V4 5/6] lavf/isom: use side data to save max bit rate for esds box

mypopy at gmail.com mypopy at gmail.com
Mon Nov 26 09:04:39 EET 2018


On Mon, Nov 26, 2018 at 5:23 AM Michael Niedermayer <michael at niedermayer.cc>
wrote:
>
> On Sat, Nov 24, 2018 at 11:31:16AM +0800, Jun Zhao wrote:
> > Use AV_PKT_DATA_CPB_PROPERTIES to save max bit rate for esds box,
> > and update fate at the same time.
> >
> > Signed-off-by: Jun Zhao <mypopydev at gmail.com>
> > ---
> >  libavformat/isom.c                           |   19 ++++++++++++-------
> >  tests/ref/fate/adtstoasc_ticket3715          |    2 +-
> >  tests/ref/fate/copy-psp                      |    4 ++--
> >  tests/ref/fate/gaplessenc-itunes-to-ipod-aac |   10 ++++++----
> >  tests/ref/fate/gaplessenc-pcm-to-mov-aac     |   10 ++++++----
> >  tests/ref/fate/gaplessinfo-itunes1           |   10 ++++++----
> >  tests/ref/fate/gaplessinfo-itunes2           |   10 ++++++----
> >  tests/ref/fate/mov-mp3-demux                 |    2 +-
> >  8 files changed, 40 insertions(+), 27 deletions(-)
> >
> > diff --git a/libavformat/isom.c b/libavformat/isom.c
> > index ca9d22e..1780490 100644
> > --- a/libavformat/isom.c
> > +++ b/libavformat/isom.c
> > @@ -508,18 +508,23 @@ int ff_mp4_read_dec_config_descr(AVFormatContext
*fc, AVStream *st, AVIOContext
> >      int len, tag;
> >      int ret;
> >      int object_type_id = avio_r8(pb);
> > +    AVCPBProperties *props = NULL;
> > +
> >      avio_r8(pb); /* stream type */
> >      avio_rb24(pb); /* buffer size db */
> >
> >      v = avio_rb32(pb);
> >
> > -    // TODO: fix this with codecpar
> > -#if FF_API_LAVF_AVCTX
> > -FF_DISABLE_DEPRECATION_WARNINGS
> > -    if (v < INT32_MAX)
> > -        st->codec->rc_max_rate = v;
> > -FF_ENABLE_DEPRECATION_WARNINGS
> > -#endif
> > +    if (v < INT32_MAX) {
> > +        props = (AVCPBProperties *)av_stream_new_side_data(
> > +            st,
> > +            AV_PKT_DATA_CPB_PROPERTIES,
> > +            sizeof(*props));
>
> av_cpb_properties_alloc()
>
>
> > +        if (!props)
> > +            return AVERROR(ENOMEM);
>
> > +        memset(props, 0, sizeof(*props));
>
> code should avoid directly using the structure size, sometines these
structs
> can end up having fields added at their end.
> For example the INT32_MAX check here shows that the struct is broken and
> a 64bit field should have been used so a change will be needed eventually
>
will use av_cpb_properties_alloc/av_stream_add_side_data, Tks the comments.

>
> > +        props->max_bitrate = v; /* max bitrate */
>
> This patch is possibly the first that sets AV_PKT_DATA_CPB_PROPERTIES
> from a demuxer.
> So this will require some things to be checked.
> Muxers store the AV_PKT_DATA_CPB_PROPERTIES and demuxers read them
> that is fine but not together because applications can copy this even when
> using an encoder in between.
> I think there is no clean way to do this currently, but someone please
> correct me if there is ...
>
> applications need to know which side data types depend on the encoder,
> which on the resolution, sample rate, input stream semantically, and so on
> So that an applications can discard side data that has become incorrect
from
> some changes.
> Some of the side data that we have is encoder specific like the
bitrate/VBV
> parameters here while others are pure input describing.
>
> The idea that all this is uneeded because side data should just be passed
> through libavfilter also does not work as an application can use its own
> filter framework. It really seems to need to know what to do with newly
added
> types.
>
> If the side data from the demuxer with an encoder ends up actually written
> that would result in broken and invalid files being produced. So this
needs
> to be looked at carefully, we do not want to create files with incorrect
> VBV parameters.
>
> also more problems can possibly result from both a demuxer and an encoder
> producing this side data.
> Its important here to also understand that the applications which calls
the
> demuxer, muxer and encoder may have been written at a time before a
specific
> type of side data existed. So a solution has to not depend on an
application
> containing code specific to each side data type. As this would not work
very
> well when in the future new types are added.
>
> thx
>
> [...]
> --
Yes, when used side data from muxter/demuxer/encoder, the redundancy maybe
lead to some issue, and now side data didn't have a good/clear define for
this type case.

And I've tried to save the max bit rate(from MP4 esds box)
to AVStream.coderpar, then save the value to decodec context, but when sync
AVStream.codecpar/decoder context will drop this information.


More information about the ffmpeg-devel mailing list