Re: [FFmpeg-devel] [PATCH] avformat/mux: stop delaying writing the header
On 9/9/2017 12:41 PM, Michael Niedermayer wrote: > On Thu, Sep 07, 2017 at 04:17:24PM -0300, James Almer wrote: >> There's no need to wait for the first packet of every stream now that >> every bitstream filter behaves as intended. >> >> Signed-off-by: James Almer>> --- >> What should we do with the AVFMT_FLAG_AUTO_BSF flag? Do we deprecate >> it and force the automatic insertion of muxer-required bitstream >> filters now that the generic muxing code will always behave the same, >> or keep it to give the user a choice to apply said bitstream filters? >> I ask because some filters, like vp9_superframe, are necessary to >> avoid creating broken files, so it's not wise to allow the user to >> disable it. >> It would also go in line with AVCodec.bsfs, which is essentially a >> non-optional (for reasons made obvious in fa1749dd34) autobsf at the >> codec level instead of container level. >> > >> The change to fate-rgb24-mkv is a regression that can be prevented by >> committing https://ffmpeg.org/pipermail/ffmpeg-devel/2017-May/211311.html >> or a similar solution, maybe using avformat_init_output(), to make sure >> ffmpeg.c sets AVCodecParameters->field_order for the output stream before >> calling avformat_write_header(). > > i do see differences in other mkv output, i assume these are also from > field_order > > one example > ./ffmpeg -copyts -ss 11 -i ~/tickets/2263/2263-slow-ss.mkv -vframes 3 moon.mkv Yes, as i said, the solution for this could be making ffmpeg.c use avformat_init_output() and only call avformat_write_header() after the output codecpar is "complete". That being said, ffmpeg.c changing output codecpar->field_order based on some heuristics like it's currently doing is weird. Or at least wrong in its current form (There are different patches to change this, and even suggestions to reimplement the field_order enum values in other threads). See https://ffmpeg.org/pipermail/ffmpeg-devel/2017-September/215669.html and https://ffmpeg.org/pipermail/ffmpeg-devel/2017-September/215698.html I have a set of about 50 patches implementing AVOutputFormat.init() on most if not all of the remaining muxers for this purpose. I can either send them to the ml or push them to some repo, to avoid the email spam. However you prefer. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat/mux: stop delaying writing the header
On Thu, Sep 07, 2017 at 04:17:24PM -0300, James Almer wrote: > There's no need to wait for the first packet of every stream now that > every bitstream filter behaves as intended. > > Signed-off-by: James Almer> --- > What should we do with the AVFMT_FLAG_AUTO_BSF flag? Do we deprecate > it and force the automatic insertion of muxer-required bitstream > filters now that the generic muxing code will always behave the same, > or keep it to give the user a choice to apply said bitstream filters? > I ask because some filters, like vp9_superframe, are necessary to > avoid creating broken files, so it's not wise to allow the user to > disable it. > It would also go in line with AVCodec.bsfs, which is essentially a > non-optional (for reasons made obvious in fa1749dd34) autobsf at the > codec level instead of container level. > > The change to fate-rgb24-mkv is a regression that can be prevented by > committing https://ffmpeg.org/pipermail/ffmpeg-devel/2017-May/211311.html > or a similar solution, maybe using avformat_init_output(), to make sure > ffmpeg.c sets AVCodecParameters->field_order for the output stream before > calling avformat_write_header(). i do see differences in other mkv output, i assume these are also from field_order one example ./ffmpeg -copyts -ss 11 -i ~/tickets/2263/2263-slow-ss.mkv -vframes 3 moon.mkv [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Freedom in capitalist society always remains about the same as it was in ancient Greek republics: Freedom for slave owners. -- Vladimir Lenin signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] avformat/mux: stop delaying writing the header
There's no need to wait for the first packet of every stream now that every bitstream filter behaves as intended. Signed-off-by: James Almer--- What should we do with the AVFMT_FLAG_AUTO_BSF flag? Do we deprecate it and force the automatic insertion of muxer-required bitstream filters now that the generic muxing code will always behave the same, or keep it to give the user a choice to apply said bitstream filters? I ask because some filters, like vp9_superframe, are necessary to avoid creating broken files, so it's not wise to allow the user to disable it. It would also go in line with AVCodec.bsfs, which is essentially a non-optional (for reasons made obvious in fa1749dd34) autobsf at the codec level instead of container level. The change to fate-rgb24-mkv is a regression that can be prevented by committing https://ffmpeg.org/pipermail/ffmpeg-devel/2017-May/211311.html or a similar solution, maybe using avformat_init_output(), to make sure ffmpeg.c sets AVCodecParameters->field_order for the output stream before calling avformat_write_header(). libavformat/internal.h | 6 - libavformat/mux.c | 51 - libavformat/options_table.h| 2 +- libavformat/tests/fifo_muxer.c | 52 -- tests/ref/fate/fifo-muxer-tst | 1 - tests/ref/fate/rgb24-mkv | 4 ++-- 6 files changed, 13 insertions(+), 103 deletions(-) diff --git a/libavformat/internal.h b/libavformat/internal.h index d136c79bdd..d46340029b 100644 --- a/libavformat/internal.h +++ b/libavformat/internal.h @@ -121,12 +121,6 @@ struct AVFormatInternal { int avoid_negative_ts_use_pts; /** - * Whether or not a header has already been written - */ -int header_written; -int write_header_ret; - -/** * Timestamp of the end of the shortest stream. */ int64_t shortest_end; diff --git a/libavformat/mux.c b/libavformat/mux.c index 53ad46df42..6a81faf3fb 100644 --- a/libavformat/mux.c +++ b/libavformat/mux.c @@ -478,25 +478,6 @@ static void flush_if_needed(AVFormatContext *s) } } -static int write_header_internal(AVFormatContext *s) -{ -if (!(s->oformat->flags & AVFMT_NOFILE) && s->pb) -avio_write_marker(s->pb, AV_NOPTS_VALUE, AVIO_DATA_MARKER_HEADER); -if (s->oformat->write_header) { -int ret = s->oformat->write_header(s); -if (ret >= 0 && s->pb && s->pb->error < 0) -ret = s->pb->error; -s->internal->write_header_ret = ret; -if (ret < 0) -return ret; -flush_if_needed(s); -} -s->internal->header_written = 1; -if (!(s->oformat->flags & AVFMT_NOFILE) && s->pb) -avio_write_marker(s->pb, AV_NOPTS_VALUE, AVIO_DATA_MARKER_UNKNOWN); -return 0; -} - int avformat_init_output(AVFormatContext *s, AVDictionary **options) { int ret = 0; @@ -535,11 +516,18 @@ int avformat_write_header(AVFormatContext *s, AVDictionary **options) if ((ret = avformat_init_output(s, options)) < 0) return ret; -if (!(s->oformat->check_bitstream && s->flags & AVFMT_FLAG_AUTO_BSF)) { -ret = write_header_internal(s); +if (!(s->oformat->flags & AVFMT_NOFILE) && s->pb) +avio_write_marker(s->pb, AV_NOPTS_VALUE, AVIO_DATA_MARKER_HEADER); +if (s->oformat->write_header) { +int ret = s->oformat->write_header(s); +if (ret >= 0 && s->pb && s->pb->error < 0) +ret = s->pb->error; if (ret < 0) goto fail; +flush_if_needed(s); } +if (!(s->oformat->flags & AVFMT_NOFILE) && s->pb) +avio_write_marker(s->pb, AV_NOPTS_VALUE, AVIO_DATA_MARKER_UNKNOWN); if (!s->internal->streams_initialized) { if ((ret = init_pts(s)) < 0) @@ -765,12 +753,6 @@ FF_DISABLE_DEPRECATION_WARNINGS FF_ENABLE_DEPRECATION_WARNINGS #endif -if (!s->internal->header_written) { -ret = s->internal->write_header_ret ? s->internal->write_header_ret : write_header_internal(s); -if (ret < 0) -goto fail; -} - if ((pkt->flags & AV_PKT_FLAG_UNCODED_FRAME)) { AVFrame *frame = (AVFrame *)pkt->data; av_assert0(pkt->size == UNCODED_FRAME_PACKET_SIZE); @@ -786,7 +768,6 @@ FF_ENABLE_DEPRECATION_WARNINGS ret = s->pb->error; } -fail: #if FF_API_LAVF_MERGE_SD FF_DISABLE_DEPRECATION_WARNINGS if (did_split) @@ -934,11 +915,6 @@ int av_write_frame(AVFormatContext *s, AVPacket *pkt) if (!pkt) { if (s->oformat->flags & AVFMT_ALLOW_FLUSH) { -if (!s->internal->header_written) { -ret = s->internal->write_header_ret ? s->internal->write_header_ret : write_header_internal(s); -if (ret < 0) -return ret; -} ret = s->oformat->write_packet(s, NULL); flush_if_needed(s); if (ret >= 0 && s->pb && s->pb->error < 0) @@ -1322,14
Re: [FFmpeg-devel] [PATCH] avformat/mux: stop delaying writing the header
On Fri, May 26, 2017 at 05:08:51PM -0300, James Almer wrote: > There's no need to wait for the first packet of every stream now that > every bitstream filter behaves as intended. > > Signed-off-by: James Almer> --- > What should we do with the AVFMT_FLAG_AUTO_BSF flag? Do we deprecate > it and force the automatic insertion of muxer-required bitstream > filters now that the generic muxing code will always behave the same, > or keep it to give the user a choice to apply said bitstream filters? > I ask because some filters, like vp9_superframe, are necessary to > avoid creating broken files, so it's not wise to allow the user to > disable it. > It would also go in line with AVCodec.bsfs, which is essentially a > non-optional (for reasons made obvious in fa1749dd34) autobsf at the > codec level instead of container level. > > The change to fate-rgb24-mkv is a regression that can be prevented by > committing https://ffmpeg.org/pipermail/ffmpeg-devel/2017-May/211311.html > or a similar solution, maybe using avformat_init_output(), to make sure > ffmpeg.c sets AVCodecParameters->field_order for the output stream before > calling avformat_write_header(). > > libavformat/internal.h | 6 - > libavformat/mux.c | 53 > +- > libavformat/options_table.h| 2 +- > libavformat/tests/fifo_muxer.c | 52 - > tests/ref/fate/fifo-muxer-tst | 1 - > tests/ref/fate/rgb24-mkv | 4 ++-- > 6 files changed, 14 insertions(+), 104 deletions(-) seems not to apply cleanly, so i cannot test it [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB No great genius has ever existed without some touch of madness. -- Aristotle signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat/mux: stop delaying writing the header
On 5/26/2017 5:08 PM, James Almer wrote: > There's no need to wait for the first packet of every stream now that > every bitstream filter behaves as intended. > > Signed-off-by: James Almer> --- > What should we do with the AVFMT_FLAG_AUTO_BSF flag? Do we deprecate > it and force the automatic insertion of muxer-required bitstream > filters now that the generic muxing code will always behave the same, > or keep it to give the user a choice to apply said bitstream filters? > I ask because some filters, like vp9_superframe, are necessary to > avoid creating broken files, so it's not wise to allow the user to > disable it. > It would also go in line with AVCodec.bsfs, which is essentially a > non-optional (for reasons made obvious in fa1749dd34) autobsf at the > codec level instead of container level. > > The change to fate-rgb24-mkv is a regression that can be prevented by > committing https://ffmpeg.org/pipermail/ffmpeg-devel/2017-May/211311.html > or a similar solution, maybe using avformat_init_output(), to make sure > ffmpeg.c sets AVCodecParameters->field_order for the output stream before > calling avformat_write_header(). > Ping? ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] avformat/mux: stop delaying writing the header
There's no need to wait for the first packet of every stream now that every bitstream filter behaves as intended. Signed-off-by: James Almer--- What should we do with the AVFMT_FLAG_AUTO_BSF flag? Do we deprecate it and force the automatic insertion of muxer-required bitstream filters now that the generic muxing code will always behave the same, or keep it to give the user a choice to apply said bitstream filters? I ask because some filters, like vp9_superframe, are necessary to avoid creating broken files, so it's not wise to allow the user to disable it. It would also go in line with AVCodec.bsfs, which is essentially a non-optional (for reasons made obvious in fa1749dd34) autobsf at the codec level instead of container level. The change to fate-rgb24-mkv is a regression that can be prevented by committing https://ffmpeg.org/pipermail/ffmpeg-devel/2017-May/211311.html or a similar solution, maybe using avformat_init_output(), to make sure ffmpeg.c sets AVCodecParameters->field_order for the output stream before calling avformat_write_header(). libavformat/internal.h | 6 - libavformat/mux.c | 53 +- libavformat/options_table.h| 2 +- libavformat/tests/fifo_muxer.c | 52 - tests/ref/fate/fifo-muxer-tst | 1 - tests/ref/fate/rgb24-mkv | 4 ++-- 6 files changed, 14 insertions(+), 104 deletions(-) diff --git a/libavformat/internal.h b/libavformat/internal.h index c856945ce9..9c6cd77166 100644 --- a/libavformat/internal.h +++ b/libavformat/internal.h @@ -121,12 +121,6 @@ struct AVFormatInternal { int avoid_negative_ts_use_pts; /** - * Whether or not a header has already been written - */ -int header_written; -int write_header_ret; - -/** * Timestamp of the end of the shortest stream. */ int64_t shortest_end; diff --git a/libavformat/mux.c b/libavformat/mux.c index 01dcb362ae..f931d6a928 100644 --- a/libavformat/mux.c +++ b/libavformat/mux.c @@ -468,26 +468,6 @@ static int init_pts(AVFormatContext *s) return 0; } -static int write_header_internal(AVFormatContext *s) -{ -if (!(s->oformat->flags & AVFMT_NOFILE) && s->pb) -avio_write_marker(s->pb, AV_NOPTS_VALUE, AVIO_DATA_MARKER_HEADER); -if (s->oformat->write_header) { -int ret = s->oformat->write_header(s); -if (ret >= 0 && s->pb && s->pb->error < 0) -ret = s->pb->error; -s->internal->write_header_ret = ret; -if (ret < 0) -return ret; -if (s->flush_packets && s->pb && s->pb->error >= 0 && s->flags & AVFMT_FLAG_FLUSH_PACKETS) -avio_flush(s->pb); -} -s->internal->header_written = 1; -if (!(s->oformat->flags & AVFMT_NOFILE) && s->pb) -avio_write_marker(s->pb, AV_NOPTS_VALUE, AVIO_DATA_MARKER_UNKNOWN); -return 0; -} - int avformat_init_output(AVFormatContext *s, AVDictionary **options) { int ret = 0; @@ -526,11 +506,19 @@ int avformat_write_header(AVFormatContext *s, AVDictionary **options) if ((ret = avformat_init_output(s, options)) < 0) return ret; -if (!(s->oformat->check_bitstream && s->flags & AVFMT_FLAG_AUTO_BSF)) { -ret = write_header_internal(s); +if (!(s->oformat->flags & AVFMT_NOFILE) && s->pb) +avio_write_marker(s->pb, AV_NOPTS_VALUE, AVIO_DATA_MARKER_HEADER); +if (s->oformat->write_header) { +int ret = s->oformat->write_header(s); +if (ret >= 0 && s->pb && s->pb->error < 0) +ret = s->pb->error; if (ret < 0) goto fail; +if (s->flush_packets && s->pb && s->pb->error >= 0 && s->flags & AVFMT_FLAG_FLUSH_PACKETS) +avio_flush(s->pb); } +if (!(s->oformat->flags & AVFMT_NOFILE) && s->pb) +avio_write_marker(s->pb, AV_NOPTS_VALUE, AVIO_DATA_MARKER_UNKNOWN); if (!s->internal->streams_initialized) { if ((ret = init_pts(s)) < 0) @@ -756,12 +744,6 @@ FF_DISABLE_DEPRECATION_WARNINGS FF_ENABLE_DEPRECATION_WARNINGS #endif -if (!s->internal->header_written) { -ret = s->internal->write_header_ret ? s->internal->write_header_ret : write_header_internal(s); -if (ret < 0) -goto fail; -} - if ((pkt->flags & AV_PKT_FLAG_UNCODED_FRAME)) { AVFrame *frame = (AVFrame *)pkt->data; av_assert0(pkt->size == UNCODED_FRAME_PACKET_SIZE); @@ -778,7 +760,6 @@ FF_ENABLE_DEPRECATION_WARNINGS ret = s->pb->error; } -fail: #if FF_API_LAVF_MERGE_SD FF_DISABLE_DEPRECATION_WARNINGS if (did_split) @@ -926,11 +907,6 @@ int av_write_frame(AVFormatContext *s, AVPacket *pkt) if (!pkt) { if (s->oformat->flags & AVFMT_ALLOW_FLUSH) { -if (!s->internal->header_written) { -ret = s->internal->write_header_ret ? s->internal->write_header_ret : write_header_internal(s); -if (ret < 0)