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

[Michael Niedermayer]

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


> +        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

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

There will always be a question for which you do not know the correct answer.
-------------- 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/20181125/f8e443ee/attachment.sig>