[FFmpeg-devel] [PATCH v7 3/3] avcodec/mpeg12dec: Add CPB coded side data

[Gaullier Nicolas]

> De : ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> De la part de Michael Niedermayer
> Envoyé : vendredi 21 février 2020 12:54
> À : FFmpeg development discussions and patches <ffmpeg-devel at ffmpeg.org>
> Objet : Re: [FFmpeg-devel] [PATCH v7 3/3] avcodec/mpeg12dec: Add CPB coded side data
> 
> On Thu, Feb 20, 2020 at 11:22:33AM +0000, Gaullier Nicolas wrote:
> > > De : ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> De la part de Michael Niedermayer
> > > Envoyé : vendredi 7 février 2020 23:39
> > > À : FFmpeg development discussions and patches <ffmpeg-devel at ffmpeg.org>
> > > Objet : Re: [FFmpeg-devel] [PATCH v7 3/3] avcodec/mpeg12dec: Add CPB coded side data
> > >
> > > On Wed, Jan 15, 2020 at 12:42:13AM +0100, Nicolas Gaullier wrote:
> > > > This fixes mpeg2video stream copies to mpeg muxer like this:
> > > >   ffmpeg -i xdcamhd.mxf -c:v copy output.mpg
> > > > ---
> > > >  libavcodec/mpeg12dec.c       | 7 +++++++
> > > >  tests/ref/fate/mxf-probe-d10 | 3 +++
> > > >  tests/ref/fate/ts-demux      | 2 +-
> > > >  3 files changed, 11 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c index
> > > > 17f9495a1d..48ac14fafa 100644
> > > > --- a/libavcodec/mpeg12dec.c
> > > > +++ b/libavcodec/mpeg12dec.c
> > > > @@ -1398,6 +1398,7 @@ static void mpeg_decode_sequence_extension(Mpeg1Context *s1)
> > > >      MpegEncContext *s = &s1->mpeg_enc_ctx;
> > > >      int horiz_size_ext, vert_size_ext;
> > > >      int bit_rate_ext;
> > > > +    AVCPBProperties *cpb_props;
> > > >
> > > >      skip_bits(&s->gb, 1); /* profile and level esc*/
> > > >      s->avctx->profile       = get_bits(&s->gb, 3);
> > > > @@ -1429,6 +1430,12 @@ static void mpeg_decode_sequence_extension(Mpeg1Context *s1)
> > > >      ff_dlog(s->avctx, "sequence extension\n");
> > > >      s->codec_id = s->avctx->codec_id = AV_CODEC_ID_MPEG2VIDEO;
> > > >
> > > > +    if (cpb_props = ff_add_cpb_side_data(s->avctx)) {
> > > > +        cpb_props->buffer_size = FFMAX(cpb_props->buffer_size, s->avctx->rc_buffer_size);
> > > > +        if (s->bit_rate != 0x3FFFF*400)
> > > > +            cpb_props->max_bitrate = FFMAX(cpb_props->max_bitrate, s->bit_rate);
> > > > +    }
> > >
> > > why does this not export exactly the numbers as read from the header?
> > >
> > > thx
> > The header values are expressed in units of 400bit/s, and the native value 0x3FFFF is reserved, in case of MPEG-1 (but the code is
> shared), for vbr signalling.
> > This is not very nice to read, but this is how it is implemented in current code.
> 
> you misunderstand, why do you take the maximum of several things instead of
> exporting the value from the header ?
> 
> Thanks
Sorry for my misunderstanding. I thought the cpb properties had to reflect the entire stream at the end and thus cumulate the size/max values.
I agree it is best to have an exact match with native header values.
Thank you for your feedback.
I will send a new version with the 2x FFMAX removed. 
Nicolas