[FFmpeg-devel] ffmpeg: always init output stream before reaping filters

Submitted by Marton Balint on Oct. 3, 2017, 10:48 p.m.

Details

Message ID 20171003224823.4888-1-cus@passwd.hu
State Accepted
Commit f4090940bd3024e69d236257d327f11d1e496229
Headers show

Commit Message

Marton Balint Oct. 3, 2017, 10:48 p.m.
Otherwise the frame size of the codec is not set in the buffersink.

Fixes ticket #6603 and the following simpler case:

ffmpeg -c aac -filter_complex "sine=d=0.1,asetnsamples=1025" out.aac

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 fftools/ffmpeg.c | 9 +++++++++
 1 file changed, 9 insertions(+)

Comments

Marton Balint Oct. 6, 2017, 6:39 p.m.
On Wed, 4 Oct 2017, Marton Balint wrote:

> Otherwise the frame size of the codec is not set in the buffersink.
>
> Fixes ticket #6603 and the following simpler case:
>
> ffmpeg -c aac -filter_complex "sine=d=0.1,asetnsamples=1025" out.aac
>
> Signed-off-by: Marton Balint <cus@passwd.hu>

As this patch fixes a regression, this should also be in 3.4, so ping for 
this.

Thanks,
Marton

> ---
> fftools/ffmpeg.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
> index 1d248bc269..5be8788ea8 100644
> --- a/fftools/ffmpeg.c
> +++ b/fftools/ffmpeg.c
> @@ -4528,6 +4528,15 @@ static int transcode_step(void)
>     }
>
>     if (ost->filter && ost->filter->graph->graph) {
> +        if (!ost->initialized) {
> +            char error[1024] = {0};
> +            ret = init_output_stream(ost, error, sizeof(error));
> +            if (ret < 0) {
> +                av_log(NULL, AV_LOG_ERROR, "Error initializing output stream %d:%d -- %s\n",
> +                       ost->file_index, ost->index, error);
> +                exit_program(1);
> +            }
> +        }
>         if ((ret = transcode_from_filter(ost->filter->graph, &ist)) < 0)
>             return ret;
>         if (!ist)
> -- 
> 2.13.5
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Paul B Mahol Oct. 7, 2017, 8:12 a.m.
On 10/6/17, Marton Balint <cus@passwd.hu> wrote:
>
>
> On Wed, 4 Oct 2017, Marton Balint wrote:
>
>> Otherwise the frame size of the codec is not set in the buffersink.
>>
>> Fixes ticket #6603 and the following simpler case:
>>
>> ffmpeg -c aac -filter_complex "sine=d=0.1,asetnsamples=1025" out.aac
>>
>> Signed-off-by: Marton Balint <cus@passwd.hu>
>
> As this patch fixes a regression, this should also be in 3.4, so ping for
> this.
>
> Thanks,
> Marton
>
>> ---
>> fftools/ffmpeg.c | 9 +++++++++
>> 1 file changed, 9 insertions(+)
>>
>> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
>> index 1d248bc269..5be8788ea8 100644
>> --- a/fftools/ffmpeg.c
>> +++ b/fftools/ffmpeg.c
>> @@ -4528,6 +4528,15 @@ static int transcode_step(void)
>>     }
>>
>>     if (ost->filter && ost->filter->graph->graph) {
>> +        if (!ost->initialized) {
>> +            char error[1024] = {0};
>> +            ret = init_output_stream(ost, error, sizeof(error));
>> +            if (ret < 0) {
>> +                av_log(NULL, AV_LOG_ERROR, "Error initializing output
>> stream %d:%d -- %s\n",
>> +                       ost->file_index, ost->index, error);
>> +                exit_program(1);
>> +            }
>> +        }
>>         if ((ret = transcode_from_filter(ost->filter->graph, &ist)) < 0)
>>             return ret;
>>         if (!ist)
>> --
>> 2.13.5
>>
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

lgtm
Hendrik Leppkes Oct. 7, 2017, 8:20 a.m.
On Wed, Oct 4, 2017 at 12:48 AM, Marton Balint <cus@passwd.hu> wrote:
> Otherwise the frame size of the codec is not set in the buffersink.
>
> Fixes ticket #6603 and the following simpler case:
>
> ffmpeg -c aac -filter_complex "sine=d=0.1,asetnsamples=1025" out.aac
>
> Signed-off-by: Marton Balint <cus@passwd.hu>
> ---
>  fftools/ffmpeg.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
> index 1d248bc269..5be8788ea8 100644
> --- a/fftools/ffmpeg.c
> +++ b/fftools/ffmpeg.c
> @@ -4528,6 +4528,15 @@ static int transcode_step(void)
>      }
>
>      if (ost->filter && ost->filter->graph->graph) {
> +        if (!ost->initialized) {
> +            char error[1024] = {0};
> +            ret = init_output_stream(ost, error, sizeof(error));
> +            if (ret < 0) {
> +                av_log(NULL, AV_LOG_ERROR, "Error initializing output stream %d:%d -- %s\n",
> +                       ost->file_index, ost->index, error);
> +                exit_program(1);
> +            }
> +        }
>          if ((ret = transcode_from_filter(ost->filter->graph, &ist)) < 0)
>              return ret;
>          if (!ist)

Doesn't this basically void all the work that was done to get proper
output frame information before initializing the output, so its
created with the proper info at all times?

- Hendrik
Marton Balint Oct. 8, 2017, 11:14 a.m.
On Sat, 7 Oct 2017, Hendrik Leppkes wrote:

> On Wed, Oct 4, 2017 at 12:48 AM, Marton Balint <cus@passwd.hu> wrote:
>> Otherwise the frame size of the codec is not set in the buffersink.
>>
>> Fixes ticket #6603 and the following simpler case:
>>
>> ffmpeg -c aac -filter_complex "sine=d=0.1,asetnsamples=1025" out.aac
>>
>> Signed-off-by: Marton Balint <cus@passwd.hu>
>> ---
>>  fftools/ffmpeg.c | 9 +++++++++
>>  1 file changed, 9 insertions(+)
>>
>> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
>> index 1d248bc269..5be8788ea8 100644
>> --- a/fftools/ffmpeg.c
>> +++ b/fftools/ffmpeg.c
>> @@ -4528,6 +4528,15 @@ static int transcode_step(void)
>>      }
>>
>>      if (ost->filter && ost->filter->graph->graph) {
>> +        if (!ost->initialized) {
>> +            char error[1024] = {0};
>> +            ret = init_output_stream(ost, error, sizeof(error));
>> +            if (ret < 0) {
>> +                av_log(NULL, AV_LOG_ERROR, "Error initializing output stream %d:%d -- %s\n",
>> +                       ost->file_index, ost->index, error);
>> +                exit_program(1);
>> +            }
>> +        }
>>          if ((ret = transcode_from_filter(ost->filter->graph, &ist)) < 0)
>>              return ret;
>>          if (!ist)
>
> Doesn't this basically void all the work that was done to get proper
> output frame information before initializing the output, so its
> created with the proper info at all times?

To be frank, I am not entirely sure. Fate passes, so this can't be that 
bad. The original patch that caused the regression is 
af1761f7b5b1b72197dc40934953b775c2d951cc which in the commit message only 
mentions the delayed initialization of filters, this patch keeps that.

Regards,
Marton
Marton Balint Oct. 12, 2017, 10:11 a.m.
On Sun, 8 Oct 2017, Marton Balint wrote:

>
>
> On Sat, 7 Oct 2017, Hendrik Leppkes wrote:
>
>> On Wed, Oct 4, 2017 at 12:48 AM, Marton Balint <cus@passwd.hu> wrote:
>>> Otherwise the frame size of the codec is not set in the buffersink.
>>>
>>> Fixes ticket #6603 and the following simpler case:
>>>
>>> ffmpeg -c aac -filter_complex "sine=d=0.1,asetnsamples=1025" out.aac
>>>
>>> Signed-off-by: Marton Balint <cus@passwd.hu>
>>> ---
>>>  fftools/ffmpeg.c | 9 +++++++++
>>>  1 file changed, 9 insertions(+)
>>>
>>> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
>>> index 1d248bc269..5be8788ea8 100644
>>> --- a/fftools/ffmpeg.c
>>> +++ b/fftools/ffmpeg.c
>>> @@ -4528,6 +4528,15 @@ static int transcode_step(void)
>>>      }
>>>
>>>      if (ost->filter && ost->filter->graph->graph) {
>>> +        if (!ost->initialized) {
>>> +            char error[1024] = {0};
>>> +            ret = init_output_stream(ost, error, sizeof(error));
>>> +            if (ret < 0) {
>>> +                av_log(NULL, AV_LOG_ERROR, "Error initializing output 
> stream %d:%d -- %s\n",
>>> +                       ost->file_index, ost->index, error);
>>> +                exit_program(1);
>>> +            }
>>> +        }
>>>          if ((ret = transcode_from_filter(ost->filter->graph, &ist)) < 0)
>>>              return ret;
>>>          if (!ist)
>>
>> Doesn't this basically void all the work that was done to get proper
>> output frame information before initializing the output, so its
>> created with the proper info at all times?
>
> To be frank, I am not entirely sure. Fate passes, so this can't be that 
> bad. The original patch that caused the regression is 
> af1761f7b5b1b72197dc40934953b775c2d951cc which in the commit message only 
> mentions the delayed initialization of filters, this patch keeps that.
>

Will push this soon.

Regards,
Marton
Marton Balint Oct. 18, 2017, 8:06 p.m.
On Thu, 12 Oct 2017, Marton Balint wrote:

>
>
> On Sun, 8 Oct 2017, Marton Balint wrote:
>
>>
>>
>> On Sat, 7 Oct 2017, Hendrik Leppkes wrote:
>>
>>> On Wed, Oct 4, 2017 at 12:48 AM, Marton Balint <cus@passwd.hu> wrote:
>>>> Otherwise the frame size of the codec is not set in the buffersink.
>>>>
>>>> Fixes ticket #6603 and the following simpler case:
>>>>
>>>> ffmpeg -c aac -filter_complex "sine=d=0.1,asetnsamples=1025" out.aac
>>>>
>>>> Signed-off-by: Marton Balint <cus@passwd.hu>
>>>> ---
>>>>  fftools/ffmpeg.c | 9 +++++++++
>>>>  1 file changed, 9 insertions(+)
>>>>
>>>> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
>>>> index 1d248bc269..5be8788ea8 100644
>>>> --- a/fftools/ffmpeg.c
>>>> +++ b/fftools/ffmpeg.c
>>>> @@ -4528,6 +4528,15 @@ static int transcode_step(void)
>>>>      }
>>>>
>>>>      if (ost->filter && ost->filter->graph->graph) {
>>>> +        if (!ost->initialized) {
>>>> +            char error[1024] = {0};
>>>> +            ret = init_output_stream(ost, error, sizeof(error));
>>>> +            if (ret < 0) {
>>>> +                av_log(NULL, AV_LOG_ERROR, "Error initializing output 
>> stream %d:%d -- %s\n",
>>>> +                       ost->file_index, ost->index, error);
>>>> +                exit_program(1);
>>>> +            }
>>>> +        }
>>>>          if ((ret = transcode_from_filter(ost->filter->graph, &ist)) < 0)
>>>>              return ret;
>>>>          if (!ist)
>>>
>>> Doesn't this basically void all the work that was done to get proper
>>> output frame information before initializing the output, so its
>>> created with the proper info at all times?
>>
>> To be frank, I am not entirely sure. Fate passes, so this can't be that 
>> bad. The original patch that caused the regression is 
>> af1761f7b5b1b72197dc40934953b775c2d951cc which in the commit message only 
>> mentions the delayed initialization of filters, this patch keeps that.
>>
>
> Will push this soon.
>

Pushed. Might make sense to backport this before making 3.4.1 if it does 
not cause any issues in git master until then.

Regards,
Marton

Patch hide | download patch | download mbox

diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index 1d248bc269..5be8788ea8 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -4528,6 +4528,15 @@  static int transcode_step(void)
     }
 
     if (ost->filter && ost->filter->graph->graph) {
+        if (!ost->initialized) {
+            char error[1024] = {0};
+            ret = init_output_stream(ost, error, sizeof(error));
+            if (ret < 0) {
+                av_log(NULL, AV_LOG_ERROR, "Error initializing output stream %d:%d -- %s\n",
+                       ost->file_index, ost->index, error);
+                exit_program(1);
+            }
+        }
         if ((ret = transcode_from_filter(ost->filter->graph, &ist)) < 0)
             return ret;
         if (!ist)