diff mbox

[FFmpeg-devel] avformat/mux: skip parameter and pts checks for data muxer

Message ID 56ac1f4e-bcdf-856d-23f3-32032948b57b@gyani.pro
State New
Headers show

Commit Message

Gyan Doshi April 26, 2019, 1:08 p.m. UTC 
From 5ec154870d9c559037598b41736bf5b216a756e0 Mon Sep 17 00:00:00 2001
From: Gyan Doshi <ffmpeg@gyani.pro>
Date: Fri, 26 Apr 2019 18:31:33 +0530
Subject: [PATCH] avformat/mux: skip parameter and pts checks for data muxer

Allows to dump a malformed stream for external inspection or repair.
---
 libavformat/mux.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

Comments

Michael Niedermayer April 26, 2019, 8:02 p.m. UTC | #1 
On Fri, Apr 26, 2019 at 06:38:37PM +0530, Gyan wrote:
> 

>  mux.c |    9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> d94a699f5dbc31a8ee8b7d1bdb33004d9cd95d46  0001-avformat-mux-skip-parameter-and-pts-checks-for-data-.patch
> From 5ec154870d9c559037598b41736bf5b216a756e0 Mon Sep 17 00:00:00 2001
> From: Gyan Doshi <ffmpeg@gyani.pro>
> Date: Fri, 26 Apr 2019 18:31:33 +0530
> Subject: [PATCH] avformat/mux: skip parameter and pts checks for data muxer
> 
> Allows to dump a malformed stream for external inspection or repair.
> ---
>  libavformat/mux.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/libavformat/mux.c b/libavformat/mux.c
> index 83fe1de78f..3699b572f2 100644
> --- a/libavformat/mux.c
> +++ b/libavformat/mux.c
> @@ -290,6 +290,9 @@ FF_ENABLE_DEPRECATION_WARNINGS
>          goto fail;
>      }
>  
> +    if (!strcmp(of->name, "data"))
> +        goto bypass;
> +
>      for (i = 0; i < s->nb_streams; i++) {
>          st  = s->streams[i];
>          par = st->codecpar;
> @@ -409,6 +412,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
>          av_dict_set(&s->metadata, e->key, NULL, 0);
>      }
>  
> +bypass:

I think this skips a bit more than what would make sense
(for example priv_data allocation but thats not the only odd thing)

also iam not sure this is the ideal approuch.
I mean "I want to dump inavlid data in a data muxer for debug"
that seems a potentially valid request for other muxers too
like the image muxer producing individual files per frame and
so on


[...]
Gyan Doshi April 27, 2019, 4:31 a.m. UTC | #2 
On 27-04-2019 01:32 AM, Michael Niedermayer wrote:
> On Fri, Apr 26, 2019 at 06:38:37PM +0530, Gyan wrote:
>>   mux.c |    9 ++++++++-
>>   1 file changed, 8 insertions(+), 1 deletion(-)
>> d94a699f5dbc31a8ee8b7d1bdb33004d9cd95d46  0001-avformat-mux-skip-parameter-and-pts-checks-for-data-.patch
>>  From 5ec154870d9c559037598b41736bf5b216a756e0 Mon Sep 17 00:00:00 2001
>> From: Gyan Doshi <ffmpeg@gyani.pro>
>> Date: Fri, 26 Apr 2019 18:31:33 +0530
>> Subject: [PATCH] avformat/mux: skip parameter and pts checks for data muxer
>>
>> Allows to dump a malformed stream for external inspection or repair.
>> ---
>>   libavformat/mux.c | 9 ++++++++-
>>   1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/libavformat/mux.c b/libavformat/mux.c
>> index 83fe1de78f..3699b572f2 100644
>> --- a/libavformat/mux.c
>> +++ b/libavformat/mux.c
>> @@ -290,6 +290,9 @@ FF_ENABLE_DEPRECATION_WARNINGS
>>           goto fail;
>>       }
>>   
>> +    if (!strcmp(of->name, "data"))
>> +        goto bypass;
>> +
>>       for (i = 0; i < s->nb_streams; i++) {
>>           st  = s->streams[i];
>>           par = st->codecpar;
>> @@ -409,6 +412,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
>>           av_dict_set(&s->metadata, e->key, NULL, 0);
>>       }
>>   
>> +bypass:
> I think this skips a bit more than what would make sense
> (for example priv_data allocation but thats not the only odd thing)
>
> also iam not sure this is the ideal approuch.
> I mean "I want to dump inavlid data in a data muxer for debug"
> that seems a potentially valid request for other muxers too
> like the image muxer producing individual files per frame and
> so on
What would be the ideal approach?

Gyan
Michael Niedermayer April 28, 2019, 9:56 a.m. UTC | #3 
On Sat, Apr 27, 2019 at 10:01:53AM +0530, Gyan wrote:
> 
> 
> On 27-04-2019 01:32 AM, Michael Niedermayer wrote:
> >On Fri, Apr 26, 2019 at 06:38:37PM +0530, Gyan wrote:
> >>  mux.c |    9 ++++++++-
> >>  1 file changed, 8 insertions(+), 1 deletion(-)
> >>d94a699f5dbc31a8ee8b7d1bdb33004d9cd95d46  0001-avformat-mux-skip-parameter-and-pts-checks-for-data-.patch
> >> From 5ec154870d9c559037598b41736bf5b216a756e0 Mon Sep 17 00:00:00 2001
> >>From: Gyan Doshi <ffmpeg@gyani.pro>
> >>Date: Fri, 26 Apr 2019 18:31:33 +0530
> >>Subject: [PATCH] avformat/mux: skip parameter and pts checks for data muxer
> >>
> >>Allows to dump a malformed stream for external inspection or repair.
> >>---
> >>  libavformat/mux.c | 9 ++++++++-
> >>  1 file changed, 8 insertions(+), 1 deletion(-)
> >>
> >>diff --git a/libavformat/mux.c b/libavformat/mux.c
> >>index 83fe1de78f..3699b572f2 100644
> >>--- a/libavformat/mux.c
> >>+++ b/libavformat/mux.c
> >>@@ -290,6 +290,9 @@ FF_ENABLE_DEPRECATION_WARNINGS
> >>          goto fail;
> >>      }
> >>+    if (!strcmp(of->name, "data"))
> >>+        goto bypass;
> >>+
> >>      for (i = 0; i < s->nb_streams; i++) {
> >>          st  = s->streams[i];
> >>          par = st->codecpar;
> >>@@ -409,6 +412,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
> >>          av_dict_set(&s->metadata, e->key, NULL, 0);
> >>      }
> >>+bypass:
> >I think this skips a bit more than what would make sense
> >(for example priv_data allocation but thats not the only odd thing)
> >
> >also iam not sure this is the ideal approuch.
> >I mean "I want to dump inavlid data in a data muxer for debug"
> >that seems a potentially valid request for other muxers too
> >like the image muxer producing individual files per frame and
> >so on
> What would be the ideal approach?

I guess the ideal approuch would be to allow the developer who needs
this to override the check when the muxer in use can actually saftely
mux it without the specific check.
There are for example muxers which would not function properly with
backward going dts or something like that

[...]
Hendrik Leppkes April 28, 2019, 10:10 a.m. UTC | #4 
On Sun, Apr 28, 2019 at 11:57 AM Michael Niedermayer
<michael@niedermayer.cc> wrote:
>
> On Sat, Apr 27, 2019 at 10:01:53AM +0530, Gyan wrote:
> >
> >
> > On 27-04-2019 01:32 AM, Michael Niedermayer wrote:
> > >On Fri, Apr 26, 2019 at 06:38:37PM +0530, Gyan wrote:
> > >>  mux.c |    9 ++++++++-
> > >>  1 file changed, 8 insertions(+), 1 deletion(-)
> > >>d94a699f5dbc31a8ee8b7d1bdb33004d9cd95d46  0001-avformat-mux-skip-parameter-and-pts-checks-for-data-.patch
> > >> From 5ec154870d9c559037598b41736bf5b216a756e0 Mon Sep 17 00:00:00 2001
> > >>From: Gyan Doshi <ffmpeg@gyani.pro>
> > >>Date: Fri, 26 Apr 2019 18:31:33 +0530
> > >>Subject: [PATCH] avformat/mux: skip parameter and pts checks for data muxer
> > >>
> > >>Allows to dump a malformed stream for external inspection or repair.
> > >>---
> > >>  libavformat/mux.c | 9 ++++++++-
> > >>  1 file changed, 8 insertions(+), 1 deletion(-)
> > >>
> > >>diff --git a/libavformat/mux.c b/libavformat/mux.c
> > >>index 83fe1de78f..3699b572f2 100644
> > >>--- a/libavformat/mux.c
> > >>+++ b/libavformat/mux.c
> > >>@@ -290,6 +290,9 @@ FF_ENABLE_DEPRECATION_WARNINGS
> > >>          goto fail;
> > >>      }
> > >>+    if (!strcmp(of->name, "data"))
> > >>+        goto bypass;
> > >>+
> > >>      for (i = 0; i < s->nb_streams; i++) {
> > >>          st  = s->streams[i];
> > >>          par = st->codecpar;
> > >>@@ -409,6 +412,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
> > >>          av_dict_set(&s->metadata, e->key, NULL, 0);
> > >>      }
> > >>+bypass:
> > >I think this skips a bit more than what would make sense
> > >(for example priv_data allocation but thats not the only odd thing)
> > >
> > >also iam not sure this is the ideal approuch.
> > >I mean "I want to dump inavlid data in a data muxer for debug"
> > >that seems a potentially valid request for other muxers too
> > >like the image muxer producing individual files per frame and
> > >so on
> > What would be the ideal approach?
>
> I guess the ideal approuch would be to allow the developer who needs
> this to override the check when the muxer in use can actually saftely
> mux it without the specific check.
> There are for example muxers which would not function properly with
> backward going dts or something like that
>

We already have a variety of flags in place, like if a muxer doesn't
care for timestamps at all, flag it AVFMT_NOTIMESTAMPS, and have code
that checks timestamps check for this flag. Checks based on muxer
names seem generally always wrong.

- Hendrik
Gyan Doshi April 28, 2019, 10:19 a.m. UTC | #5 
On 28-04-2019 03:26 PM, Michael Niedermayer wrote:
> On Sat, Apr 27, 2019 at 10:01:53AM +0530, Gyan wrote:
>>
>> On 27-04-2019 01:32 AM, Michael Niedermayer wrote:
>>> On Fri, Apr 26, 2019 at 06:38:37PM +0530, Gyan wrote:
>>>>   mux.c |    9 ++++++++-
>>>>   1 file changed, 8 insertions(+), 1 deletion(-)
>>>> d94a699f5dbc31a8ee8b7d1bdb33004d9cd95d46  0001-avformat-mux-skip-parameter-and-pts-checks-for-data-.patch
>>>>  From 5ec154870d9c559037598b41736bf5b216a756e0 Mon Sep 17 00:00:00 2001
>>>> From: Gyan Doshi <ffmpeg@gyani.pro>
>>>> Date: Fri, 26 Apr 2019 18:31:33 +0530
>>>> Subject: [PATCH] avformat/mux: skip parameter and pts checks for data muxer
>>>>
>>>> Allows to dump a malformed stream for external inspection or repair.
>>>> ---
>>>>   libavformat/mux.c | 9 ++++++++-
>>>>   1 file changed, 8 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/libavformat/mux.c b/libavformat/mux.c
>>>> index 83fe1de78f..3699b572f2 100644
>>>> --- a/libavformat/mux.c
>>>> +++ b/libavformat/mux.c
>>>> @@ -290,6 +290,9 @@ FF_ENABLE_DEPRECATION_WARNINGS
>>>>           goto fail;
>>>>       }
>>>> +    if (!strcmp(of->name, "data"))
>>>> +        goto bypass;
>>>> +
>>>>       for (i = 0; i < s->nb_streams; i++) {
>>>>           st  = s->streams[i];
>>>>           par = st->codecpar;
>>>> @@ -409,6 +412,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
>>>>           av_dict_set(&s->metadata, e->key, NULL, 0);
>>>>       }
>>>> +bypass:
>>> I think this skips a bit more than what would make sense
>>> (for example priv_data allocation but thats not the only odd thing)
>>>
>>> also iam not sure this is the ideal approuch.
>>> I mean "I want to dump inavlid data in a data muxer for debug"
>>> that seems a potentially valid request for other muxers too
>>> like the image muxer producing individual files per frame and
>>> so on
>> What would be the ideal approach?
> I guess the ideal approuch would be to allow the developer who needs
> this to override the check when the muxer in use can actually saftely
> mux it without the specific check.
> There are for example muxers which would not function properly with
> backward going dts or something like that
The data muxer doesn't care about timestamps or any other codec 
parameter yet lavf will error out if the checks fail since its primary 
design is to cater to media-savvy containers. So I skip the checks only 
for the 'data' muxer.

Gyan
Nicolas George April 28, 2019, 10:22 a.m. UTC | #6 
Gyan (12019-04-26):
> From 5ec154870d9c559037598b41736bf5b216a756e0 Mon Sep 17 00:00:00 2001
> From: Gyan Doshi <ffmpeg@gyani.pro>
> Date: Fri, 26 Apr 2019 18:31:33 +0530
> Subject: [PATCH] avformat/mux: skip parameter and pts checks for data muxer
> 
> Allows to dump a malformed stream for external inspection or repair.

What is your exact use case? I think there are much simpler and much
more robust solutions.

Regards,
Gyan Doshi April 28, 2019, 10:36 a.m. UTC | #7 
On 28-04-2019 03:40 PM, Hendrik Leppkes wrote:
> On Sun, Apr 28, 2019 at 11:57 AM Michael Niedermayer
> <michael@niedermayer.cc> wrote:
>> On Sat, Apr 27, 2019 at 10:01:53AM +0530, Gyan wrote:
>>>
>>> On 27-04-2019 01:32 AM, Michael Niedermayer wrote:
>>>> On Fri, Apr 26, 2019 at 06:38:37PM +0530, Gyan wrote:
>>>>>   mux.c |    9 ++++++++-
>>>>>   1 file changed, 8 insertions(+), 1 deletion(-)
>>>>> d94a699f5dbc31a8ee8b7d1bdb33004d9cd95d46  0001-avformat-mux-skip-parameter-and-pts-checks-for-data-.patch
>>>>>  From 5ec154870d9c559037598b41736bf5b216a756e0 Mon Sep 17 00:00:00 2001
>>>>> From: Gyan Doshi <ffmpeg@gyani.pro>
>>>>> Date: Fri, 26 Apr 2019 18:31:33 +0530
>>>>> Subject: [PATCH] avformat/mux: skip parameter and pts checks for data muxer
>>>>>
>>>>> Allows to dump a malformed stream for external inspection or repair.
>>>>> ---
>>>>>   libavformat/mux.c | 9 ++++++++-
>>>>>   1 file changed, 8 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/libavformat/mux.c b/libavformat/mux.c
>>>>> index 83fe1de78f..3699b572f2 100644
>>>>> --- a/libavformat/mux.c
>>>>> +++ b/libavformat/mux.c
>>>>> @@ -290,6 +290,9 @@ FF_ENABLE_DEPRECATION_WARNINGS
>>>>>           goto fail;
>>>>>       }
>>>>> +    if (!strcmp(of->name, "data"))
>>>>> +        goto bypass;
>>>>> +
>>>>>       for (i = 0; i < s->nb_streams; i++) {
>>>>>           st  = s->streams[i];
>>>>>           par = st->codecpar;
>>>>> @@ -409,6 +412,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
>>>>>           av_dict_set(&s->metadata, e->key, NULL, 0);
>>>>>       }
>>>>> +bypass:
>>>> I think this skips a bit more than what would make sense
>>>> (for example priv_data allocation but thats not the only odd thing)
>>>>
>>>> also iam not sure this is the ideal approuch.
>>>> I mean "I want to dump inavlid data in a data muxer for debug"
>>>> that seems a potentially valid request for other muxers too
>>>> like the image muxer producing individual files per frame and
>>>> so on
>>> What would be the ideal approach?
>> I guess the ideal approuch would be to allow the developer who needs
>> this to override the check when the muxer in use can actually saftely
>> mux it without the specific check.
>> There are for example muxers which would not function properly with
>> backward going dts or something like that
>>
> We already have a variety of flags in place, like if a muxer doesn't
> care for timestamps at all, flag it AVFMT_NOTIMESTAMPS, and have code
> that checks timestamps check for this flag. Checks based on muxer
> names seem generally always wrong.
AVFMT_NOTIMESTAMPS is somewhat overloaded. There are formats which don't 
_write_ timestamps but still have some code which looks at them e.g. 
image2 muxer or the mxf family of muxers. Some lavf code like 
compute_muxer_pkt_fields is still run even for formats with the noTS 
flag.  lavf simply ignores errors for formats with the flag set. 
However, if the dumped stream is audio with no  sample rate set, then 
init_pts will fail and so would compute. Hence the selection of muxer by 
name.

Gyan
Gyan Doshi April 28, 2019, 10:39 a.m. UTC | #8 
On 28-04-2019 03:52 PM, Nicolas George wrote:
> Gyan (12019-04-26):
>>  From 5ec154870d9c559037598b41736bf5b216a756e0 Mon Sep 17 00:00:00 2001
>> From: Gyan Doshi <ffmpeg@gyani.pro>
>> Date: Fri, 26 Apr 2019 18:31:33 +0530
>> Subject: [PATCH] avformat/mux: skip parameter and pts checks for data muxer
>>
>> Allows to dump a malformed stream for external inspection or repair.
> What is your exact use case? I think there are much simpler and much
> more robust solutions.
Corrupt streams in sufficiently intact containers (MP4, TS) so they can 
be demuxed but decoder context fields are incomplete/invalid, so ffmpeg 
won't streamcopy-mux them.

Depending on the exact situation, I would use a repair or analysis tool 
to check them or supply an alternate esds..etc

Gyan
Nicolas George April 28, 2019, 10:45 a.m. UTC | #9 
Gyan (12019-04-28):
> Corrupt streams in sufficiently intact containers (MP4, TS) so they can be
> demuxed but decoder context fields are incomplete/invalid, so ffmpeg won't
> streamcopy-mux them.
> 
> Depending on the exact situation, I would use a repair or analysis tool to
> check them or supply an alternate esds..etc

And you want to just dump the packets payload in a file? With the ffmpeg
command-line?

Then I suggest to implement that as a ffmpeg option:

	ffmpeg -dump_stream:0 stream0.bin -i damaged.mp4 -f null -

It is not efficient nor robust to have the packets go through all
ffmpeg's and libavformat processing only to have an option to disable
that processing.

Regards,
Paul B Mahol April 28, 2019, 10:54 a.m. UTC | #10 
On 4/28/19, Gyan <ffmpeg@gyani.pro> wrote:
>
>
> On 28-04-2019 03:52 PM, Nicolas George wrote:
>> Gyan (12019-04-26):
>>>  From 5ec154870d9c559037598b41736bf5b216a756e0 Mon Sep 17 00:00:00 2001
>>> From: Gyan Doshi <ffmpeg@gyani.pro>
>>> Date: Fri, 26 Apr 2019 18:31:33 +0530
>>> Subject: [PATCH] avformat/mux: skip parameter and pts checks for data
>>> muxer
>>>
>>> Allows to dump a malformed stream for external inspection or repair.
>> What is your exact use case? I think there are much simpler and much
>> more robust solutions.
> Corrupt streams in sufficiently intact containers (MP4, TS) so they can
> be demuxed but decoder context fields are incomplete/invalid, so ffmpeg
> won't streamcopy-mux them.
>
> Depending on the exact situation, I would use a repair or analysis tool
> to check them or supply an alternate esds..etc
>

If this just dumps demuxed packets use:

ffmpeg -i input -c:v copy -f rawvideo outv.raw
ffmpeg -i input -c:a copy -f u8 outa.raw
Gyan Doshi April 28, 2019, 1:06 p.m. UTC | #11 
On 28-04-2019 04:15 PM, Nicolas George wrote:
> Gyan (12019-04-28):
>> Corrupt streams in sufficiently intact containers (MP4, TS) so they can be
>> demuxed but decoder context fields are incomplete/invalid, so ffmpeg won't
>> streamcopy-mux them.
>>
>> Depending on the exact situation, I would use a repair or analysis tool to
>> check them or supply an alternate esds..etc
> And you want to just dump the packets payload in a file? With the ffmpeg
> command-line?
>
> Then I suggest to implement that as a ffmpeg option:
>
> 	ffmpeg -dump_stream:0 stream0.bin -i damaged.mp4 -f null -
I'll try this but I don't think the command as presented will work because

a) there's no codec option set, so ffmpeg has to decode frames and that 
will fail for damaged streams. But this can be quickly remedied by 
adding -c copy

b) ffmpeg will call avformat_write_header for the output, which will 
likely fail because of the aforementioned codec parameter issues. When 
that happens, ffmpeg will exit and only the packets demuxed and flushed 
up to that point will be present in the dumped file. This could possibly 
be averted by providing a valid dummy input and mapping only that to the 
output. However, I believe ffmpeg will then exit when that output is 
marked as finished, but the dumped stream may not be because the main 
transcode() loop isn't sensitive to it.

The pipeline for dumping damaged streams is identical to dumping valid 
streams except for the part when timestamps are regulated and codec 
fields validated for the target muxer, which is what my patch disables 
for a single muxer that doesn't need them. I don't see much (any?) 
"wasted" processing with my patch, relative to streamcopying a valid stream.

Gyan
Nicolas George May 2, 2019, 9:53 a.m. UTC | #12 
Gyan (12019-04-28):
> b) ffmpeg will call avformat_write_header for the output, which will likely
> fail because of the aforementioned codec parameter issues.

Are you sure? It looks to me like avformat_write_header() does not
perform checks by itself, and null does not either.

> The pipeline for dumping damaged streams is identical to dumping valid
> streams except for the part when timestamps are regulated and codec fields
> validated for the target muxer, which is what my patch disables for a single
> muxer that doesn't need them. I don't see much (any?) "wasted" processing
> with my patch, relative to streamcopying a valid stream.

The problem is not about waste as a performance issue, the problem is
about complexity as a maintenance issue. Exceptions are expensive in
that regard.

Regards,
Gyan Doshi May 2, 2019, 10:23 a.m. UTC | #13 
On 02-05-2019 03:23 PM, Nicolas George wrote:
> Gyan (12019-04-28):
>> b) ffmpeg will call avformat_write_header for the output, which will likely
>> fail because of the aforementioned codec parameter issues.
> Are you sure? It looks to me like avformat_write_header() does not
> perform checks by itself, and null does not either.

The flow is

avformat_write_header -> avformat_init_output -> init_muxer

And in the last function, lines 293-385 (as of 7eba26451) carry out the 
checks, and which my patch skips.

>> The pipeline for dumping damaged streams is identical to dumping valid
>> streams except for the part when timestamps are regulated and codec fields
>> validated for the target muxer, which is what my patch disables for a single
>> muxer that doesn't need them. I don't see much (any?) "wasted" processing
>> with my patch, relative to streamcopying a valid stream.
> The problem is not about waste as a performance issue, the problem is
> about complexity as a maintenance issue. Exceptions are expensive in
> that regard.

The problem is that there is no clean set of flags which isolate the 
parameter-agnostic muxers - not surprising as this is a library for 
making valid media files. A new flag could be invented, if the present 
patch is too idiosyncratic. On a related note, as I mentioned elsewhere 
in this thread, an existing flag AVFMT_NOTIMESTAMPS, which in theory I 
could use, is of no help here, as it has been set for muxers which do 
handle timestamps in some way.

Gyan
Nicolas George May 4, 2019, 1:12 p.m. UTC | #14 
Gyan (12019-05-02):
> The flow is
> 
> avformat_write_header -> avformat_init_output -> init_muxer
> 
> And in the last function, lines 293-385 (as of 7eba26451) carry out the
> checks, and which my patch skips.

I missed this. My bad.

> The problem is that there is no clean set of flags which isolate the
> parameter-agnostic muxers - not surprising as this is a library for making
> valid media files. A new flag could be invented, if the present patch is too
> idiosyncratic. On a related note, as I mentioned elsewhere in this thread,
> an existing flag AVFMT_NOTIMESTAMPS, which in theory I could use, is of no
> help here, as it has been set for muxers which do handle timestamps in some
> way.

Yet, the name test is wrong: there are other muxers that absolutely do
not care about timestamps, or any other kind of parameters. The null
muxer, for starter.

I see a few options to make this work properly:

- Add an option "-noout" for ffmpeg to let it run without output, until
  the end of inputs. This would be useful for other situations,
  including cases when the log from filters is what the user is
  interested in. It would replace "-f null -" that we always use.

- Add an option "-mux_params_check 0" for ffmpeg output streams, tu
  allow skipping these tests explicitly. Your use case is specific
  enough that an additional option is acceptable.

- Create the required flags to mark muxers that do not require these
  tests.

Regards,
Gyan Doshi May 6, 2019, 4:28 a.m. UTC | #15 
On 04-05-2019 06:42 PM, Nicolas George wrote:
> Gyan (12019-05-02):
>> The flow is
>>
>> avformat_write_header -> avformat_init_output -> init_muxer
>>
>> And in the last function, lines 293-385 (as of 7eba26451) carry out the
>> checks, and which my patch skips.
> I missed this. My bad.
>
>> The problem is that there is no clean set of flags which isolate the
>> parameter-agnostic muxers - not surprising as this is a library for making
>> valid media files. A new flag could be invented, if the present patch is too
>> idiosyncratic. On a related note, as I mentioned elsewhere in this thread,
>> an existing flag AVFMT_NOTIMESTAMPS, which in theory I could use, is of no
>> help here, as it has been set for muxers which do handle timestamps in some
>> way.
> Yet, the name test is wrong: there are other muxers that absolutely do
> not care about timestamps, or any other kind of parameters. The null
> muxer, for starter.
>
> I see a few options to make this work properly:
>
> - Add an option "-noout" for ffmpeg to let it run without output, until
>    the end of inputs. This would be useful for other situations,
>    including cases when the log from filters is what the user is
>    interested in. It would replace "-f null -" that we always use.
>
> - Add an option "-mux_params_check 0" for ffmpeg output streams, tu
>    allow skipping these tests explicitly. Your use case is specific
>    enough that an additional option is acceptable.
>
> - Create the required flags to mark muxers that do not require these
>    tests.
I think what I would prefer is a combination of 2 and 3. Mark muxers 
which don't need checks but add an option to actually disable checks. 2 
alone can produce invalid files if used with a regular muxer. At 
minimum, a warning should be issued. 3 alone has no flexibility.

If acceptable, I'll revise the patch that way.

Thanks,
Gyan
diff mbox

Patch

diff --git a/libavformat/mux.c b/libavformat/mux.c
index 83fe1de78f..3699b572f2 100644
--- a/libavformat/mux.c
+++ b/libavformat/mux.c
@@ -290,6 +290,9 @@  FF_ENABLE_DEPRECATION_WARNINGS
         goto fail;
     }
 
+    if (!strcmp(of->name, "data"))
+        goto bypass;
+
     for (i = 0; i < s->nb_streams; i++) {
         st  = s->streams[i];
         par = st->codecpar;
@@ -409,6 +412,7 @@  FF_ENABLE_DEPRECATION_WARNINGS
         av_dict_set(&s->metadata, e->key, NULL, 0);
     }
 
+bypass:
     if (options) {
          av_dict_free(options);
          *options = tmp;
@@ -528,7 +532,7 @@  int avformat_write_header(AVFormatContext *s, AVDictionary **options)
     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 (!s->internal->streams_initialized && strcmp(s->oformat->name, "data")) {
         if ((ret = init_pts(s)) < 0)
             goto fail;
     }
@@ -559,6 +563,9 @@  static int compute_muxer_pkt_fields(AVFormatContext *s, AVStream *st, AVPacket *
     int num, den, i;
     int frame_size;
 
+    if (!strcmp(s->oformat->name, "data"))
+        return 0;
+
     if (!s->internal->missing_ts_warning &&
         !(s->oformat->flags & AVFMT_NOTIMESTAMPS) &&
         (!(st->disposition & AV_DISPOSITION_ATTACHED_PIC) || (st->disposition & AV_DISPOSITION_TIMED_THUMBNAILS)) &&