Re: [FFmpeg-devel] [PATCH] avformat/mux: stop delaying writing the header

2017-09-09 Thread James Almer
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

2017-09-09 Thread Michael Niedermayer
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

2017-09-07 Thread James Almer
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

2017-09-07 Thread Michael Niedermayer
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

2017-09-06 Thread James Almer
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

2017-05-26 Thread James Almer
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)