* [FFmpeg-devel] [PATCH 1/8] fftools/ffmpeg_demux: replace abort() by av_assert0(0) @ 2024-01-05 16:42 Anton Khirnov 2024-01-05 16:42 ` [FFmpeg-devel] [PATCH 2/8] doc/ffmpeg: drop documentation for non-existent -sbsf Anton Khirnov ` (7 more replies) 0 siblings, 8 replies; 28+ messages in thread From: Anton Khirnov @ 2024-01-05 16:42 UTC (permalink / raw) To: ffmpeg-devel This is the standard way to mark unreachable cases in a switch --- fftools/ffmpeg_demux.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c index 95af31e9ef..5d07b7153d 100644 --- a/fftools/ffmpeg_demux.c +++ b/fftools/ffmpeg_demux.c @@ -1223,8 +1223,7 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st) case AVMEDIA_TYPE_ATTACHMENT: case AVMEDIA_TYPE_UNKNOWN: break; - default: - abort(); + default: av_assert0(0); } ist->par = avcodec_parameters_alloc(); -- 2.42.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-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 28+ messages in thread
* [FFmpeg-devel] [PATCH 2/8] doc/ffmpeg: drop documentation for non-existent -sbsf 2024-01-05 16:42 [FFmpeg-devel] [PATCH 1/8] fftools/ffmpeg_demux: replace abort() by av_assert0(0) Anton Khirnov @ 2024-01-05 16:42 ` Anton Khirnov 2024-01-06 11:22 ` Stefano Sabatini 2024-01-05 16:42 ` [FFmpeg-devel] [PATCH 3/8] fftools/ffmpeg_demux: do not set bitexact directly on the decoder Anton Khirnov ` (6 subsequent siblings) 7 siblings, 1 reply; 28+ messages in thread From: Anton Khirnov @ 2024-01-05 16:42 UTC (permalink / raw) To: ffmpeg-devel --- doc/ffmpeg.texi | 2 -- 1 file changed, 2 deletions(-) diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi index 7246a46d2f..d75517b443 100644 --- a/doc/ffmpeg.texi +++ b/doc/ffmpeg.texi @@ -1643,8 +1643,6 @@ option to disable streams individually. As an output option, disables subtitle recording i.e. automatic selection or mapping of any subtitle stream. For full manual control see the @code{-map} option. -@item -sbsf @var{bitstream_filter} -Deprecated, see -bsf @end table @section Advanced Subtitle options -- 2.42.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-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [FFmpeg-devel] [PATCH 2/8] doc/ffmpeg: drop documentation for non-existent -sbsf 2024-01-05 16:42 ` [FFmpeg-devel] [PATCH 2/8] doc/ffmpeg: drop documentation for non-existent -sbsf Anton Khirnov @ 2024-01-06 11:22 ` Stefano Sabatini 0 siblings, 0 replies; 28+ messages in thread From: Stefano Sabatini @ 2024-01-06 11:22 UTC (permalink / raw) To: FFmpeg development discussions and patches On date Friday 2024-01-05 17:42:45 +0100, Anton Khirnov wrote: > --- > doc/ffmpeg.texi | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi > index 7246a46d2f..d75517b443 100644 > --- a/doc/ffmpeg.texi > +++ b/doc/ffmpeg.texi > @@ -1643,8 +1643,6 @@ option to disable streams individually. > As an output option, disables subtitle recording i.e. automatic selection or > mapping of any subtitle stream. For full manual control see the @code{-map} > option. > -@item -sbsf @var{bitstream_filter} > -Deprecated, see -bsf > @end table LGTM given this has been deprecated for twelve years and counting. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 28+ messages in thread
* [FFmpeg-devel] [PATCH 3/8] fftools/ffmpeg_demux: do not set bitexact directly on the decoder 2024-01-05 16:42 [FFmpeg-devel] [PATCH 1/8] fftools/ffmpeg_demux: replace abort() by av_assert0(0) Anton Khirnov 2024-01-05 16:42 ` [FFmpeg-devel] [PATCH 2/8] doc/ffmpeg: drop documentation for non-existent -sbsf Anton Khirnov @ 2024-01-05 16:42 ` Anton Khirnov 2024-01-06 11:24 ` Stefano Sabatini 2024-01-05 16:42 ` [FFmpeg-devel] [PATCH 4/8] fftools/ffmpeg_demux: set options on codec parameters rather than decoder Anton Khirnov ` (5 subsequent siblings) 7 siblings, 1 reply; 28+ messages in thread From: Anton Khirnov @ 2024-01-05 16:42 UTC (permalink / raw) To: ffmpeg-devel Add it to decoder options instead, to be processed when opening the decoder. This way it won't be overridden by flags the user might be setting otherwise. --- fftools/ffmpeg_demux.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c index 5d07b7153d..cacdc76a71 100644 --- a/fftools/ffmpeg_demux.c +++ b/fftools/ffmpeg_demux.c @@ -1156,7 +1156,7 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st) } if (o->bitexact) - ist->dec_ctx->flags |= AV_CODEC_FLAG_BITEXACT; + av_dict_set(&ist->decoder_opts, "flags", "+bitexact", AV_DICT_MULTIKEY); switch (par->codec_type) { case AVMEDIA_TYPE_VIDEO: -- 2.42.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-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [FFmpeg-devel] [PATCH 3/8] fftools/ffmpeg_demux: do not set bitexact directly on the decoder 2024-01-05 16:42 ` [FFmpeg-devel] [PATCH 3/8] fftools/ffmpeg_demux: do not set bitexact directly on the decoder Anton Khirnov @ 2024-01-06 11:24 ` Stefano Sabatini 0 siblings, 0 replies; 28+ messages in thread From: Stefano Sabatini @ 2024-01-06 11:24 UTC (permalink / raw) To: FFmpeg development discussions and patches On date Friday 2024-01-05 17:42:46 +0100, Anton Khirnov wrote: > Add it to decoder options instead, to be processed when opening the > decoder. This way it won't be overridden by flags the user might be > setting otherwise. > --- > fftools/ffmpeg_demux.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c > index 5d07b7153d..cacdc76a71 100644 > --- a/fftools/ffmpeg_demux.c > +++ b/fftools/ffmpeg_demux.c > @@ -1156,7 +1156,7 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st) > } > > if (o->bitexact) > - ist->dec_ctx->flags |= AV_CODEC_FLAG_BITEXACT; > + av_dict_set(&ist->decoder_opts, "flags", "+bitexact", AV_DICT_MULTIKEY); LGTM. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 28+ messages in thread
* [FFmpeg-devel] [PATCH 4/8] fftools/ffmpeg_demux: set options on codec parameters rather than decoder 2024-01-05 16:42 [FFmpeg-devel] [PATCH 1/8] fftools/ffmpeg_demux: replace abort() by av_assert0(0) Anton Khirnov 2024-01-05 16:42 ` [FFmpeg-devel] [PATCH 2/8] doc/ffmpeg: drop documentation for non-existent -sbsf Anton Khirnov 2024-01-05 16:42 ` [FFmpeg-devel] [PATCH 3/8] fftools/ffmpeg_demux: do not set bitexact directly on the decoder Anton Khirnov @ 2024-01-05 16:42 ` Anton Khirnov 2024-01-06 11:31 ` Stefano Sabatini 2024-01-06 14:22 ` James Almer 2024-01-05 16:42 ` [FFmpeg-devel] [PATCH 5/8] fftools/ffmpeg_demux: only allocate the decoder when actually decoding Anton Khirnov ` (4 subsequent siblings) 7 siblings, 2 replies; 28+ messages in thread From: Anton Khirnov @ 2024-01-05 16:42 UTC (permalink / raw) To: ffmpeg-devel This avoids the requirement to always have a decoder context. --- fftools/ffmpeg_demux.c | 43 +++++++++++++++++++++--------------------- 1 file changed, 21 insertions(+), 22 deletions(-) diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c index cacdc76a71..892094c512 100644 --- a/fftools/ffmpeg_demux.c +++ b/fftools/ffmpeg_demux.c @@ -899,19 +899,18 @@ static int choose_decoder(const OptionsContext *o, AVFormatContext *s, AVStream } } -static int guess_input_channel_layout(InputStream *ist, int guess_layout_max) +static int guess_input_channel_layout(InputStream *ist, AVCodecParameters *par, + int guess_layout_max) { - AVCodecContext *dec = ist->dec_ctx; - - if (dec->ch_layout.order == AV_CHANNEL_ORDER_UNSPEC) { + if (par->ch_layout.order == AV_CHANNEL_ORDER_UNSPEC) { char layout_name[256]; - if (dec->ch_layout.nb_channels > guess_layout_max) + if (par->ch_layout.nb_channels > guess_layout_max) return 0; - av_channel_layout_default(&dec->ch_layout, dec->ch_layout.nb_channels); - if (dec->ch_layout.order == AV_CHANNEL_ORDER_UNSPEC) + av_channel_layout_default(&par->ch_layout, par->ch_layout.nb_channels); + if (par->ch_layout.order == AV_CHANNEL_ORDER_UNSPEC) return 0; - av_channel_layout_describe(&dec->ch_layout, layout_name, sizeof(layout_name)); + av_channel_layout_describe(&par->ch_layout, layout_name, sizeof(layout_name)); av_log(ist, AV_LOG_WARNING, "Guessed Channel Layout: %s\n", layout_name); } return 1; @@ -1145,16 +1144,6 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st) ist->user_set_discard = ist->st->discard; } - ist->dec_ctx = avcodec_alloc_context3(ist->dec); - if (!ist->dec_ctx) - return AVERROR(ENOMEM); - - ret = avcodec_parameters_to_context(ist->dec_ctx, par); - if (ret < 0) { - av_log(ist, AV_LOG_ERROR, "Error initializing the decoder context.\n"); - return ret; - } - if (o->bitexact) av_dict_set(&ist->decoder_opts, "flags", "+bitexact", AV_DICT_MULTIKEY); @@ -1181,7 +1170,7 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st) case AVMEDIA_TYPE_AUDIO: { int guess_layout_max = INT_MAX; MATCH_PER_STREAM_OPT(guess_layout_max, i, guess_layout_max, ic, st); - guess_input_channel_layout(ist, guess_layout_max); + guess_input_channel_layout(ist, par, guess_layout_max); break; } case AVMEDIA_TYPE_DATA: @@ -1190,7 +1179,7 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st) MATCH_PER_STREAM_OPT(fix_sub_duration, i, ist->fix_sub_duration, ic, st); MATCH_PER_STREAM_OPT(canvas_sizes, str, canvas_size, ic, st); if (canvas_size) { - ret = av_parse_video_size(&ist->dec_ctx->width, &ist->dec_ctx->height, + ret = av_parse_video_size(&par->width, &par->height, canvas_size); if (ret < 0) { av_log(ist, AV_LOG_FATAL, "Invalid canvas size: %s.\n", canvas_size); @@ -1201,8 +1190,8 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st) /* Compute the size of the canvas for the subtitles stream. If the subtitles codecpar has set a size, use it. Otherwise use the maximum dimensions of the video streams in the same file. */ - ist->sub2video.w = ist->dec_ctx->width; - ist->sub2video.h = ist->dec_ctx->height; + ist->sub2video.w = par->width; + ist->sub2video.h = par->height; if (!(ist->sub2video.w && ist->sub2video.h)) { for (int j = 0; j < ic->nb_streams; j++) { AVCodecParameters *par1 = ic->streams[j]->codecpar; @@ -1226,6 +1215,16 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st) default: av_assert0(0); } + ist->dec_ctx = avcodec_alloc_context3(ist->dec); + if (!ist->dec_ctx) + return AVERROR(ENOMEM); + + ret = avcodec_parameters_to_context(ist->dec_ctx, par); + if (ret < 0) { + av_log(ist, AV_LOG_ERROR, "Error initializing the decoder context.\n"); + return ret; + } + ist->par = avcodec_parameters_alloc(); if (!ist->par) return AVERROR(ENOMEM); -- 2.42.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-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [FFmpeg-devel] [PATCH 4/8] fftools/ffmpeg_demux: set options on codec parameters rather than decoder 2024-01-05 16:42 ` [FFmpeg-devel] [PATCH 4/8] fftools/ffmpeg_demux: set options on codec parameters rather than decoder Anton Khirnov @ 2024-01-06 11:31 ` Stefano Sabatini 2024-01-06 14:22 ` James Almer 1 sibling, 0 replies; 28+ messages in thread From: Stefano Sabatini @ 2024-01-06 11:31 UTC (permalink / raw) To: FFmpeg development discussions and patches On date Friday 2024-01-05 17:42:47 +0100, Anton Khirnov wrote: > This avoids the requirement to always have a decoder context. > --- > fftools/ffmpeg_demux.c | 43 +++++++++++++++++++++--------------------- > 1 file changed, 21 insertions(+), 22 deletions(-) > > diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c > index cacdc76a71..892094c512 100644 > --- a/fftools/ffmpeg_demux.c > +++ b/fftools/ffmpeg_demux.c > @@ -899,19 +899,18 @@ static int choose_decoder(const OptionsContext *o, AVFormatContext *s, AVStream > } > } > > -static int guess_input_channel_layout(InputStream *ist, int guess_layout_max) > +static int guess_input_channel_layout(InputStream *ist, AVCodecParameters *par, > + int guess_layout_max) > { > - AVCodecContext *dec = ist->dec_ctx; > - > - if (dec->ch_layout.order == AV_CHANNEL_ORDER_UNSPEC) { > + if (par->ch_layout.order == AV_CHANNEL_ORDER_UNSPEC) { > char layout_name[256]; > > - if (dec->ch_layout.nb_channels > guess_layout_max) > + if (par->ch_layout.nb_channels > guess_layout_max) > return 0; > - av_channel_layout_default(&dec->ch_layout, dec->ch_layout.nb_channels); > - if (dec->ch_layout.order == AV_CHANNEL_ORDER_UNSPEC) > + av_channel_layout_default(&par->ch_layout, par->ch_layout.nb_channels); > + if (par->ch_layout.order == AV_CHANNEL_ORDER_UNSPEC) > return 0; > - av_channel_layout_describe(&dec->ch_layout, layout_name, sizeof(layout_name)); > + av_channel_layout_describe(&par->ch_layout, layout_name, sizeof(layout_name)); > av_log(ist, AV_LOG_WARNING, "Guessed Channel Layout: %s\n", layout_name); > } > return 1; > @@ -1145,16 +1144,6 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st) > ist->user_set_discard = ist->st->discard; > } > > - ist->dec_ctx = avcodec_alloc_context3(ist->dec); > - if (!ist->dec_ctx) > - return AVERROR(ENOMEM); > - > - ret = avcodec_parameters_to_context(ist->dec_ctx, par); > - if (ret < 0) { > - av_log(ist, AV_LOG_ERROR, "Error initializing the decoder context.\n"); > - return ret; > - } > - > if (o->bitexact) > av_dict_set(&ist->decoder_opts, "flags", "+bitexact", AV_DICT_MULTIKEY); > > @@ -1181,7 +1170,7 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st) > case AVMEDIA_TYPE_AUDIO: { > int guess_layout_max = INT_MAX; > MATCH_PER_STREAM_OPT(guess_layout_max, i, guess_layout_max, ic, st); > - guess_input_channel_layout(ist, guess_layout_max); > + guess_input_channel_layout(ist, par, guess_layout_max); > break; > } > case AVMEDIA_TYPE_DATA: > @@ -1190,7 +1179,7 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st) > MATCH_PER_STREAM_OPT(fix_sub_duration, i, ist->fix_sub_duration, ic, st); > MATCH_PER_STREAM_OPT(canvas_sizes, str, canvas_size, ic, st); > if (canvas_size) { > - ret = av_parse_video_size(&ist->dec_ctx->width, &ist->dec_ctx->height, > + ret = av_parse_video_size(&par->width, &par->height, > canvas_size); > if (ret < 0) { > av_log(ist, AV_LOG_FATAL, "Invalid canvas size: %s.\n", canvas_size); > @@ -1201,8 +1190,8 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st) > /* Compute the size of the canvas for the subtitles stream. > If the subtitles codecpar has set a size, use it. Otherwise use the > maximum dimensions of the video streams in the same file. */ > - ist->sub2video.w = ist->dec_ctx->width; > - ist->sub2video.h = ist->dec_ctx->height; > + ist->sub2video.w = par->width; > + ist->sub2video.h = par->height; > if (!(ist->sub2video.w && ist->sub2video.h)) { > for (int j = 0; j < ic->nb_streams; j++) { > AVCodecParameters *par1 = ic->streams[j]->codecpar; > @@ -1226,6 +1215,16 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st) > default: av_assert0(0); > } > > + ist->dec_ctx = avcodec_alloc_context3(ist->dec); > + if (!ist->dec_ctx) > + return AVERROR(ENOMEM); > + > + ret = avcodec_parameters_to_context(ist->dec_ctx, par); > + if (ret < 0) { > + av_log(ist, AV_LOG_ERROR, "Error initializing the decoder context.\n"); > + return ret; > + } > + LGTM. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [FFmpeg-devel] [PATCH 4/8] fftools/ffmpeg_demux: set options on codec parameters rather than decoder 2024-01-05 16:42 ` [FFmpeg-devel] [PATCH 4/8] fftools/ffmpeg_demux: set options on codec parameters rather than decoder Anton Khirnov 2024-01-06 11:31 ` Stefano Sabatini @ 2024-01-06 14:22 ` James Almer 2024-01-16 19:49 ` Anton Khirnov 1 sibling, 1 reply; 28+ messages in thread From: James Almer @ 2024-01-06 14:22 UTC (permalink / raw) To: ffmpeg-devel On 1/5/2024 1:42 PM, Anton Khirnov wrote: > This avoids the requirement to always have a decoder context. > --- > fftools/ffmpeg_demux.c | 43 +++++++++++++++++++++--------------------- > 1 file changed, 21 insertions(+), 22 deletions(-) > > diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c > index cacdc76a71..892094c512 100644 > --- a/fftools/ffmpeg_demux.c > +++ b/fftools/ffmpeg_demux.c > @@ -899,19 +899,18 @@ static int choose_decoder(const OptionsContext *o, AVFormatContext *s, AVStream > } > } > > -static int guess_input_channel_layout(InputStream *ist, int guess_layout_max) > +static int guess_input_channel_layout(InputStream *ist, AVCodecParameters *par, > + int guess_layout_max) > { > - AVCodecContext *dec = ist->dec_ctx; > - > - if (dec->ch_layout.order == AV_CHANNEL_ORDER_UNSPEC) { > + if (par->ch_layout.order == AV_CHANNEL_ORDER_UNSPEC) { > char layout_name[256]; > > - if (dec->ch_layout.nb_channels > guess_layout_max) > + if (par->ch_layout.nb_channels > guess_layout_max) > return 0; > - av_channel_layout_default(&dec->ch_layout, dec->ch_layout.nb_channels); > - if (dec->ch_layout.order == AV_CHANNEL_ORDER_UNSPEC) > + av_channel_layout_default(&par->ch_layout, par->ch_layout.nb_channels); > + if (par->ch_layout.order == AV_CHANNEL_ORDER_UNSPEC) > return 0; > - av_channel_layout_describe(&dec->ch_layout, layout_name, sizeof(layout_name)); > + av_channel_layout_describe(&par->ch_layout, layout_name, sizeof(layout_name)); > av_log(ist, AV_LOG_WARNING, "Guessed Channel Layout: %s\n", layout_name); > } > return 1; > @@ -1145,16 +1144,6 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st) > ist->user_set_discard = ist->st->discard; > } > > - ist->dec_ctx = avcodec_alloc_context3(ist->dec); > - if (!ist->dec_ctx) > - return AVERROR(ENOMEM); > - > - ret = avcodec_parameters_to_context(ist->dec_ctx, par); > - if (ret < 0) { > - av_log(ist, AV_LOG_ERROR, "Error initializing the decoder context.\n"); > - return ret; > - } > - > if (o->bitexact) > av_dict_set(&ist->decoder_opts, "flags", "+bitexact", AV_DICT_MULTIKEY); > > @@ -1181,7 +1170,7 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st) > case AVMEDIA_TYPE_AUDIO: { > int guess_layout_max = INT_MAX; > MATCH_PER_STREAM_OPT(guess_layout_max, i, guess_layout_max, ic, st); > - guess_input_channel_layout(ist, guess_layout_max); > + guess_input_channel_layout(ist, par, guess_layout_max); > break; > } > case AVMEDIA_TYPE_DATA: > @@ -1190,7 +1179,7 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st) > MATCH_PER_STREAM_OPT(fix_sub_duration, i, ist->fix_sub_duration, ic, st); > MATCH_PER_STREAM_OPT(canvas_sizes, str, canvas_size, ic, st); > if (canvas_size) { > - ret = av_parse_video_size(&ist->dec_ctx->width, &ist->dec_ctx->height, > + ret = av_parse_video_size(&par->width, &par->height, > canvas_size); > if (ret < 0) { > av_log(ist, AV_LOG_FATAL, "Invalid canvas size: %s.\n", canvas_size); > @@ -1201,8 +1190,8 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st) > /* Compute the size of the canvas for the subtitles stream. > If the subtitles codecpar has set a size, use it. Otherwise use the > maximum dimensions of the video streams in the same file. */ > - ist->sub2video.w = ist->dec_ctx->width; > - ist->sub2video.h = ist->dec_ctx->height; > + ist->sub2video.w = par->width; > + ist->sub2video.h = par->height; > if (!(ist->sub2video.w && ist->sub2video.h)) { > for (int j = 0; j < ic->nb_streams; j++) { > AVCodecParameters *par1 = ic->streams[j]->codecpar; > @@ -1226,6 +1215,16 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st) > default: av_assert0(0); > } > > + ist->dec_ctx = avcodec_alloc_context3(ist->dec); > + if (!ist->dec_ctx) > + return AVERROR(ENOMEM); > + > + ret = avcodec_parameters_to_context(ist->dec_ctx, par); > + if (ret < 0) { > + av_log(ist, AV_LOG_ERROR, "Error initializing the decoder context.\n"); > + return ret; > + } > + > ist->par = avcodec_parameters_alloc(); > if (!ist->par) > return AVERROR(ENOMEM); This does not fix the issue of the guessed channel layout not being present on the output stream, but might be a change in the right direction. before: > $ ./ffmpeg -i ~/samples/wav/200828-005.wav out.wav > [aist#0:0/pcm_s16le @ 00000252b1d2d420] Guessed Channel Layout: stereo > Input #0, wav, from '../samples/wav/200828-005.wav': > Duration: 00:00:12.80, bitrate: 1536 kb/s > Stream #0:0: Audio: pcm_s16le ([1][0][0][0] / 0x0001), 48000 Hz, 2 channels, s16, 1536 kb/s > Stream mapping: > Stream #0:0 -> #0:0 (pcm_s16le (native) -> pcm_s16le (native)) > Press [q] to stop, [?] for help > Output #0, wav, to 'out.wav': > Stream #0:0: Audio: pcm_s16le ([1][0][0][0] / 0x0001), 48000 Hz, stereo, s16, 1536 kb/s > > > $ ./ffmpeg -i out.wav > [aist#0:0/pcm_s16le @ 000001d616b1d3e0] Guessed Channel Layout: stereo > Duration: 00:00:12.80, bitrate: 1536 kb/s > Stream #0:0: Audio: pcm_s16le ([1][0][0][0] / 0x0001), 48000 Hz, 2 channels, s16, 1536 kb/s After: > $ ./ffmpeg -i ../samples/wav/200828-005.wav out.wav > [aist#0:0/pcm_s16le @ 0000024603c9d420] Guessed Channel Layout: stereo > Input #0, wav, from '../samples/wav/200828-005.wav': > Duration: 00:00:12.80, bitrate: 1536 kb/ > Stream #0:0: Audio: pcm_s16le ([1][0][0][0] / 0x0001), 48000 Hz, stereo, s16, 1536 kb/s > Stream mapping: > Stream #0:0 -> #0:0 (pcm_s16le (native) -> pcm_s16le (native)) > Press [q] to stop, [?] for help > Output #0, wav, to 'out.wav': > Stream #0:0: Audio: pcm_s16le ([1][0][0][0] / 0x0001), 48000 Hz, stereo, s16, 1536 kb/s > > $ ./ffmpeg -i out.wav > [aist#0:0/pcm_s16le @ 000001a1467ed3e0] Guessed Channel Layout: stereo > Duration: 00:00:12.80, bitrate: 1536 kb/s > Stream #0:0: Audio: pcm_s16le ([1][0][0][0] / 0x0001), 48000 Hz, stereo, s16, 1536 kb/s So the printed output shows the guessed layout for the input stream, but the written output stream is still unspec. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [FFmpeg-devel] [PATCH 4/8] fftools/ffmpeg_demux: set options on codec parameters rather than decoder 2024-01-06 14:22 ` James Almer @ 2024-01-16 19:49 ` Anton Khirnov 2024-01-16 22:37 ` James Almer 0 siblings, 1 reply; 28+ messages in thread From: Anton Khirnov @ 2024-01-16 19:49 UTC (permalink / raw) To: FFmpeg development discussions and patches Quoting James Almer (2024-01-06 15:22:10) > This does not fix the issue of the guessed channel layout not being > present on the output stream, Well, it wasn't supposed to fix anything. This patch exists as a prerequisite for the next one. -- Anton Khirnov _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [FFmpeg-devel] [PATCH 4/8] fftools/ffmpeg_demux: set options on codec parameters rather than decoder 2024-01-16 19:49 ` Anton Khirnov @ 2024-01-16 22:37 ` James Almer 0 siblings, 0 replies; 28+ messages in thread From: James Almer @ 2024-01-16 22:37 UTC (permalink / raw) To: ffmpeg-devel On 1/16/2024 4:49 PM, Anton Khirnov wrote: > Quoting James Almer (2024-01-06 15:22:10) >> This does not fix the issue of the guessed channel layout not being >> present on the output stream, > > Well, it wasn't supposed to fix anything. This patch exists as a > prerequisite for the next one. I know, hence the "but might be a change in the right direction." part followed by the changed output you removed from the quote :p _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 28+ messages in thread
* [FFmpeg-devel] [PATCH 5/8] fftools/ffmpeg_demux: only allocate the decoder when actually decoding 2024-01-05 16:42 [FFmpeg-devel] [PATCH 1/8] fftools/ffmpeg_demux: replace abort() by av_assert0(0) Anton Khirnov ` (2 preceding siblings ...) 2024-01-05 16:42 ` [FFmpeg-devel] [PATCH 4/8] fftools/ffmpeg_demux: set options on codec parameters rather than decoder Anton Khirnov @ 2024-01-05 16:42 ` Anton Khirnov 2024-01-06 11:34 ` Stefano Sabatini 2024-01-05 16:42 ` [FFmpeg-devel] [PATCH 6/8] fftools/ffmpeg_demux: add demuxing thread private data Anton Khirnov ` (3 subsequent siblings) 7 siblings, 1 reply; 28+ messages in thread From: Anton Khirnov @ 2024-01-05 16:42 UTC (permalink / raw) To: ffmpeg-devel It is not needed otherwise. --- fftools/ffmpeg_demux.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c index 892094c512..c51140b1c5 100644 --- a/fftools/ffmpeg_demux.c +++ b/fftools/ffmpeg_demux.c @@ -782,6 +782,16 @@ static int ist_use(InputStream *ist, int decoding_needed) if (decoding_needed && ds->sch_idx_dec < 0) { int is_audio = ist->st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO; + ist->dec_ctx = avcodec_alloc_context3(ist->dec); + if (!ist->dec_ctx) + return AVERROR(ENOMEM); + + ret = avcodec_parameters_to_context(ist->dec_ctx, ist->par); + if (ret < 0) { + av_log(ist, AV_LOG_ERROR, "Error initializing the decoder context.\n"); + return ret; + } + ret = sch_add_dec(d->sch, decoder_thread, ist, d->loop && is_audio); if (ret < 0) return ret; @@ -1215,23 +1225,13 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st) default: av_assert0(0); } - ist->dec_ctx = avcodec_alloc_context3(ist->dec); - if (!ist->dec_ctx) - return AVERROR(ENOMEM); - - ret = avcodec_parameters_to_context(ist->dec_ctx, par); - if (ret < 0) { - av_log(ist, AV_LOG_ERROR, "Error initializing the decoder context.\n"); - return ret; - } - ist->par = avcodec_parameters_alloc(); if (!ist->par) return AVERROR(ENOMEM); - ret = avcodec_parameters_from_context(ist->par, ist->dec_ctx); + ret = avcodec_parameters_copy(ist->par, par); if (ret < 0) { - av_log(ist, AV_LOG_ERROR, "Error initializing the decoder context.\n"); + av_log(ist, AV_LOG_ERROR, "Error exporting stream parameters.\n"); return ret; } -- 2.42.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-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [FFmpeg-devel] [PATCH 5/8] fftools/ffmpeg_demux: only allocate the decoder when actually decoding 2024-01-05 16:42 ` [FFmpeg-devel] [PATCH 5/8] fftools/ffmpeg_demux: only allocate the decoder when actually decoding Anton Khirnov @ 2024-01-06 11:34 ` Stefano Sabatini 2024-01-16 19:50 ` Anton Khirnov 0 siblings, 1 reply; 28+ messages in thread From: Stefano Sabatini @ 2024-01-06 11:34 UTC (permalink / raw) To: FFmpeg development discussions and patches On date Friday 2024-01-05 17:42:48 +0100, Anton Khirnov wrote: > It is not needed otherwise. > --- > fftools/ffmpeg_demux.c | 24 ++++++++++++------------ > 1 file changed, 12 insertions(+), 12 deletions(-) > > diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c > index 892094c512..c51140b1c5 100644 > --- a/fftools/ffmpeg_demux.c > +++ b/fftools/ffmpeg_demux.c > @@ -782,6 +782,16 @@ static int ist_use(InputStream *ist, int decoding_needed) > if (decoding_needed && ds->sch_idx_dec < 0) { > int is_audio = ist->st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO; > > + ist->dec_ctx = avcodec_alloc_context3(ist->dec); > + if (!ist->dec_ctx) > + return AVERROR(ENOMEM); > + > + ret = avcodec_parameters_to_context(ist->dec_ctx, ist->par); > + if (ret < 0) { > + av_log(ist, AV_LOG_ERROR, "Error initializing the decoder context.\n"); unrelated, here and below: might be useful to notify the failing stream identifier, assuming it is not already shown in the log > + return ret; > + } > + > ret = sch_add_dec(d->sch, decoder_thread, ist, d->loop && is_audio); > if (ret < 0) > return ret; > @@ -1215,23 +1225,13 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st) > default: av_assert0(0); > } > > - ist->dec_ctx = avcodec_alloc_context3(ist->dec); > - if (!ist->dec_ctx) > - return AVERROR(ENOMEM); > - > - ret = avcodec_parameters_to_context(ist->dec_ctx, par); > - if (ret < 0) { > - av_log(ist, AV_LOG_ERROR, "Error initializing the decoder context.\n"); > - return ret; > - } > - > ist->par = avcodec_parameters_alloc(); > if (!ist->par) > return AVERROR(ENOMEM); > > - ret = avcodec_parameters_from_context(ist->par, ist->dec_ctx); > + ret = avcodec_parameters_copy(ist->par, par); > if (ret < 0) { > - av_log(ist, AV_LOG_ERROR, "Error initializing the decoder context.\n"); > + av_log(ist, AV_LOG_ERROR, "Error exporting stream parameters.\n"); > return ret; > } LGTM anyway. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [FFmpeg-devel] [PATCH 5/8] fftools/ffmpeg_demux: only allocate the decoder when actually decoding 2024-01-06 11:34 ` Stefano Sabatini @ 2024-01-16 19:50 ` Anton Khirnov 0 siblings, 0 replies; 28+ messages in thread From: Anton Khirnov @ 2024-01-16 19:50 UTC (permalink / raw) To: FFmpeg development discussions and patches Quoting Stefano Sabatini (2024-01-06 12:34:53) > On date Friday 2024-01-05 17:42:48 +0100, Anton Khirnov wrote: > > It is not needed otherwise. > > --- > > fftools/ffmpeg_demux.c | 24 ++++++++++++------------ > > 1 file changed, 12 insertions(+), 12 deletions(-) > > > > diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c > > index 892094c512..c51140b1c5 100644 > > --- a/fftools/ffmpeg_demux.c > > +++ b/fftools/ffmpeg_demux.c > > @@ -782,6 +782,16 @@ static int ist_use(InputStream *ist, int decoding_needed) > > if (decoding_needed && ds->sch_idx_dec < 0) { > > int is_audio = ist->st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO; > > > > + ist->dec_ctx = avcodec_alloc_context3(ist->dec); > > + if (!ist->dec_ctx) > > + return AVERROR(ENOMEM); > > + > > + ret = avcodec_parameters_to_context(ist->dec_ctx, ist->par); > > + if (ret < 0) { > > > + av_log(ist, AV_LOG_ERROR, "Error initializing the decoder context.\n"); > > unrelated, here and below: might be useful to notify the failing > stream identifier, assuming it is not already shown in the log It is, logging to ist prints something like [aist#0:0/pcm_s16le @ 0x563a40304b40] .... -- Anton Khirnov _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 28+ messages in thread
* [FFmpeg-devel] [PATCH 6/8] fftools/ffmpeg_demux: add demuxing thread private data 2024-01-05 16:42 [FFmpeg-devel] [PATCH 1/8] fftools/ffmpeg_demux: replace abort() by av_assert0(0) Anton Khirnov ` (3 preceding siblings ...) 2024-01-05 16:42 ` [FFmpeg-devel] [PATCH 5/8] fftools/ffmpeg_demux: only allocate the decoder when actually decoding Anton Khirnov @ 2024-01-05 16:42 ` Anton Khirnov 2024-01-06 11:44 ` Stefano Sabatini 2024-01-05 16:42 ` [FFmpeg-devel] [PATCH 7/8] fftools/ffmpeg_demux: implement -bsf for input Anton Khirnov ` (2 subsequent siblings) 7 siblings, 1 reply; 28+ messages in thread From: Anton Khirnov @ 2024-01-05 16:42 UTC (permalink / raw) To: ffmpeg-devel To be used for data that never needs to be visible outside of the demuxer thread, similarly as was previously done for other components. --- fftools/ffmpeg_demux.c | 67 ++++++++++++++++++++++++++++-------------- 1 file changed, 45 insertions(+), 22 deletions(-) diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c index c51140b1c5..eae1f0bde5 100644 --- a/fftools/ffmpeg_demux.c +++ b/fftools/ffmpeg_demux.c @@ -115,6 +115,11 @@ typedef struct Demuxer { int nb_streams_finished; } Demuxer; +typedef struct DemuxThreadContext { + // packet used for reading from the demuxer + AVPacket *pkt_demux; +} DemuxThreadContext; + static DemuxStream *ds_from_ist(InputStream *ist) { return (DemuxStream*)ist; @@ -565,18 +570,36 @@ static void thread_set_name(InputFile *f) ff_thread_setname(name); } +static void demux_thread_uninit(DemuxThreadContext *dt) +{ + av_packet_free(&dt->pkt_demux); + + memset(dt, 0, sizeof(*dt)); +} + +static int demux_thread_init(DemuxThreadContext *dt) +{ + memset(dt, 0, sizeof(*dt)); + + dt->pkt_demux = av_packet_alloc(); + if (!dt->pkt_demux) + return AVERROR(ENOMEM); + + return 0; +} + static void *input_thread(void *arg) { Demuxer *d = arg; InputFile *f = &d->f; - AVPacket *pkt; + + DemuxThreadContext dt; + int ret = 0; - pkt = av_packet_alloc(); - if (!pkt) { - ret = AVERROR(ENOMEM); + ret = demux_thread_init(&dt); + if (ret < 0) goto finish; - } thread_set_name(f); @@ -589,7 +612,7 @@ static void *input_thread(void *arg) DemuxStream *ds; unsigned send_flags = 0; - ret = av_read_frame(f->ctx, pkt); + ret = av_read_frame(f->ctx, dt.pkt_demux); if (ret == AVERROR(EAGAIN)) { av_usleep(10000); @@ -598,12 +621,12 @@ static void *input_thread(void *arg) if (ret < 0) { if (d->loop) { /* signal looping to our consumers */ - pkt->stream_index = -1; + dt.pkt_demux->stream_index = -1; - ret = sch_demux_send(d->sch, f->index, pkt, 0); + ret = sch_demux_send(d->sch, f->index, dt.pkt_demux, 0); if (ret >= 0) - ret = seek_to_start(d, (Timestamp){ .ts = pkt->pts, - .tb = pkt->time_base }); + ret = seek_to_start(d, (Timestamp){ .ts = dt.pkt_demux->pts, + .tb = dt.pkt_demux->time_base }); if (ret >= 0) continue; @@ -622,39 +645,39 @@ static void *input_thread(void *arg) } if (do_pkt_dump) { - av_pkt_dump_log2(NULL, AV_LOG_INFO, pkt, do_hex_dump, - f->ctx->streams[pkt->stream_index]); + av_pkt_dump_log2(NULL, AV_LOG_INFO, dt.pkt_demux, do_hex_dump, + f->ctx->streams[dt.pkt_demux->stream_index]); } /* the following test is needed in case new streams appear dynamically in stream : we ignore them */ - ds = pkt->stream_index < f->nb_streams ? - ds_from_ist(f->streams[pkt->stream_index]) : NULL; + ds = dt.pkt_demux->stream_index < f->nb_streams ? + ds_from_ist(f->streams[dt.pkt_demux->stream_index]) : NULL; if (!ds || ds->discard || ds->finished) { - report_new_stream(d, pkt); - av_packet_unref(pkt); + report_new_stream(d, dt.pkt_demux); + av_packet_unref(dt.pkt_demux); continue; } - if (pkt->flags & AV_PKT_FLAG_CORRUPT) { + if (dt.pkt_demux->flags & AV_PKT_FLAG_CORRUPT) { av_log(d, exit_on_error ? AV_LOG_FATAL : AV_LOG_WARNING, "corrupt input packet in stream %d\n", - pkt->stream_index); + dt.pkt_demux->stream_index); if (exit_on_error) { - av_packet_unref(pkt); + av_packet_unref(dt.pkt_demux); ret = AVERROR_INVALIDDATA; break; } } - ret = input_packet_process(d, pkt, &send_flags); + ret = input_packet_process(d, dt.pkt_demux, &send_flags); if (ret < 0) break; if (d->readrate) readrate_sleep(d); - ret = demux_send(d, ds, pkt, send_flags); + ret = demux_send(d, ds, dt.pkt_demux, send_flags); if (ret < 0) break; } @@ -664,7 +687,7 @@ static void *input_thread(void *arg) ret = 0; finish: - av_packet_free(&pkt); + demux_thread_uninit(&dt); return (void*)(intptr_t)ret; } -- 2.42.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-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [FFmpeg-devel] [PATCH 6/8] fftools/ffmpeg_demux: add demuxing thread private data 2024-01-05 16:42 ` [FFmpeg-devel] [PATCH 6/8] fftools/ffmpeg_demux: add demuxing thread private data Anton Khirnov @ 2024-01-06 11:44 ` Stefano Sabatini 2024-01-16 19:52 ` Anton Khirnov 0 siblings, 1 reply; 28+ messages in thread From: Stefano Sabatini @ 2024-01-06 11:44 UTC (permalink / raw) To: FFmpeg development discussions and patches On date Friday 2024-01-05 17:42:49 +0100, Anton Khirnov wrote: > To be used for data that never needs to be visible outside of the > demuxer thread, similarly as was previously done for other components. > --- > fftools/ffmpeg_demux.c | 67 ++++++++++++++++++++++++++++-------------- > 1 file changed, 45 insertions(+), 22 deletions(-) > > diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c > index c51140b1c5..eae1f0bde5 100644 > --- a/fftools/ffmpeg_demux.c > +++ b/fftools/ffmpeg_demux.c > @@ -115,6 +115,11 @@ typedef struct Demuxer { > int nb_streams_finished; > } Demuxer; > > +typedef struct DemuxThreadContext { > + // packet used for reading from the demuxer > + AVPacket *pkt_demux; nit: you might drop the _demux suffix since this is already clear from the context and it's adding no information > +} DemuxThreadContext; > + > static DemuxStream *ds_from_ist(InputStream *ist) > { > return (DemuxStream*)ist; > @@ -565,18 +570,36 @@ static void thread_set_name(InputFile *f) > ff_thread_setname(name); > } > > +static void demux_thread_uninit(DemuxThreadContext *dt) > +{ > + av_packet_free(&dt->pkt_demux); > + > + memset(dt, 0, sizeof(*dt)); > +} > + > +static int demux_thread_init(DemuxThreadContext *dt) > +{ > + memset(dt, 0, sizeof(*dt)); > + > + dt->pkt_demux = av_packet_alloc(); > + if (!dt->pkt_demux) > + return AVERROR(ENOMEM); > + > + return 0; > +} > + > static void *input_thread(void *arg) > { > Demuxer *d = arg; > InputFile *f = &d->f; > - AVPacket *pkt; > + > + DemuxThreadContext dt; > + nit++: weird style, you might drop the empty lines around the declaration > int ret = 0; > > - pkt = av_packet_alloc(); > - if (!pkt) { > - ret = AVERROR(ENOMEM); > + ret = demux_thread_init(&dt); > + if (ret < 0) > goto finish; > - } > > thread_set_name(f); > > @@ -589,7 +612,7 @@ static void *input_thread(void *arg) > DemuxStream *ds; > unsigned send_flags = 0; > > - ret = av_read_frame(f->ctx, pkt); > + ret = av_read_frame(f->ctx, dt.pkt_demux); > > if (ret == AVERROR(EAGAIN)) { > av_usleep(10000); > @@ -598,12 +621,12 @@ static void *input_thread(void *arg) > if (ret < 0) { > if (d->loop) { > /* signal looping to our consumers */ > - pkt->stream_index = -1; > + dt.pkt_demux->stream_index = -1; > > - ret = sch_demux_send(d->sch, f->index, pkt, 0); > + ret = sch_demux_send(d->sch, f->index, dt.pkt_demux, 0); > if (ret >= 0) > - ret = seek_to_start(d, (Timestamp){ .ts = pkt->pts, > - .tb = pkt->time_base }); > + ret = seek_to_start(d, (Timestamp){ .ts = dt.pkt_demux->pts, > + .tb = dt.pkt_demux->time_base }); > if (ret >= 0) > continue; > > @@ -622,39 +645,39 @@ static void *input_thread(void *arg) > } > > if (do_pkt_dump) { > - av_pkt_dump_log2(NULL, AV_LOG_INFO, pkt, do_hex_dump, > - f->ctx->streams[pkt->stream_index]); > + av_pkt_dump_log2(NULL, AV_LOG_INFO, dt.pkt_demux, do_hex_dump, > + f->ctx->streams[dt.pkt_demux->stream_index]); > } > > /* the following test is needed in case new streams appear > dynamically in stream : we ignore them */ > - ds = pkt->stream_index < f->nb_streams ? > - ds_from_ist(f->streams[pkt->stream_index]) : NULL; > + ds = dt.pkt_demux->stream_index < f->nb_streams ? > + ds_from_ist(f->streams[dt.pkt_demux->stream_index]) : NULL; > if (!ds || ds->discard || ds->finished) { > - report_new_stream(d, pkt); > - av_packet_unref(pkt); > + report_new_stream(d, dt.pkt_demux); > + av_packet_unref(dt.pkt_demux); > continue; > } > > - if (pkt->flags & AV_PKT_FLAG_CORRUPT) { > + if (dt.pkt_demux->flags & AV_PKT_FLAG_CORRUPT) { > av_log(d, exit_on_error ? AV_LOG_FATAL : AV_LOG_WARNING, > "corrupt input packet in stream %d\n", > - pkt->stream_index); > + dt.pkt_demux->stream_index); > if (exit_on_error) { > - av_packet_unref(pkt); > + av_packet_unref(dt.pkt_demux); > ret = AVERROR_INVALIDDATA; > break; > } > } > > - ret = input_packet_process(d, pkt, &send_flags); > + ret = input_packet_process(d, dt.pkt_demux, &send_flags); > if (ret < 0) > break; > > if (d->readrate) > readrate_sleep(d); > > - ret = demux_send(d, ds, pkt, send_flags); > + ret = demux_send(d, ds, dt.pkt_demux, send_flags); > if (ret < 0) > break; > } > @@ -664,7 +687,7 @@ static void *input_thread(void *arg) > ret = 0; > > finish: > - av_packet_free(&pkt); > + demux_thread_uninit(&dt); LGTM if useful for the following changes. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [FFmpeg-devel] [PATCH 6/8] fftools/ffmpeg_demux: add demuxing thread private data 2024-01-06 11:44 ` Stefano Sabatini @ 2024-01-16 19:52 ` Anton Khirnov 0 siblings, 0 replies; 28+ messages in thread From: Anton Khirnov @ 2024-01-16 19:52 UTC (permalink / raw) To: FFmpeg development discussions and patches Quoting Stefano Sabatini (2024-01-06 12:44:27) > On date Friday 2024-01-05 17:42:49 +0100, Anton Khirnov wrote: > > To be used for data that never needs to be visible outside of the > > demuxer thread, similarly as was previously done for other components. > > --- > > fftools/ffmpeg_demux.c | 67 ++++++++++++++++++++++++++++-------------- > > 1 file changed, 45 insertions(+), 22 deletions(-) > > > > diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c > > index c51140b1c5..eae1f0bde5 100644 > > --- a/fftools/ffmpeg_demux.c > > +++ b/fftools/ffmpeg_demux.c > > @@ -115,6 +115,11 @@ typedef struct Demuxer { > > int nb_streams_finished; > > } Demuxer; > > > > > +typedef struct DemuxThreadContext { > > + // packet used for reading from the demuxer > > > + AVPacket *pkt_demux; > > nit: you might drop the _demux suffix since this is already clear from > the context and it's adding no information I'm adding the suffix because a following commit adds another packet to the struct, so I prefer to make it clear what each one's role is. > > static void *input_thread(void *arg) > > { > > Demuxer *d = arg; > > InputFile *f = &d->f; > > - AVPacket *pkt; > > + > > + DemuxThreadContext dt; > > + > > nit++: weird style, you might drop the empty lines around the > declaration I prefer it this way. -- Anton Khirnov _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 28+ messages in thread
* [FFmpeg-devel] [PATCH 7/8] fftools/ffmpeg_demux: implement -bsf for input 2024-01-05 16:42 [FFmpeg-devel] [PATCH 1/8] fftools/ffmpeg_demux: replace abort() by av_assert0(0) Anton Khirnov ` (4 preceding siblings ...) 2024-01-05 16:42 ` [FFmpeg-devel] [PATCH 6/8] fftools/ffmpeg_demux: add demuxing thread private data Anton Khirnov @ 2024-01-05 16:42 ` Anton Khirnov 2024-01-06 12:12 ` Stefano Sabatini 2024-01-05 16:42 ` [FFmpeg-devel] [PATCH 8/8] fftools/ffmpeg_opt: mark -stream_group as expert option Anton Khirnov 2024-01-06 11:18 ` [FFmpeg-devel] [PATCH 1/8] fftools/ffmpeg_demux: replace abort() by av_assert0(0) Stefano Sabatini 7 siblings, 1 reply; 28+ messages in thread From: Anton Khirnov @ 2024-01-05 16:42 UTC (permalink / raw) To: ffmpeg-devel Previously bitstream filters could only be applied right before muxing, this allows to apply them right after demuxing. --- Changelog | 1 + doc/ffmpeg.texi | 22 +++-- fftools/ffmpeg_demux.c | 139 ++++++++++++++++++++++++++++---- fftools/ffmpeg_opt.c | 2 +- tests/fate/ffmpeg.mak | 5 ++ tests/ref/fate/ffmpeg-bsf-input | 18 +++++ 6 files changed, 164 insertions(+), 23 deletions(-) create mode 100644 tests/ref/fate/ffmpeg-bsf-input diff --git a/Changelog b/Changelog index 5b2899d05b..f8191d88c7 100644 --- a/Changelog +++ b/Changelog @@ -18,6 +18,7 @@ version <next>: - lavu/eval: introduce randomi() function in expressions - VVC decoder - fsync filter +- ffmpeg CLI -bsf option may now be used for input as well as output version 6.1: - libaribcaption decoder diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi index d75517b443..59f7badcb6 100644 --- a/doc/ffmpeg.texi +++ b/doc/ffmpeg.texi @@ -2093,26 +2093,34 @@ an output mpegts file: ffmpeg -i inurl -streamid 0:33 -streamid 1:36 out.ts @end example -@item -bsf[:@var{stream_specifier}] @var{bitstream_filters} (@emph{output,per-stream}) -Apply bitstream filters to matching streams. +@item -bsf[:@var{stream_specifier}] @var{bitstream_filters} (@emph{input/output,per-stream}) +Apply bitstream filters to matching streams. The filters are applied to each +packet as it is received from the demuxer (when used as an input option) or +before it is sent to the muxer (when used as an output option). @var{bitstream_filters} is a comma-separated list of bitstream filter -specifications. The specified bitstream filters are applied to coded packets in -the order they are written in. Each bitstream filter specification is of the -form +specifications, each of the form @example @var{filter}[=@var{optname0}=@var{optval0}:@var{optname1}=@var{optval1}:...] @end example Any of the ',=:' characters that are to be a part of an option value need to be escaped with a backslash. -Use the @code{-bsfs} option to get the list of bitstream filters. +Use the @code{-bsfs} option to get the list of bitstream filters. E.g. @example -ffmpeg -i h264.mp4 -c:v copy -bsf:v h264_mp4toannexb -an out.h264 +ffmpeg -bsf:v h264_mp4toannexb -i h264.mp4 -c:v copy -an out.h264 @end example +applies the @code{h264_mp4toannexb} bitstream filter (which converts +MP4-encapsulated H.264 stream to Annex B) to the @emph{input} video stream. + +On the other hand, @example ffmpeg -i file.mov -an -vn -bsf:s mov2textsub -c:s copy -f rawvideo sub.txt @end example +applies the @code{mov2textsub} bitstream filter (which extracts text from MOV +subtitles) to the @emph{output} subtitle stream. Note, however, that since both +examples use @code{-c copy}, it matters little whether the filters are applied +on input or output - that would change if transcoding was hapenning. @item -tag[:@var{stream_specifier}] @var{codec_tag} (@emph{input/output,per-stream}) Force a tag/fourcc for matching streams. diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c index eae1f0bde5..16d4f67e59 100644 --- a/fftools/ffmpeg_demux.c +++ b/fftools/ffmpeg_demux.c @@ -34,6 +34,7 @@ #include "libavutil/time.h" #include "libavutil/timestamp.h" +#include "libavcodec/bsf.h" #include "libavcodec/packet.h" #include "libavformat/avformat.h" @@ -71,6 +72,8 @@ typedef struct DemuxStream { const AVCodecDescriptor *codec_desc; + AVBSFContext *bsf; + /* number of packets successfully read for this stream */ uint64_t nb_packets; // combined size of all the packets read @@ -118,6 +121,8 @@ typedef struct Demuxer { typedef struct DemuxThreadContext { // packet used for reading from the demuxer AVPacket *pkt_demux; + // packet for reading from BSFs + AVPacket *pkt_bsf; } DemuxThreadContext; static DemuxStream *ds_from_ist(InputStream *ist) @@ -513,13 +518,17 @@ static int do_send(Demuxer *d, DemuxStream *ds, AVPacket *pkt, unsigned flags, return 0; } -static int demux_send(Demuxer *d, DemuxStream *ds, AVPacket *pkt, unsigned flags) +static int demux_send(Demuxer *d, DemuxThreadContext *dt, DemuxStream *ds, + AVPacket *pkt, unsigned flags) { InputFile *f = &d->f; int ret; + // pkt can be NULL only when flushing BSFs + av_assert0(ds->bsf || pkt); + // send heartbeat for sub2video streams - if (d->pkt_heartbeat && pkt->pts != AV_NOPTS_VALUE) { + if (d->pkt_heartbeat && pkt && pkt->pts != AV_NOPTS_VALUE) { for (int i = 0; i < f->nb_streams; i++) { DemuxStream *ds1 = ds_from_ist(f->streams[i]); @@ -537,10 +546,69 @@ static int demux_send(Demuxer *d, DemuxStream *ds, AVPacket *pkt, unsigned flags } } - ret = do_send(d, ds, pkt, flags, "demuxed"); - if (ret < 0) - return ret; + if (ds->bsf) { + if (pkt) + av_packet_rescale_ts(pkt, pkt->time_base, ds->bsf->time_base_in); + ret = av_bsf_send_packet(ds->bsf, pkt); + if (ret < 0) { + if (pkt) + av_packet_unref(pkt); + av_log(ds, AV_LOG_ERROR, "Error submitting a packet for filtering: %s\n", + av_err2str(ret)); + return ret; + } + + while (1) { + ret = av_bsf_receive_packet(ds->bsf, dt->pkt_bsf); + if (ret == AVERROR(EAGAIN)) + return 0; + else if (ret < 0) { + if (ret != AVERROR_EOF) + av_log(ds, AV_LOG_ERROR, + "Error applying bitstream filters to a packet: %s\n", + av_err2str(ret)); + return ret; + } + + dt->pkt_bsf->time_base = ds->bsf->time_base_out; + + ret = do_send(d, ds, dt->pkt_bsf, 0, "filtered"); + if (ret < 0) { + av_packet_unref(dt->pkt_bsf); + return ret; + } + } + } else { + ret = do_send(d, ds, pkt, flags, "demuxed"); + if (ret < 0) + return ret; + } + + return 0; +} + +static int demux_bsf_flush(Demuxer *d, DemuxThreadContext *dt) +{ + InputFile *f = &d->f; + int ret; + + for (unsigned i = 0; i < f->nb_streams; i++) { + DemuxStream *ds = ds_from_ist(f->streams[i]); + + if (!ds->bsf) + continue; + + ret = demux_send(d, dt, ds, NULL, 0); + ret = (ret == AVERROR_EOF) ? 0 : (ret < 0) ? ret : AVERROR_BUG; + if (ret < 0) { + av_log(ds, AV_LOG_ERROR, "Error flushing BSFs: %s\n", + av_err2str(ret)); + return ret; + } + + av_bsf_flush(ds->bsf); + } return 0; } @@ -573,6 +641,7 @@ static void thread_set_name(InputFile *f) static void demux_thread_uninit(DemuxThreadContext *dt) { av_packet_free(&dt->pkt_demux); + av_packet_free(&dt->pkt_bsf); memset(dt, 0, sizeof(*dt)); } @@ -585,6 +654,10 @@ static int demux_thread_init(DemuxThreadContext *dt) if (!dt->pkt_demux) return AVERROR(ENOMEM); + dt->pkt_bsf = av_packet_alloc(); + if (!dt->pkt_bsf) + return AVERROR(ENOMEM); + return 0; } @@ -619,10 +692,22 @@ static void *input_thread(void *arg) continue; } if (ret < 0) { + int ret_bsf; + + if (ret == AVERROR_EOF) + av_log(d, AV_LOG_VERBOSE, "EOF while reading input\n"); + else { + av_log(d, AV_LOG_ERROR, "Error during demuxing: %s\n", + av_err2str(ret)); + ret = exit_on_error ? ret : 0; + } + + ret_bsf = demux_bsf_flush(d, &dt); + ret = err_merge(ret == AVERROR_EOF ? 0 : ret, ret_bsf); + if (d->loop) { /* signal looping to our consumers */ dt.pkt_demux->stream_index = -1; - ret = sch_demux_send(d->sch, f->index, dt.pkt_demux, 0); if (ret >= 0) ret = seek_to_start(d, (Timestamp){ .ts = dt.pkt_demux->pts, @@ -633,14 +718,6 @@ static void *input_thread(void *arg) /* fallthrough to the error path */ } - if (ret == AVERROR_EOF) - av_log(d, AV_LOG_VERBOSE, "EOF while reading input\n"); - else { - av_log(d, AV_LOG_ERROR, "Error during demuxing: %s\n", - av_err2str(ret)); - ret = exit_on_error ? ret : 0; - } - break; } @@ -677,7 +754,7 @@ static void *input_thread(void *arg) if (d->readrate) readrate_sleep(d); - ret = demux_send(d, ds, dt.pkt_demux, send_flags); + ret = demux_send(d, &dt, ds, dt.pkt_demux, send_flags); if (ret < 0) break; } @@ -735,9 +812,11 @@ static void demux_final_stats(Demuxer *d) static void ist_free(InputStream **pist) { InputStream *ist = *pist; + DemuxStream *ds; if (!ist) return; + ds = ds_from_ist(ist); dec_free(&ist->decoder); @@ -749,6 +828,8 @@ static void ist_free(InputStream **pist) avcodec_free_context(&ist->dec_ctx); avcodec_parameters_free(&ist->par); + av_bsf_free(&ds->bsf); + av_freep(pist); } @@ -1039,6 +1120,7 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st) const char *hwaccel = NULL; char *hwaccel_output_format = NULL; char *codec_tag = NULL; + char *bsfs = NULL; char *next; char *discard_str = NULL; int ret; @@ -1258,6 +1340,33 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st) return ret; } + MATCH_PER_STREAM_OPT(bitstream_filters, str, bsfs, ic, st); + if (bsfs) { + ret = av_bsf_list_parse_str(bsfs, &ds->bsf); + if (ret < 0) { + av_log(ist, AV_LOG_ERROR, + "Error parsing bitstream filter sequence '%s': %s\n", + bsfs, av_err2str(ret)); + return ret; + } + + ret = avcodec_parameters_copy(ds->bsf->par_in, ist->par); + if (ret < 0) + return ret; + ds->bsf->time_base_in = ist->st->time_base; + + ret = av_bsf_init(ds->bsf); + if (ret < 0) { + av_log(ist, AV_LOG_ERROR, "Error initializing bitstream filters: %s\n", + av_err2str(ret)); + return ret; + } + + ret = avcodec_parameters_copy(ist->par, ds->bsf->par_out); + if (ret < 0) + return ret; + } + ds->codec_desc = avcodec_descriptor_get(ist->par->codec_id); return 0; diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c index c189cf373b..76b50c0bad 100644 --- a/fftools/ffmpeg_opt.c +++ b/fftools/ffmpeg_opt.c @@ -1919,7 +1919,7 @@ const OptionDef options[] = { "0 = use frame rate (video) or sample rate (audio)," "-1 = match source time base", "ratio" }, - { "bsf", OPT_TYPE_STRING, OPT_SPEC | OPT_EXPERT | OPT_OUTPUT, + { "bsf", OPT_TYPE_STRING, OPT_SPEC | OPT_EXPERT | OPT_OUTPUT | OPT_INPUT, { .off = OFFSET(bitstream_filters) }, "A comma-separated list of bitstream filters", "bitstream_filters", }, diff --git a/tests/fate/ffmpeg.mak b/tests/fate/ffmpeg.mak index 1bfd5c1b31..df955df4d0 100644 --- a/tests/fate/ffmpeg.mak +++ b/tests/fate/ffmpeg.mak @@ -256,3 +256,8 @@ FATE_SAMPLES_FFMPEG-$(call FRAMECRC, MPEGVIDEO, MPEG2VIDEO) += fate-ffmpeg-input fate-ffmpeg-error-rate-fail: CMD = ffmpeg -i $(TARGET_SAMPLES)/mkv/h264_tta_undecodable.mkv -c:v copy -f null -; test $$? -eq 69 fate-ffmpeg-error-rate-pass: CMD = ffmpeg -i $(TARGET_SAMPLES)/mkv/h264_tta_undecodable.mkv -c:v copy -f null - -max_error_rate 1 FATE_SAMPLES_FFMPEG-$(call ENCDEC, PCM_S16LE TTA, NULL MATROSKA) += fate-ffmpeg-error-rate-fail fate-ffmpeg-error-rate-pass + +# test input -bsf +# use -stream_loop, because it tests flushing bsfs +fate-ffmpeg-bsf-input: CMD = framecrc -stream_loop 2 -bsf setts=PTS*2 -i $(TARGET_SAMPLES)/hevc/extradata-reload-multi-stsd.mov -c copy +FATE_SAMPLES_FFMPEG-$(call FRAMECRC, MOV, , SETTS_BSF) += fate-ffmpeg-bsf-input diff --git a/tests/ref/fate/ffmpeg-bsf-input b/tests/ref/fate/ffmpeg-bsf-input new file mode 100644 index 0000000000..67dd57cf6d --- /dev/null +++ b/tests/ref/fate/ffmpeg-bsf-input @@ -0,0 +1,18 @@ +#extradata 0: 110, 0xb4031479 +#tb 0: 1/25 +#media_type 0: video +#codec_id 0: hevc +#dimensions 0: 128x128 +#sar 0: 1/1 +0, 0, 0, 1, 2108, 0x57c38f64 +0, 2, 2, 1, 31, 0xabe10d25, F=0x0 +0, 4, 4, 1, 1915, 0xd430347f, S=1, 109 +0, 6, 6, 1, 35, 0xc4ad0d4c, F=0x0 +0, 8, 8, 1, 2108, 0x57c38f64, S=1, 110 +0, 10, 10, 1, 31, 0xabe10d25, F=0x0 +0, 12, 12, 1, 1915, 0xd430347f, S=1, 109 +0, 14, 14, 1, 35, 0xc4ad0d4c, F=0x0 +0, 16, 16, 1, 2108, 0x57c38f64, S=1, 110 +0, 18, 18, 1, 31, 0xabe10d25, F=0x0 +0, 20, 20, 1, 1915, 0xd430347f, S=1, 109 +0, 22, 22, 1, 35, 0xc4ad0d4c, F=0x0 -- 2.42.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-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [FFmpeg-devel] [PATCH 7/8] fftools/ffmpeg_demux: implement -bsf for input 2024-01-05 16:42 ` [FFmpeg-devel] [PATCH 7/8] fftools/ffmpeg_demux: implement -bsf for input Anton Khirnov @ 2024-01-06 12:12 ` Stefano Sabatini 2024-01-17 9:02 ` Anton Khirnov 0 siblings, 1 reply; 28+ messages in thread From: Stefano Sabatini @ 2024-01-06 12:12 UTC (permalink / raw) To: FFmpeg development discussions and patches On date Friday 2024-01-05 17:42:50 +0100, Anton Khirnov wrote: > Previously bitstream filters could only be applied right before muxing, > this allows to apply them right after demuxing. > --- > Changelog | 1 + > doc/ffmpeg.texi | 22 +++-- > fftools/ffmpeg_demux.c | 139 ++++++++++++++++++++++++++++---- > fftools/ffmpeg_opt.c | 2 +- > tests/fate/ffmpeg.mak | 5 ++ > tests/ref/fate/ffmpeg-bsf-input | 18 +++++ > 6 files changed, 164 insertions(+), 23 deletions(-) > create mode 100644 tests/ref/fate/ffmpeg-bsf-input > > diff --git a/Changelog b/Changelog > index 5b2899d05b..f8191d88c7 100644 > --- a/Changelog > +++ b/Changelog > @@ -18,6 +18,7 @@ version <next>: > - lavu/eval: introduce randomi() function in expressions > - VVC decoder > - fsync filter > +- ffmpeg CLI -bsf option may now be used for input as well as output > > version 6.1: > - libaribcaption decoder > diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi > index d75517b443..59f7badcb6 100644 > --- a/doc/ffmpeg.texi > +++ b/doc/ffmpeg.texi > @@ -2093,26 +2093,34 @@ an output mpegts file: > ffmpeg -i inurl -streamid 0:33 -streamid 1:36 out.ts > @end example > > -@item -bsf[:@var{stream_specifier}] @var{bitstream_filters} (@emph{output,per-stream}) > -Apply bitstream filters to matching streams. > +@item -bsf[:@var{stream_specifier}] @var{bitstream_filters} (@emph{input/output,per-stream}) > +Apply bitstream filters to matching streams. The filters are applied to each > +packet as it is received from the demuxer (when used as an input option) or > +before it is sent to the muxer (when used as an output option). > > @var{bitstream_filters} is a comma-separated list of bitstream filter > -specifications. The specified bitstream filters are applied to coded packets in > -the order they are written in. Each bitstream filter specification is of the > -form > +specifications, each of the form > @example > @var{filter}[=@var{optname0}=@var{optval0}:@var{optname1}=@var{optval1}:...] > @end example > Any of the ',=:' characters that are to be a part of an option value need to be > escaped with a backslash. > > -Use the @code{-bsfs} option to get the list of bitstream filters. > +Use the @code{-bsfs} option to get the list of bitstream filters. E.g. This looks spurious, since this suggests the example is about the listing, and it's applying a weird order of example/explanation (rather than the opposite). What about something as: -------------------------- [...] Use the @code{-bsfs} option to get the list of bitstream filters. Some examples follow. To apply the @code{h264_mp4toannexb} bitstream filter (which converts MP4-encapsulated H.264 stream to Annex B) to the @emph{input} video stream: @example ... @end example To apply the @code{mov2textsub} bitstream filter (which extracts text from MOV subtitles) to the @emph{output} subtitle stream: @example ... @end example Note, however, that since both examples use @code{-c copy}, it matters little whether the filters are applied on input or output - that would change if transcoding was happening. -------------------------- [...] > +on input or output - that would change if transcoding was hapenning. hapenning typo > > @item -tag[:@var{stream_specifier}] @var{codec_tag} (@emph{input/output,per-stream}) > Force a tag/fourcc for matching streams. > diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c > index eae1f0bde5..16d4f67e59 100644 > --- a/fftools/ffmpeg_demux.c > +++ b/fftools/ffmpeg_demux.c > @@ -34,6 +34,7 @@ > #include "libavutil/time.h" > #include "libavutil/timestamp.h" > > +#include "libavcodec/bsf.h" > #include "libavcodec/packet.h" > > #include "libavformat/avformat.h" > @@ -71,6 +72,8 @@ typedef struct DemuxStream { > > const AVCodecDescriptor *codec_desc; > > + AVBSFContext *bsf; > + > /* number of packets successfully read for this stream */ > uint64_t nb_packets; > // combined size of all the packets read > @@ -118,6 +121,8 @@ typedef struct Demuxer { > typedef struct DemuxThreadContext { > // packet used for reading from the demuxer > AVPacket *pkt_demux; > + // packet for reading from BSFs > + AVPacket *pkt_bsf; > } DemuxThreadContext; > > static DemuxStream *ds_from_ist(InputStream *ist) > @@ -513,13 +518,17 @@ static int do_send(Demuxer *d, DemuxStream *ds, AVPacket *pkt, unsigned flags, > return 0; > } > > -static int demux_send(Demuxer *d, DemuxStream *ds, AVPacket *pkt, unsigned flags) > +static int demux_send(Demuxer *d, DemuxThreadContext *dt, DemuxStream *ds, > + AVPacket *pkt, unsigned flags) > { > InputFile *f = &d->f; > int ret; > > + // pkt can be NULL only when flushing BSFs > + av_assert0(ds->bsf || pkt); > + > // send heartbeat for sub2video streams > - if (d->pkt_heartbeat && pkt->pts != AV_NOPTS_VALUE) { > + if (d->pkt_heartbeat && pkt && pkt->pts != AV_NOPTS_VALUE) { > for (int i = 0; i < f->nb_streams; i++) { > DemuxStream *ds1 = ds_from_ist(f->streams[i]); > > @@ -537,10 +546,69 @@ static int demux_send(Demuxer *d, DemuxStream *ds, AVPacket *pkt, unsigned flags > } > } > > - ret = do_send(d, ds, pkt, flags, "demuxed"); > - if (ret < 0) > - return ret; > + if (ds->bsf) { > + if (pkt) > + av_packet_rescale_ts(pkt, pkt->time_base, ds->bsf->time_base_in); > > + ret = av_bsf_send_packet(ds->bsf, pkt); > + if (ret < 0) { > + if (pkt) > + av_packet_unref(pkt); > + av_log(ds, AV_LOG_ERROR, "Error submitting a packet for filtering: %s\n", might be useful to signal the stream identifier > + av_err2str(ret)); possibly redundant? (IIRC this is shown anyway in the outer level failure message) > + return ret; > + } > + > + while (1) { > + ret = av_bsf_receive_packet(ds->bsf, dt->pkt_bsf); > + if (ret == AVERROR(EAGAIN)) > + return 0; > + else if (ret < 0) { > + if (ret != AVERROR_EOF) > + av_log(ds, AV_LOG_ERROR, > + "Error applying bitstream filters to a packet: %s\n", > + av_err2str(ret)); ditto > + return ret; > + } > + > + dt->pkt_bsf->time_base = ds->bsf->time_base_out; > + > + ret = do_send(d, ds, dt->pkt_bsf, 0, "filtered"); > + if (ret < 0) { > + av_packet_unref(dt->pkt_bsf); > + return ret; > + } > + } > + } else { > + ret = do_send(d, ds, pkt, flags, "demuxed"); > + if (ret < 0) > + return ret; > + } > + > + return 0; > +} > + > +static int demux_bsf_flush(Demuxer *d, DemuxThreadContext *dt) > +{ > + InputFile *f = &d->f; > + int ret; > + > + for (unsigned i = 0; i < f->nb_streams; i++) { > + DemuxStream *ds = ds_from_ist(f->streams[i]); > + > + if (!ds->bsf) > + continue; > + > + ret = demux_send(d, dt, ds, NULL, 0); > + ret = (ret == AVERROR_EOF) ? 0 : (ret < 0) ? ret : AVERROR_BUG; > + if (ret < 0) { > + av_log(ds, AV_LOG_ERROR, "Error flushing BSFs: %s\n", > + av_err2str(ret)); ditto > + return ret; > + } > + > + av_bsf_flush(ds->bsf); > + } > > return 0; > } > @@ -573,6 +641,7 @@ static void thread_set_name(InputFile *f) > static void demux_thread_uninit(DemuxThreadContext *dt) > { > av_packet_free(&dt->pkt_demux); > + av_packet_free(&dt->pkt_bsf); > > memset(dt, 0, sizeof(*dt)); > } > @@ -585,6 +654,10 @@ static int demux_thread_init(DemuxThreadContext *dt) > if (!dt->pkt_demux) > return AVERROR(ENOMEM); > > + dt->pkt_bsf = av_packet_alloc(); > + if (!dt->pkt_bsf) > + return AVERROR(ENOMEM); > + > return 0; > } > > @@ -619,10 +692,22 @@ static void *input_thread(void *arg) > continue; > } > if (ret < 0) { > + int ret_bsf; > + > + if (ret == AVERROR_EOF) > + av_log(d, AV_LOG_VERBOSE, "EOF while reading input\n"); > + else { > + av_log(d, AV_LOG_ERROR, "Error during demuxing: %s\n", > + av_err2str(ret)); > + ret = exit_on_error ? ret : 0; > + } > + > + ret_bsf = demux_bsf_flush(d, &dt); > + ret = err_merge(ret == AVERROR_EOF ? 0 : ret, ret_bsf); > + > if (d->loop) { > /* signal looping to our consumers */ > dt.pkt_demux->stream_index = -1; > - > ret = sch_demux_send(d->sch, f->index, dt.pkt_demux, 0); > if (ret >= 0) > ret = seek_to_start(d, (Timestamp){ .ts = dt.pkt_demux->pts, > @@ -633,14 +718,6 @@ static void *input_thread(void *arg) > /* fallthrough to the error path */ > } > > - if (ret == AVERROR_EOF) > - av_log(d, AV_LOG_VERBOSE, "EOF while reading input\n"); > - else { > - av_log(d, AV_LOG_ERROR, "Error during demuxing: %s\n", > - av_err2str(ret)); > - ret = exit_on_error ? ret : 0; > - } > - > break; > } > > @@ -677,7 +754,7 @@ static void *input_thread(void *arg) > if (d->readrate) > readrate_sleep(d); > > - ret = demux_send(d, ds, dt.pkt_demux, send_flags); > + ret = demux_send(d, &dt, ds, dt.pkt_demux, send_flags); > if (ret < 0) > break; > } > @@ -735,9 +812,11 @@ static void demux_final_stats(Demuxer *d) > static void ist_free(InputStream **pist) > { > InputStream *ist = *pist; > + DemuxStream *ds; > > if (!ist) > return; > + ds = ds_from_ist(ist); > > dec_free(&ist->decoder); > > @@ -749,6 +828,8 @@ static void ist_free(InputStream **pist) > avcodec_free_context(&ist->dec_ctx); > avcodec_parameters_free(&ist->par); > > + av_bsf_free(&ds->bsf); > + > av_freep(pist); > } > > @@ -1039,6 +1120,7 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st) > const char *hwaccel = NULL; > char *hwaccel_output_format = NULL; > char *codec_tag = NULL; > + char *bsfs = NULL; > char *next; > char *discard_str = NULL; > int ret; > @@ -1258,6 +1340,33 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st) > return ret; > } > > + MATCH_PER_STREAM_OPT(bitstream_filters, str, bsfs, ic, st); > + if (bsfs) { > + ret = av_bsf_list_parse_str(bsfs, &ds->bsf); > + if (ret < 0) { > + av_log(ist, AV_LOG_ERROR, > + "Error parsing bitstream filter sequence '%s': %s\n", > + bsfs, av_err2str(ret)); > + return ret; > + } > + > + ret = avcodec_parameters_copy(ds->bsf->par_in, ist->par); > + if (ret < 0) > + return ret; > + ds->bsf->time_base_in = ist->st->time_base; > + > + ret = av_bsf_init(ds->bsf); > + if (ret < 0) { > + av_log(ist, AV_LOG_ERROR, "Error initializing bitstream filters: %s\n", > + av_err2str(ret)); > + return ret; > + } > + > + ret = avcodec_parameters_copy(ist->par, ds->bsf->par_out); > + if (ret < 0) > + return ret; > + } > + > ds->codec_desc = avcodec_descriptor_get(ist->par->codec_id); > > return 0; > diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c > index c189cf373b..76b50c0bad 100644 > --- a/fftools/ffmpeg_opt.c > +++ b/fftools/ffmpeg_opt.c > @@ -1919,7 +1919,7 @@ const OptionDef options[] = { > "0 = use frame rate (video) or sample rate (audio)," > "-1 = match source time base", "ratio" }, > > - { "bsf", OPT_TYPE_STRING, OPT_SPEC | OPT_EXPERT | OPT_OUTPUT, > + { "bsf", OPT_TYPE_STRING, OPT_SPEC | OPT_EXPERT | OPT_OUTPUT | OPT_INPUT, > { .off = OFFSET(bitstream_filters) }, > "A comma-separated list of bitstream filters", "bitstream_filters", }, > > diff --git a/tests/fate/ffmpeg.mak b/tests/fate/ffmpeg.mak > index 1bfd5c1b31..df955df4d0 100644 > --- a/tests/fate/ffmpeg.mak > +++ b/tests/fate/ffmpeg.mak > @@ -256,3 +256,8 @@ FATE_SAMPLES_FFMPEG-$(call FRAMECRC, MPEGVIDEO, MPEG2VIDEO) += fate-ffmpeg-input > fate-ffmpeg-error-rate-fail: CMD = ffmpeg -i $(TARGET_SAMPLES)/mkv/h264_tta_undecodable.mkv -c:v copy -f null -; test $$? -eq 69 > fate-ffmpeg-error-rate-pass: CMD = ffmpeg -i $(TARGET_SAMPLES)/mkv/h264_tta_undecodable.mkv -c:v copy -f null - -max_error_rate 1 > FATE_SAMPLES_FFMPEG-$(call ENCDEC, PCM_S16LE TTA, NULL MATROSKA) += fate-ffmpeg-error-rate-fail fate-ffmpeg-error-rate-pass > + > +# test input -bsf > +# use -stream_loop, because it tests flushing bsfs > +fate-ffmpeg-bsf-input: CMD = framecrc -stream_loop 2 -bsf setts=PTS*2 -i $(TARGET_SAMPLES)/hevc/extradata-reload-multi-stsd.mov -c copy > +FATE_SAMPLES_FFMPEG-$(call FRAMECRC, MOV, , SETTS_BSF) += fate-ffmpeg-bsf-input > diff --git a/tests/ref/fate/ffmpeg-bsf-input b/tests/ref/fate/ffmpeg-bsf-input > new file mode 100644 > index 0000000000..67dd57cf6d > --- /dev/null > +++ b/tests/ref/fate/ffmpeg-bsf-input > @@ -0,0 +1,18 @@ > +#extradata 0: 110, 0xb4031479 > +#tb 0: 1/25 > +#media_type 0: video > +#codec_id 0: hevc > +#dimensions 0: 128x128 > +#sar 0: 1/1 > +0, 0, 0, 1, 2108, 0x57c38f64 > +0, 2, 2, 1, 31, 0xabe10d25, F=0x0 > +0, 4, 4, 1, 1915, 0xd430347f, S=1, 109 > +0, 6, 6, 1, 35, 0xc4ad0d4c, F=0x0 > +0, 8, 8, 1, 2108, 0x57c38f64, S=1, 110 > +0, 10, 10, 1, 31, 0xabe10d25, F=0x0 > +0, 12, 12, 1, 1915, 0xd430347f, S=1, 109 > +0, 14, 14, 1, 35, 0xc4ad0d4c, F=0x0 > +0, 16, 16, 1, 2108, 0x57c38f64, S=1, 110 > +0, 18, 18, 1, 31, 0xabe10d25, F=0x0 > +0, 20, 20, 1, 1915, 0xd430347f, S=1, 109 > +0, 22, 22, 1, 35, 0xc4ad0d4c, F=0x0 LGTM otherwise (and nice feature!). _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [FFmpeg-devel] [PATCH 7/8] fftools/ffmpeg_demux: implement -bsf for input 2024-01-06 12:12 ` Stefano Sabatini @ 2024-01-17 9:02 ` Anton Khirnov 2024-01-20 11:32 ` Stefano Sabatini 0 siblings, 1 reply; 28+ messages in thread From: Anton Khirnov @ 2024-01-17 9:02 UTC (permalink / raw) To: FFmpeg development discussions and patches Quoting Stefano Sabatini (2024-01-06 13:12:19) > > This looks spurious, since this suggests the example is about the > listing, and it's applying a weird order of example/explanation > (rather than the opposite). I see nothing weird about this order, it's the standard way it is done in most literature I encounter. I find the reverse order you're suggesting far more weird and unnatural. -- Anton Khirnov _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [FFmpeg-devel] [PATCH 7/8] fftools/ffmpeg_demux: implement -bsf for input 2024-01-17 9:02 ` Anton Khirnov @ 2024-01-20 11:32 ` Stefano Sabatini 2024-01-21 17:43 ` Anton Khirnov 0 siblings, 1 reply; 28+ messages in thread From: Stefano Sabatini @ 2024-01-20 11:32 UTC (permalink / raw) To: FFmpeg development discussions and patches On date Wednesday 2024-01-17 10:02:31 +0100, Anton Khirnov wrote: > Quoting Stefano Sabatini (2024-01-06 13:12:19) > > > > This looks spurious, since this suggests the example is about the > > listing, and it's applying a weird order of example/explanation > > (rather than the opposite). Use the @code{-bsfs} option to get the list of bitstream filters. E.g. @example ... The problem here is that "E.g." is placed close to a statement about the listing, therefore it might sound like the example is about the listing (which is not). > I see nothing weird about this order, it's the standard way it is done > in most literature I encounter. I find the reverse order you're > suggesting far more weird and unnatural. When you present an example you usually start with an explanation (what it does) and then present the command, not the other way around. Also the following: -------------------------------------- ffmpeg -bsf:v h264_mp4toannexb -i h264.mp4 -c:v copy -an out.h264 @end example applies the @code{h264_mp4toannexb} bitstream filter (which converts MP4-encapsulated H.264 stream to Annex B) to the @emph{input} video stream. On the other hand, @example ffmpeg -i file.mov -an -vn -bsf:s mov2textsub -c:s copy -f rawvideo sub.txt @end example applies the @code{mov2textsub} bitstream filter (which extracts text from MOV subtitles) to the @emph{output} subtitle stream. Note, however, that since both examples use @code{-c copy}, it matters little whether the filters are applied on input or output - that would change if transcoding was hapenning. --------------------------------------- this makes the reader need to correlate the two examples to figure them out, that's why I reworked the presentation in my suggestion as a more linear sequence of presentation/command/presentation/command. In general examples should focus on how a task can be done, not on the explanation of the command itself. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [FFmpeg-devel] [PATCH 7/8] fftools/ffmpeg_demux: implement -bsf for input 2024-01-20 11:32 ` Stefano Sabatini @ 2024-01-21 17:43 ` Anton Khirnov 2024-01-21 18:22 ` Stefano Sabatini 0 siblings, 1 reply; 28+ messages in thread From: Anton Khirnov @ 2024-01-21 17:43 UTC (permalink / raw) To: FFmpeg development discussions and patches Quoting Stefano Sabatini (2024-01-20 12:32:42) > On date Wednesday 2024-01-17 10:02:31 +0100, Anton Khirnov wrote: > > Quoting Stefano Sabatini (2024-01-06 13:12:19) > > > > > > This looks spurious, since this suggests the example is about the > > > listing, and it's applying a weird order of example/explanation > > > (rather than the opposite). > > Use the @code{-bsfs} option to get the list of bitstream filters. E.g. > @example > ... > > The problem here is that "E.g." is placed close to a statement about > the listing, therefore it might sound like the example is about the > listing (which is not). I moved it to a new paragraph. > > I see nothing weird about this order, it's the standard way it is done > > in most literature I encounter. I find the reverse order you're > > suggesting far more weird and unnatural. > > When you present an example you usually start with an explanation > (what it does) and then present the command, not the other way around. I don't, neither does most literature I can recall. Typically you first present a thing, then explain its structure. Explaning the structure of something the reader has not seen yet is backwards, unnatural, and hard to understand. > > Also the following: > -------------------------------------- > ffmpeg -bsf:v h264_mp4toannexb -i h264.mp4 -c:v copy -an out.h264 > @end example > applies the @code{h264_mp4toannexb} bitstream filter (which converts > MP4-encapsulated H.264 stream to Annex B) to the @emph{input} video stream. > > On the other hand, > @example > ffmpeg -i file.mov -an -vn -bsf:s mov2textsub -c:s copy -f rawvideo sub.txt > @end example > applies the @code{mov2textsub} bitstream filter (which extracts text from MOV > subtitles) to the @emph{output} subtitle stream. Note, however, that since both > examples use @code{-c copy}, it matters little whether the filters are applied > on input or output - that would change if transcoding was hapenning. > --------------------------------------- > > this makes the reader need to correlate the two examples to figure > them out, that's why I reworked the presentation in my suggestion as a > more linear sequence of presentation/command/presentation/command. > > In general examples should focus on how a task can be done, not on the > explanation of the command itself. I disagree. Examples should focus on whatever can be usefully explained with an example. -- Anton Khirnov _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [FFmpeg-devel] [PATCH 7/8] fftools/ffmpeg_demux: implement -bsf for input 2024-01-21 17:43 ` Anton Khirnov @ 2024-01-21 18:22 ` Stefano Sabatini 2024-01-21 18:35 ` Anton Khirnov 0 siblings, 1 reply; 28+ messages in thread From: Stefano Sabatini @ 2024-01-21 18:22 UTC (permalink / raw) To: FFmpeg development discussions and patches On date Sunday 2024-01-21 18:43:36 +0100, Anton Khirnov wrote: > Quoting Stefano Sabatini (2024-01-20 12:32:42) [...] > > When you present an example you usually start with an explanation > > (what it does) and then present the command, not the other way around. > > I don't, neither does most literature I can recall. Typically you first > present a thing, then explain its structure. Explaning the structure of > something the reader has not seen yet is backwards, unnatural, and hard > to understand. I still don't understand what "literature" you are referring to. If you see most examples in the FFmpeg docs they are in the form: @item This does this and that...: @example ... @end example An explanation is presented *before* introducing the example itself, in other words plain English before the actual command/code. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [FFmpeg-devel] [PATCH 7/8] fftools/ffmpeg_demux: implement -bsf for input 2024-01-21 18:22 ` Stefano Sabatini @ 2024-01-21 18:35 ` Anton Khirnov 2024-01-21 19:15 ` Stefano Sabatini 0 siblings, 1 reply; 28+ messages in thread From: Anton Khirnov @ 2024-01-21 18:35 UTC (permalink / raw) To: FFmpeg development discussions and patches Quoting Stefano Sabatini (2024-01-21 19:22:35) > On date Sunday 2024-01-21 18:43:36 +0100, Anton Khirnov wrote: > > Quoting Stefano Sabatini (2024-01-20 12:32:42) > [...] > > > When you present an example you usually start with an explanation > > > (what it does) and then present the command, not the other way around. > > > > I don't, neither does most literature I can recall. Typically you first > > present a thing, then explain its structure. Explaning the structure of > > something the reader has not seen yet is backwards, unnatural, and hard > > to understand. > > I still don't understand what "literature" you are referring to. Various manuals and textbooks I've read. > If you see most examples in the FFmpeg docs they are in the form: Our documentation is widely considered to be somewhere between atrocious and unusable (and sometimes actively misleading), so the fact that it does something in a specific way does not at all mean that it's a good idea. I have also personally seen (and fixed) countless instances of mindlessly perpetuated cargo cults in it. -- Anton Khirnov _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [FFmpeg-devel] [PATCH 7/8] fftools/ffmpeg_demux: implement -bsf for input 2024-01-21 18:35 ` Anton Khirnov @ 2024-01-21 19:15 ` Stefano Sabatini 2024-01-22 8:57 ` Anton Khirnov 0 siblings, 1 reply; 28+ messages in thread From: Stefano Sabatini @ 2024-01-21 19:15 UTC (permalink / raw) To: FFmpeg development discussions and patches On date Sunday 2024-01-21 19:35:01 +0100, Anton Khirnov wrote: > Quoting Stefano Sabatini (2024-01-21 19:22:35) > > On date Sunday 2024-01-21 18:43:36 +0100, Anton Khirnov wrote: > > > Quoting Stefano Sabatini (2024-01-20 12:32:42) > > [...] > > > > When you present an example you usually start with an explanation > > > > (what it does) and then present the command, not the other way around. > > > > > > I don't, neither does most literature I can recall. Typically you first > > > present a thing, then explain its structure. Explaning the structure of > > > something the reader has not seen yet is backwards, unnatural, and hard > > > to understand. > > > > I still don't understand what "literature" you are referring to. > > Various manuals and textbooks I've read. > > > If you see most examples in the FFmpeg docs they are in the form: > > Our documentation is widely considered to be somewhere between atrocious > and unusable nah, it's not so bad, also this applies to most documentation Besides FFmpeg is possibly the most sophisticated existing toolkit in terms of features/configuration, so this is somehow expected (at least if you expect a tutorial rather than a reference). > (and sometimes actively misleading), so the fact that it > does something in a specific way does not at all mean that it's a good > idea. So what do you propose instead? The fact that it is not perfect does not mean that everything is bad. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [FFmpeg-devel] [PATCH 7/8] fftools/ffmpeg_demux: implement -bsf for input 2024-01-21 19:15 ` Stefano Sabatini @ 2024-01-22 8:57 ` Anton Khirnov 0 siblings, 0 replies; 28+ messages in thread From: Anton Khirnov @ 2024-01-22 8:57 UTC (permalink / raw) To: FFmpeg development discussions and patches Quoting Stefano Sabatini (2024-01-21 20:15:46) > On date Sunday 2024-01-21 19:35:01 +0100, Anton Khirnov wrote: > > Quoting Stefano Sabatini (2024-01-21 19:22:35) > > > On date Sunday 2024-01-21 18:43:36 +0100, Anton Khirnov wrote: > > > > Quoting Stefano Sabatini (2024-01-20 12:32:42) > > > [...] > > > > > When you present an example you usually start with an explanation > > > > > (what it does) and then present the command, not the other way around. > > > > > > > > I don't, neither does most literature I can recall. Typically you first > > > > present a thing, then explain its structure. Explaning the structure of > > > > something the reader has not seen yet is backwards, unnatural, and hard > > > > to understand. > > > > > > I still don't understand what "literature" you are referring to. > > > > Various manuals and textbooks I've read. > > > > > If you see most examples in the FFmpeg docs they are in the form: > > > > > Our documentation is widely considered to be somewhere between atrocious > > and unusable > > nah, it's not so bad, also this applies to most documentation > > Besides FFmpeg is possibly the most sophisticated existing toolkit in > terms of features/configuration, so this is somehow expected (at least > if you expect a tutorial rather than a reference). I wouldn't be so sure. E.g. Qt has a bigger and more complex API than ours, yet its documentation is more complete and coherent. > > (and sometimes actively misleading), so the fact that it > > does something in a specific way does not at all mean that it's a good > > idea. > > So what do you propose instead? The fact that it is not perfect does > not mean that everything is bad. I'm not saying everything is bad. I'm saying this specific way of writing examples is bad and should be changed. Which is what I'm doing in this patch. -- Anton Khirnov _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 28+ messages in thread
* [FFmpeg-devel] [PATCH 8/8] fftools/ffmpeg_opt: mark -stream_group as expert option 2024-01-05 16:42 [FFmpeg-devel] [PATCH 1/8] fftools/ffmpeg_demux: replace abort() by av_assert0(0) Anton Khirnov ` (5 preceding siblings ...) 2024-01-05 16:42 ` [FFmpeg-devel] [PATCH 7/8] fftools/ffmpeg_demux: implement -bsf for input Anton Khirnov @ 2024-01-05 16:42 ` Anton Khirnov 2024-01-06 12:12 ` Stefano Sabatini 2024-01-06 11:18 ` [FFmpeg-devel] [PATCH 1/8] fftools/ffmpeg_demux: replace abort() by av_assert0(0) Stefano Sabatini 7 siblings, 1 reply; 28+ messages in thread From: Anton Khirnov @ 2024-01-05 16:42 UTC (permalink / raw) To: ffmpeg-devel --- fftools/ffmpeg_opt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c index 76b50c0bad..ea995f2b5f 100644 --- a/fftools/ffmpeg_opt.c +++ b/fftools/ffmpeg_opt.c @@ -1531,7 +1531,7 @@ const OptionDef options[] = { { "program", OPT_TYPE_STRING, OPT_SPEC | OPT_EXPERT | OPT_OUTPUT, { .off = OFFSET(program) }, "add program with specified streams", "title=string:st=number..." }, - { "stream_group", OPT_TYPE_STRING, OPT_SPEC | OPT_OUTPUT, + { "stream_group", OPT_TYPE_STRING, OPT_SPEC | OPT_OUTPUT | OPT_EXPERT, { .off = OFFSET(stream_groups) }, "add stream group with specified streams and group type-specific arguments", "id=number:st=number..." }, { "dframes", OPT_TYPE_FUNC, OPT_FUNC_ARG | OPT_PERFILE | OPT_EXPERT | OPT_OUTPUT | OPT_HAS_CANON, -- 2.42.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-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [FFmpeg-devel] [PATCH 8/8] fftools/ffmpeg_opt: mark -stream_group as expert option 2024-01-05 16:42 ` [FFmpeg-devel] [PATCH 8/8] fftools/ffmpeg_opt: mark -stream_group as expert option Anton Khirnov @ 2024-01-06 12:12 ` Stefano Sabatini 0 siblings, 0 replies; 28+ messages in thread From: Stefano Sabatini @ 2024-01-06 12:12 UTC (permalink / raw) To: FFmpeg development discussions and patches On date Friday 2024-01-05 17:42:51 +0100, Anton Khirnov wrote: > --- > fftools/ffmpeg_opt.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c > index 76b50c0bad..ea995f2b5f 100644 > --- a/fftools/ffmpeg_opt.c > +++ b/fftools/ffmpeg_opt.c > @@ -1531,7 +1531,7 @@ const OptionDef options[] = { > { "program", OPT_TYPE_STRING, OPT_SPEC | OPT_EXPERT | OPT_OUTPUT, > { .off = OFFSET(program) }, > "add program with specified streams", "title=string:st=number..." }, > - { "stream_group", OPT_TYPE_STRING, OPT_SPEC | OPT_OUTPUT, > + { "stream_group", OPT_TYPE_STRING, OPT_SPEC | OPT_OUTPUT | OPT_EXPERT, > { .off = OFFSET(stream_groups) }, > "add stream group with specified streams and group type-specific arguments", "id=number:st=number..." }, > { "dframes", OPT_TYPE_FUNC, OPT_FUNC_ARG | OPT_PERFILE | OPT_EXPERT | OPT_OUTPUT | OPT_HAS_CANON, Possibly ok. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [FFmpeg-devel] [PATCH 1/8] fftools/ffmpeg_demux: replace abort() by av_assert0(0) 2024-01-05 16:42 [FFmpeg-devel] [PATCH 1/8] fftools/ffmpeg_demux: replace abort() by av_assert0(0) Anton Khirnov ` (6 preceding siblings ...) 2024-01-05 16:42 ` [FFmpeg-devel] [PATCH 8/8] fftools/ffmpeg_opt: mark -stream_group as expert option Anton Khirnov @ 2024-01-06 11:18 ` Stefano Sabatini 7 siblings, 0 replies; 28+ messages in thread From: Stefano Sabatini @ 2024-01-06 11:18 UTC (permalink / raw) To: FFmpeg development discussions and patches On date Friday 2024-01-05 17:42:44 +0100, Anton Khirnov wrote: > This is the standard way to mark unreachable cases in a switch > --- > fftools/ffmpeg_demux.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c > index 95af31e9ef..5d07b7153d 100644 > --- a/fftools/ffmpeg_demux.c > +++ b/fftools/ffmpeg_demux.c > @@ -1223,8 +1223,7 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st) > case AVMEDIA_TYPE_ATTACHMENT: > case AVMEDIA_TYPE_UNKNOWN: > break; > - default: > - abort(); > + default: av_assert0(0); LGTM, also probably we might employ a self-documentation trick of the kind: av_assert0(!"handled media type"); _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2024-01-22 8:57 UTC | newest] Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-01-05 16:42 [FFmpeg-devel] [PATCH 1/8] fftools/ffmpeg_demux: replace abort() by av_assert0(0) Anton Khirnov 2024-01-05 16:42 ` [FFmpeg-devel] [PATCH 2/8] doc/ffmpeg: drop documentation for non-existent -sbsf Anton Khirnov 2024-01-06 11:22 ` Stefano Sabatini 2024-01-05 16:42 ` [FFmpeg-devel] [PATCH 3/8] fftools/ffmpeg_demux: do not set bitexact directly on the decoder Anton Khirnov 2024-01-06 11:24 ` Stefano Sabatini 2024-01-05 16:42 ` [FFmpeg-devel] [PATCH 4/8] fftools/ffmpeg_demux: set options on codec parameters rather than decoder Anton Khirnov 2024-01-06 11:31 ` Stefano Sabatini 2024-01-06 14:22 ` James Almer 2024-01-16 19:49 ` Anton Khirnov 2024-01-16 22:37 ` James Almer 2024-01-05 16:42 ` [FFmpeg-devel] [PATCH 5/8] fftools/ffmpeg_demux: only allocate the decoder when actually decoding Anton Khirnov 2024-01-06 11:34 ` Stefano Sabatini 2024-01-16 19:50 ` Anton Khirnov 2024-01-05 16:42 ` [FFmpeg-devel] [PATCH 6/8] fftools/ffmpeg_demux: add demuxing thread private data Anton Khirnov 2024-01-06 11:44 ` Stefano Sabatini 2024-01-16 19:52 ` Anton Khirnov 2024-01-05 16:42 ` [FFmpeg-devel] [PATCH 7/8] fftools/ffmpeg_demux: implement -bsf for input Anton Khirnov 2024-01-06 12:12 ` Stefano Sabatini 2024-01-17 9:02 ` Anton Khirnov 2024-01-20 11:32 ` Stefano Sabatini 2024-01-21 17:43 ` Anton Khirnov 2024-01-21 18:22 ` Stefano Sabatini 2024-01-21 18:35 ` Anton Khirnov 2024-01-21 19:15 ` Stefano Sabatini 2024-01-22 8:57 ` Anton Khirnov 2024-01-05 16:42 ` [FFmpeg-devel] [PATCH 8/8] fftools/ffmpeg_opt: mark -stream_group as expert option Anton Khirnov 2024-01-06 12:12 ` Stefano Sabatini 2024-01-06 11:18 ` [FFmpeg-devel] [PATCH 1/8] fftools/ffmpeg_demux: replace abort() by av_assert0(0) Stefano Sabatini
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel This inbox may be cloned and mirrored by anyone: git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \ ffmpegdev@gitmailbox.com public-inbox-index ffmpegdev Example config snippet for mirrors. AGPL code for this site: git clone https://public-inbox.org/public-inbox.git