Re: [FFmpeg-devel] [PATCH 1/1] avformat/mpegenc.c: vbvsize option
On Tue, Nov 5, 2019 at 5:44 PM Gaullier Nicolas wrote: > > >>On Thu, Oct 31, 2019 at 06:04:58PM +0100, Nicolas Gaullier wrote: > >> Allow the user to set or override the vbv size > >> --- > >> libavformat/mpegenc.c | 6 +- > >> 1 file changed, 5 insertions(+), 1 deletion(-) > > > >This is not the "correct" way to handle this, because one mpeg container > >can contain many streams and this is just one parameter for the container. > >while the vbvsize is a parameter per stream. > > FYI, my main concern/use case is to be able to rewrap an input mpg file to a > new mpg file while keeping its vbv size. > This was working back in version 2.8.9, just before codecpar was introduced > (ie. regression)... > My first proposal would be very straightforward : insert a new > "rc_buffer_size" field at the end of AVCodecParameters but that would break > the ABI? > Another point is that rc_buffer_size may be carried by stream side_data when > encoding, so that would mean two representations for the same information, I > don't know if this sounds "correct". > At the end, do you think amending AVCodecParameters would be acceptable ? > The fact is, I have no other idea because I don't see anyway how to setup > stream side_data from within the codec of the input stream. Amending codecpar with such specific fields is not acceptable. As you noted, we already have stream sidedata to carry these particular values, so you should figure out a way to fill that sidedata from the origin container, its the only proper way. - Hendrik ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 1/1] avformat/mpegenc.c: vbvsize option
>>On Thu, Oct 31, 2019 at 06:04:58PM +0100, Nicolas Gaullier wrote: >> Allow the user to set or override the vbv size >> --- >> libavformat/mpegenc.c | 6 +- >> 1 file changed, 5 insertions(+), 1 deletion(-) > >This is not the "correct" way to handle this, because one mpeg container >can contain many streams and this is just one parameter for the container. >while the vbvsize is a parameter per stream. FYI, my main concern/use case is to be able to rewrap an input mpg file to a new mpg file while keeping its vbv size. This was working back in version 2.8.9, just before codecpar was introduced (ie. regression)... My first proposal would be very straightforward : insert a new "rc_buffer_size" field at the end of AVCodecParameters but that would break the ABI? Another point is that rc_buffer_size may be carried by stream side_data when encoding, so that would mean two representations for the same information, I don't know if this sounds "correct". At the end, do you think amending AVCodecParameters would be acceptable ? The fact is, I have no other idea because I don't see anyway how to setup stream side_data from within the codec of the input stream. Thank you for your review and ... help Nicolas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/1] avformat/mpegenc.c: vbvsize option
On Thu, Oct 31, 2019 at 06:04:58PM +0100, Nicolas Gaullier wrote:
> Allow the user to set or override the vbv size
> ---
> libavformat/mpegenc.c | 6 +-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/libavformat/mpegenc.c b/libavformat/mpegenc.c
> index f6980231a2..1613c8afa1 100644
> --- a/libavformat/mpegenc.c
> +++ b/libavformat/mpegenc.c
> @@ -67,6 +67,7 @@ typedef struct MpegMuxContext {
> int system_header_freq;
> int system_header_size;
> int user_mux_rate; /* bitrate in units of bits/s */
> +int user_vbv_size; /* vbv buffer size in units of bits/s */
> int mux_rate; /* bitrate in units of 50 bytes/s */
> /* stream info */
> int audio_bound;
> @@ -433,7 +434,9 @@ static av_cold int mpeg_mux_init(AVFormatContext *ctx)
> stream->id = mpv_id++;
>
> props = (AVCPBProperties*)av_stream_get_side_data(st,
> AV_PKT_DATA_CPB_PROPERTIES, NULL);
> -if (props && props->buffer_size)
> +if (s->user_vbv_size)
> +stream->max_buffer_size = 6 * 1024 + s->user_vbv_size / 8;
> +else if (props && props->buffer_size)
> stream->max_buffer_size = 6 * 1024 + props->buffer_size / 8;
> else {
> av_log(ctx, AV_LOG_WARNING,
> @@ -1268,6 +1271,7 @@ static void mpeg_mux_deinit(AVFormatContext *ctx)
> #define E AV_OPT_FLAG_ENCODING_PARAM
> static const AVOption options[] = {
> { "muxrate", NULL,
> OFFSET(user_mux_rate), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, ((1<<22) - 1) * (8 *
> 50), E },
> +{ "vbvsize", "set vbv buffer size (in bits)",
> OFFSET(user_vbv_size), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, (1LL<<32) - 1, E},
> { "preload", "Initial demux-decode delay in microseconds.",
> OFFSET(preload), AV_OPT_TYPE_INT, { .i64 = 50 }, 0, INT_MAX, E },
> { NULL },
> };
This is not the "correct" way to handle this, because one mpeg container
can contain many streams and this is just one parameter for the container.
while the vbvsize is a parameter per stream.
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
In a rich man's house there is no place to spit but his face.
-- Diogenes of Sinope
signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 1/1] avformat/mpegenc.c: vbvsize option
Allow the user to set or override the vbv size
---
libavformat/mpegenc.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/libavformat/mpegenc.c b/libavformat/mpegenc.c
index f6980231a2..1613c8afa1 100644
--- a/libavformat/mpegenc.c
+++ b/libavformat/mpegenc.c
@@ -67,6 +67,7 @@ typedef struct MpegMuxContext {
int system_header_freq;
int system_header_size;
int user_mux_rate; /* bitrate in units of bits/s */
+int user_vbv_size; /* vbv buffer size in units of bits/s */
int mux_rate; /* bitrate in units of 50 bytes/s */
/* stream info */
int audio_bound;
@@ -433,7 +434,9 @@ static av_cold int mpeg_mux_init(AVFormatContext *ctx)
stream->id = mpv_id++;
props = (AVCPBProperties*)av_stream_get_side_data(st,
AV_PKT_DATA_CPB_PROPERTIES, NULL);
-if (props && props->buffer_size)
+if (s->user_vbv_size)
+stream->max_buffer_size = 6 * 1024 + s->user_vbv_size / 8;
+else if (props && props->buffer_size)
stream->max_buffer_size = 6 * 1024 + props->buffer_size / 8;
else {
av_log(ctx, AV_LOG_WARNING,
@@ -1268,6 +1271,7 @@ static void mpeg_mux_deinit(AVFormatContext *ctx)
#define E AV_OPT_FLAG_ENCODING_PARAM
static const AVOption options[] = {
{ "muxrate", NULL,
OFFSET(user_mux_rate), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, ((1<<22) - 1) * (8 *
50), E },
+{ "vbvsize", "set vbv buffer size (in bits)",
OFFSET(user_vbv_size), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, (1LL<<32) - 1, E},
{ "preload", "Initial demux-decode delay in microseconds.",
OFFSET(preload), AV_OPT_TYPE_INT, { .i64 = 50 }, 0, INT_MAX, E },
{ NULL },
};
--
2.22.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
