| Message ID | 20200114234213.3224-4-nicolas.gaullier@cji.paris |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Fix mpeg1/2 stream copy | expand |
| Context | Check | Description |
|---|---|---|
| andriy/ffmpeg-patchwork | success | Make fate finished |
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 [...]
On 1/14/2020 8:42 PM, 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); cpb_props->buffer_size is initialized as zero, and rc_buffer_size is not meant to be used for decoding, so i imagine is also zero. > + if (s->bit_rate != 0x3FFFF*400) > + cpb_props->max_bitrate = FFMAX(cpb_props->max_bitrate, s->bit_rate); > + } > + > if (s->avctx->debug & FF_DEBUG_PICT_INFO) > av_log(s->avctx, AV_LOG_DEBUG, > "profile: %d, level: %d ps: %d cf:%d vbv buffer: %d, bitrate:%"PRId64"\n", > diff --git a/tests/ref/fate/mxf-probe-d10 b/tests/ref/fate/mxf-probe-d10 > index ab564467b5..317d4ae4c5 100644 > --- a/tests/ref/fate/mxf-probe-d10 > +++ b/tests/ref/fate/mxf-probe-d10 > @@ -50,6 +50,9 @@ DISPOSITION:clean_effects=0 > DISPOSITION:attached_pic=0 > DISPOSITION:timed_thumbnails=0 > TAG:file_package_umid=0x060A2B340101010501010D1313000000AE86B200913105800000080046A54011 > +[SIDE_DATA] > +side_data_type=CPB properties > +[/SIDE_DATA] > [/STREAM] > [STREAM] > index=1 > diff --git a/tests/ref/fate/ts-demux b/tests/ref/fate/ts-demux > index eb13ecc684..cdf34d6af0 100644 > --- a/tests/ref/fate/ts-demux > +++ b/tests/ref/fate/ts-demux > @@ -15,7 +15,7 @@ > 1, 5760, 5760, 2880, 1536, 0xbab5129c > 1, 8640, 8640, 2880, 1536, 0x602f034b, S=1, 1, 0x00bd00bd > 1, 11520, 11520, 2880, 906, 0x69cdcbcd > -0, 32037, 36541, 1501, 114336, 0x37a215a8, S=1, 1, 0x00e000e0 > +0, 32037, 36541, 1501, 114336, 0x37a215a8, S=2, 1, 0x00e000e0, 24, 0x663d0b52 > 0, 33538, 33538, 1501, 12560, 0xb559a3d4, F=0x0, S=1, 1, 0x00e000e0 > 0, 35040, 35040, 1501, 12704, 0x2614adf4, F=0x0, S=1, 1, 0x00e000e0 > 0, 36541, 41046, 1501, 51976, 0x9ff1dbfe, F=0x0, S=1, 1, 0x00e000e0 >
On 2/7/2020 7:43 PM, James Almer wrote: > On 1/14/2020 8:42 PM, 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); > > cpb_props->buffer_size is initialized as zero, and rc_buffer_size is not > meant to be used for decoding, so i imagine is also zero. Ok, so i see this decoder is setting rc_buffer_size despite the doxy stating it shouldn't, so nevermind that part. I think this should be done using an internal field in Mpeg1Context instead, or the API changed and it reflected in the doxy. I'm more inclined for the former option, since you'll be exporting the value using AVCPBProperties after all. > >> + if (s->bit_rate != 0x3FFFF*400) >> + cpb_props->max_bitrate = FFMAX(cpb_props->max_bitrate, s->bit_rate); >> + } >> + >> if (s->avctx->debug & FF_DEBUG_PICT_INFO) >> av_log(s->avctx, AV_LOG_DEBUG, >> "profile: %d, level: %d ps: %d cf:%d vbv buffer: %d, bitrate:%"PRId64"\n", >> diff --git a/tests/ref/fate/mxf-probe-d10 b/tests/ref/fate/mxf-probe-d10 >> index ab564467b5..317d4ae4c5 100644 >> --- a/tests/ref/fate/mxf-probe-d10 >> +++ b/tests/ref/fate/mxf-probe-d10 >> @@ -50,6 +50,9 @@ DISPOSITION:clean_effects=0 >> DISPOSITION:attached_pic=0 >> DISPOSITION:timed_thumbnails=0 >> TAG:file_package_umid=0x060A2B340101010501010D1313000000AE86B200913105800000080046A54011 >> +[SIDE_DATA] >> +side_data_type=CPB properties >> +[/SIDE_DATA] >> [/STREAM] >> [STREAM] >> index=1 >> diff --git a/tests/ref/fate/ts-demux b/tests/ref/fate/ts-demux >> index eb13ecc684..cdf34d6af0 100644 >> --- a/tests/ref/fate/ts-demux >> +++ b/tests/ref/fate/ts-demux >> @@ -15,7 +15,7 @@ >> 1, 5760, 5760, 2880, 1536, 0xbab5129c >> 1, 8640, 8640, 2880, 1536, 0x602f034b, S=1, 1, 0x00bd00bd >> 1, 11520, 11520, 2880, 906, 0x69cdcbcd >> -0, 32037, 36541, 1501, 114336, 0x37a215a8, S=1, 1, 0x00e000e0 >> +0, 32037, 36541, 1501, 114336, 0x37a215a8, S=2, 1, 0x00e000e0, 24, 0x663d0b52 >> 0, 33538, 33538, 1501, 12560, 0xb559a3d4, F=0x0, S=1, 1, 0x00e000e0 >> 0, 35040, 35040, 1501, 12704, 0x2614adf4, F=0x0, S=1, 1, 0x00e000e0 >> 0, 36541, 41046, 1501, 51976, 0x9ff1dbfe, F=0x0, S=1, 1, 0x00e000e0 >> >
> De : ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> De la part de Michael Niedermayer > Envoyé : vendredi 7 février 2020 23:39 > À : FFmpeg development discussions and patches <ffmpeg-devel@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. Nicolas
> De : ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> De la part de James Almer > Envoyé : vendredi 7 février 2020 23:48 > À : ffmpeg-devel@ffmpeg.org > Objet : Re: [FFmpeg-devel] [PATCH v7 3/3] avcodec/mpeg12dec: Add CPB coded side data > > On 2/7/2020 7:43 PM, James Almer wrote: > > On 1/14/2020 8:42 PM, 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); > > > > cpb_props->buffer_size is initialized as zero, and rc_buffer_size is not > > meant to be used for decoding, so i imagine is also zero. > > Ok, so i see this decoder is setting rc_buffer_size despite the doxy > stating it shouldn't, so nevermind that part. > > I think this should be done using an internal field in Mpeg1Context > instead, or the API changed and it reflected in the doxy. I'm more > inclined for the former option, since you'll be exporting the value > using AVCPBProperties after all. I agree with you, but this is not related to my patchset, so this should be fixed in another patch. And I would like not to postpone this again... do you mind if this patch is applied first and if I fix this mpeg12dec design issue later (I am committing myself to do it) ? Nicolas
On Thu, Feb 20, 2020 at 11:22:33AM +0000, Gaullier Nicolas wrote: > > De : ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> De la part de Michael Niedermayer > > Envoyé : vendredi 7 février 2020 23:39 > > À : FFmpeg development discussions and patches <ffmpeg-devel@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 [...]
> De : ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> De la part de Michael Niedermayer > Envoyé : vendredi 21 février 2020 12:54 > À : FFmpeg development discussions and patches <ffmpeg-devel@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@ffmpeg.org> De la part de Michael Niedermayer > > > Envoyé : vendredi 7 février 2020 23:39 > > > À : FFmpeg development discussions and patches <ffmpeg-devel@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
On Fri, Feb 21, 2020 at 01:20:47PM +0000, Gaullier Nicolas wrote: > > De : ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> De la part de Michael Niedermayer > > Envoyé : vendredi 21 février 2020 12:54 > > À : FFmpeg development discussions and patches <ffmpeg-devel@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@ffmpeg.org> De la part de Michael Niedermayer > > > > Envoyé : vendredi 7 février 2020 23:39 > > > > À : FFmpeg development discussions and patches <ffmpeg-devel@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. well, i see your point but that would not work in practice because in a scenario where a stream is remuxed the output properties would be written in regular intervalls before the input is read to the end so that would not work. Outputing the values as is would at least preserve the values as they are > 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 [...]
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); + } + if (s->avctx->debug & FF_DEBUG_PICT_INFO) av_log(s->avctx, AV_LOG_DEBUG, "profile: %d, level: %d ps: %d cf:%d vbv buffer: %d, bitrate:%"PRId64"\n", diff --git a/tests/ref/fate/mxf-probe-d10 b/tests/ref/fate/mxf-probe-d10 index ab564467b5..317d4ae4c5 100644 --- a/tests/ref/fate/mxf-probe-d10 +++ b/tests/ref/fate/mxf-probe-d10 @@ -50,6 +50,9 @@ DISPOSITION:clean_effects=0 DISPOSITION:attached_pic=0 DISPOSITION:timed_thumbnails=0 TAG:file_package_umid=0x060A2B340101010501010D1313000000AE86B200913105800000080046A54011 +[SIDE_DATA] +side_data_type=CPB properties +[/SIDE_DATA] [/STREAM] [STREAM] index=1 diff --git a/tests/ref/fate/ts-demux b/tests/ref/fate/ts-demux index eb13ecc684..cdf34d6af0 100644 --- a/tests/ref/fate/ts-demux +++ b/tests/ref/fate/ts-demux @@ -15,7 +15,7 @@ 1, 5760, 5760, 2880, 1536, 0xbab5129c 1, 8640, 8640, 2880, 1536, 0x602f034b, S=1, 1, 0x00bd00bd 1, 11520, 11520, 2880, 906, 0x69cdcbcd -0, 32037, 36541, 1501, 114336, 0x37a215a8, S=1, 1, 0x00e000e0 +0, 32037, 36541, 1501, 114336, 0x37a215a8, S=2, 1, 0x00e000e0, 24, 0x663d0b52 0, 33538, 33538, 1501, 12560, 0xb559a3d4, F=0x0, S=1, 1, 0x00e000e0 0, 35040, 35040, 1501, 12704, 0x2614adf4, F=0x0, S=1, 1, 0x00e000e0 0, 36541, 41046, 1501, 51976, 0x9ff1dbfe, F=0x0, S=1, 1, 0x00e000e0