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

2017-11-27 Thread James Almer
On 11/27/2017 8:52 PM, James Almer wrote:
> On 11/27/2017 8:18 PM, Michael Niedermayer wrote:
>> On Sun, Nov 26, 2017 at 05:51:03PM -0300, James Almer wrote:
>>> Every bitstream filter behaves as intended now, so there's no need to
>>> wait for the first packet of every stream.
>>>
>>> Signed-off-by: James Almer 
>>> ---
>>>  libavformat/avformat.h |  2 +-
>>>  libavformat/internal.h |  6 -
>>>  libavformat/mux.c  | 52 
>>> --
>>>  libavformat/options_table.h|  2 +-
>>>  libavformat/tests/fifo_muxer.c | 52 
>>> --
>>>  tests/ref/fate/fifo-muxer-tst  |  1 -
>>>  6 files changed, 12 insertions(+), 103 deletions(-)
>>
>> Is this intended to change any files ?
>>
>> this changes slightly:
>> ./ffmpeg -i ~/tickets/2263/2263-slow-ss.mkv -vframes 3 file.mkv
>>
>> not saying this is a issue in the patch or anything (i am in favor of
>> the simplification in fact), just that i saw this change
> 
> I can't access that file, but just in case it's related, this patch is
> meant to be applied after the first. Otherwise, as the first patch
> states, stream parameters that might be changed by ffmpeg.c while
> processing an input frame (specifically field_order) will not be taken
> into account during write_header.
> You might remember i have sent this patch alone before, and how it
> affected matroska files because of differing field_order values,
> including fate-rgb24-mkv.
> 
> For that matter, ffmpeg.c changing any output codecpar values while
> reading an input frame (that is, long after the output file and streams
> have been initialized) is a very hacky thing to do. This is something
> I'd expect avformat_find_stream_info() to do instead, since it decodes
> some frames to get all the info it needs, and not the API user (ffmpeg.c).

Just tested it, and it was indeed field_order in the output codecpar.
And it unfortunately happens with both patches applied.
Looks like the packet that gets processed by ffmpeg.c to fill
field_order is not necessarily the first to show up, and the only way to
effectively write the header after it is with the delayed header code as
it's implemented in av_interleaved_write_frame.

So yeah, patch 1 can be applied or dropped as preferred (it after all
does at least cover some cases, like fate-rgb24-mkv), but this one
should still be applied regardless of the above.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


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

2017-11-27 Thread James Almer
On 11/27/2017 9:00 PM, Carl Eugen Hoyos wrote:
> 2017-11-28 0:52 GMT+01:00 James Almer :
>> On 11/27/2017 8:18 PM, Michael Niedermayer wrote:
> 
>>> this changes slightly:
>>> ./ffmpeg -i ~/tickets/2263/2263-slow-ss.mkv -vframes 3 file.mkv
>>>
>>> not saying this is a issue in the patch or anything (i am in favor of
>>> the simplification in fact), just that i saw this change
>>
>> I can't access that file
> 
> http://samples.ffmpeg.org/ffmpeg-bugs/trac/ticket2263/
> Will remove it again.

Thanks.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


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

2017-11-27 Thread Carl Eugen Hoyos
2017-11-28 0:52 GMT+01:00 James Almer :
> On 11/27/2017 8:18 PM, Michael Niedermayer wrote:

>> this changes slightly:
>> ./ffmpeg -i ~/tickets/2263/2263-slow-ss.mkv -vframes 3 file.mkv
>>
>> not saying this is a issue in the patch or anything (i am in favor of
>> the simplification in fact), just that i saw this change
>
> I can't access that file

http://samples.ffmpeg.org/ffmpeg-bugs/trac/ticket2263/
Will remove it again.

Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


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

2017-11-27 Thread James Almer
On 11/27/2017 8:18 PM, Michael Niedermayer wrote:
> On Sun, Nov 26, 2017 at 05:51:03PM -0300, James Almer wrote:
>> Every bitstream filter behaves as intended now, so there's no need to
>> wait for the first packet of every stream.
>>
>> Signed-off-by: James Almer 
>> ---
>>  libavformat/avformat.h |  2 +-
>>  libavformat/internal.h |  6 -
>>  libavformat/mux.c  | 52 
>> --
>>  libavformat/options_table.h|  2 +-
>>  libavformat/tests/fifo_muxer.c | 52 
>> --
>>  tests/ref/fate/fifo-muxer-tst  |  1 -
>>  6 files changed, 12 insertions(+), 103 deletions(-)
> 
> Is this intended to change any files ?
> 
> this changes slightly:
> ./ffmpeg -i ~/tickets/2263/2263-slow-ss.mkv -vframes 3 file.mkv
> 
> not saying this is a issue in the patch or anything (i am in favor of
> the simplification in fact), just that i saw this change

I can't access that file, but just in case it's related, this patch is
meant to be applied after the first. Otherwise, as the first patch
states, stream parameters that might be changed by ffmpeg.c while
processing an input frame (specifically field_order) will not be taken
into account during write_header.
You might remember i have sent this patch alone before, and how it
affected matroska files because of differing field_order values,
including fate-rgb24-mkv.

For that matter, ffmpeg.c changing any output codecpar values while
reading an input frame (that is, long after the output file and streams
have been initialized) is a very hacky thing to do. This is something
I'd expect avformat_find_stream_info() to do instead, since it decodes
some frames to get all the info it needs, and not the API user (ffmpeg.c).
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


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

2017-11-27 Thread Michael Niedermayer
On Sun, Nov 26, 2017 at 05:51:03PM -0300, James Almer wrote:
> Every bitstream filter behaves as intended now, so there's no need to
> wait for the first packet of every stream.
> 
> Signed-off-by: James Almer 
> ---
>  libavformat/avformat.h |  2 +-
>  libavformat/internal.h |  6 -
>  libavformat/mux.c  | 52 
> --
>  libavformat/options_table.h|  2 +-
>  libavformat/tests/fifo_muxer.c | 52 
> --
>  tests/ref/fate/fifo-muxer-tst  |  1 -
>  6 files changed, 12 insertions(+), 103 deletions(-)

Is this intended to change any files ?

this changes slightly:
./ffmpeg -i ~/tickets/2263/2263-slow-ss.mkv -vframes 3 file.mkv

not saying this is a issue in the patch or anything (i am in favor of
the simplification in fact), just that i saw this change

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Old school: Use the lowest level language in which you can solve the problem
conveniently.
New school: Use the highest level language in which the latest supercomputer
can solve the problem without the user falling asleep waiting.


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


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

2017-11-27 Thread James Almer
On 11/27/2017 6:00 AM, Tobias Rapp wrote:
> On 26.11.2017 21:51, James Almer wrote:
>> Every bitstream filter behaves as intended now, so there's no need to
>> wait for the first packet of every stream.
>>
>> Signed-off-by: James Almer 
>> ---
>>   libavformat/avformat.h |  2 +-
>>   libavformat/internal.h |  6 -
>>   libavformat/mux.c  | 52
>> --
>>   libavformat/options_table.h    |  2 +-
>>   libavformat/tests/fifo_muxer.c | 52
>> --
>>   tests/ref/fate/fifo-muxer-tst  |  1 -
>>   6 files changed, 12 insertions(+), 103 deletions(-)
>>
>> [...]
> 
> I'm just curious: Will this fix ticket #6375?
> 
> Regards,
> Tobias

Unlikely. That's from ffmpeg.c buffering logic which I'm not changing here.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


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

2017-11-27 Thread Tobias Rapp

On 26.11.2017 21:51, James Almer wrote:

Every bitstream filter behaves as intended now, so there's no need to
wait for the first packet of every stream.

Signed-off-by: James Almer 
---
  libavformat/avformat.h |  2 +-
  libavformat/internal.h |  6 -
  libavformat/mux.c  | 52 --
  libavformat/options_table.h|  2 +-
  libavformat/tests/fifo_muxer.c | 52 --
  tests/ref/fate/fifo-muxer-tst  |  1 -
  6 files changed, 12 insertions(+), 103 deletions(-)

[...]


I'm just curious: Will this fix ticket #6375?

Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel