* [FFmpeg-devel] [PATCH] ffprobe: restore reporting error code for failed inputs
@ 2022-08-15 18:34 Gyan Doshi
2022-08-16 23:06 ` Stefano Sabatini
0 siblings, 1 reply; 6+ messages in thread
From: Gyan Doshi @ 2022-08-15 18:34 UTC (permalink / raw)
To: ffmpeg-devel
c11fb46731 led to a regression whereby the return code for missing
input or input probe is overridden by writer close return code and
hence not conveyed in the exit code.
---
fftools/ffprobe.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
Affects 5.1 so will need to be backported there.
diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c
index ad633ccc44..8983dc28cc 100644
--- a/fftools/ffprobe.c
+++ b/fftools/ffprobe.c
@@ -4032,7 +4032,7 @@ int main(int argc, char **argv)
WriterContext *wctx;
char *buf;
char *w_name = NULL, *w_args = NULL;
- int ret, i;
+ int ret, input_ret, i;
init_dynload();
@@ -4156,10 +4156,14 @@ int main(int argc, char **argv)
show_error(wctx, ret);
}
+ input_ret = ret;
+
writer_print_section_footer(wctx);
ret = writer_close(&wctx);
if (ret < 0)
av_log(NULL, AV_LOG_ERROR, "Writing output failed: %s\n", av_err2str(ret));
+
+ ret = FFMIN(ret, input_ret);
}
end:
--
2.36.1
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [FFmpeg-devel] [PATCH] ffprobe: restore reporting error code for failed inputs
2022-08-15 18:34 [FFmpeg-devel] [PATCH] ffprobe: restore reporting error code for failed inputs Gyan Doshi
@ 2022-08-16 23:06 ` Stefano Sabatini
2022-08-17 4:12 ` Gyan Doshi
0 siblings, 1 reply; 6+ messages in thread
From: Stefano Sabatini @ 2022-08-16 23:06 UTC (permalink / raw)
To: FFmpeg development discussions and patches
On date Tuesday 2022-08-16 00:04:10 +0530, Gyan Doshi wrote:
> c11fb46731 led to a regression whereby the return code for missing
> input or input probe is overridden by writer close return code and
> hence not conveyed in the exit code.
> ---
> fftools/ffprobe.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> Affects 5.1 so will need to be backported there.
>
> diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c
> index ad633ccc44..8983dc28cc 100644
> --- a/fftools/ffprobe.c
> +++ b/fftools/ffprobe.c
> @@ -4032,7 +4032,7 @@ int main(int argc, char **argv)
> WriterContext *wctx;
> char *buf;
> char *w_name = NULL, *w_args = NULL;
> - int ret, i;
> + int ret, input_ret, i;
>
> init_dynload();
>
> @@ -4156,10 +4156,14 @@ int main(int argc, char **argv)
> show_error(wctx, ret);
> }
>
> + input_ret = ret;
> +
> writer_print_section_footer(wctx);
> ret = writer_close(&wctx);
> if (ret < 0)
> av_log(NULL, AV_LOG_ERROR, "Writing output failed: %s\n", av_err2str(ret));
> +
> + ret = FFMIN(ret, input_ret);
maybe we should give priority to input_ret in case they are both
negative?
return input_ret < 0 ? input_ret : ret;
?
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [FFmpeg-devel] [PATCH] ffprobe: restore reporting error code for failed inputs
2022-08-16 23:06 ` Stefano Sabatini
@ 2022-08-17 4:12 ` Gyan Doshi
2022-08-17 7:07 ` Marton Balint
0 siblings, 1 reply; 6+ messages in thread
From: Gyan Doshi @ 2022-08-17 4:12 UTC (permalink / raw)
To: ffmpeg-devel
On 2022-08-17 04:36 am, Stefano Sabatini wrote:
> On date Tuesday 2022-08-16 00:04:10 +0530, Gyan Doshi wrote:
>> c11fb46731 led to a regression whereby the return code for missing
>> input or input probe is overridden by writer close return code and
>> hence not conveyed in the exit code.
>> ---
>> fftools/ffprobe.c | 6 +++++-
>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> Affects 5.1 so will need to be backported there.
>>
>> diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c
>> index ad633ccc44..8983dc28cc 100644
>> --- a/fftools/ffprobe.c
>> +++ b/fftools/ffprobe.c
>> @@ -4032,7 +4032,7 @@ int main(int argc, char **argv)
>> WriterContext *wctx;
>> char *buf;
>> char *w_name = NULL, *w_args = NULL;
>> - int ret, i;
>> + int ret, input_ret, i;
>>
>> init_dynload();
>>
>> @@ -4156,10 +4156,14 @@ int main(int argc, char **argv)
>> show_error(wctx, ret);
>> }
>>
>> + input_ret = ret;
>> +
>> writer_print_section_footer(wctx);
>> ret = writer_close(&wctx);
>> if (ret < 0)
>> av_log(NULL, AV_LOG_ERROR, "Writing output failed: %s\n", av_err2str(ret));
>> +
>> + ret = FFMIN(ret, input_ret);
> maybe we should give priority to input_ret in case they are both
> negative?
>
> return input_ret < 0 ? input_ret : ret;
Scripts usually depend on exit code being 0 or something else. Also,
error is logged for both input failure and writer_close failure, so it
doesn't matter.
Finally, input_ret is not initialized if writer_open fails, so we
shouldn't be referencing it outside.
Regards,
Gyan
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [FFmpeg-devel] [PATCH] ffprobe: restore reporting error code for failed inputs
2022-08-17 4:12 ` Gyan Doshi
@ 2022-08-17 7:07 ` Marton Balint
2022-08-17 7:13 ` Gyan Doshi
0 siblings, 1 reply; 6+ messages in thread
From: Marton Balint @ 2022-08-17 7:07 UTC (permalink / raw)
To: FFmpeg development discussions and patches
On Wed, 17 Aug 2022, Gyan Doshi wrote:
>
>
> On 2022-08-17 04:36 am, Stefano Sabatini wrote:
>> On date Tuesday 2022-08-16 00:04:10 +0530, Gyan Doshi wrote:
>>> c11fb46731 led to a regression whereby the return code for missing
>>> input or input probe is overridden by writer close return code and
>>> hence not conveyed in the exit code.
>>> ---
>>> fftools/ffprobe.c | 6 +++++-
>>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> Affects 5.1 so will need to be backported there.
>>>
>>> diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c
>>> index ad633ccc44..8983dc28cc 100644
>>> --- a/fftools/ffprobe.c
>>> +++ b/fftools/ffprobe.c
>>> @@ -4032,7 +4032,7 @@ int main(int argc, char **argv)
>>> WriterContext *wctx;
>>> char *buf;
>>> char *w_name = NULL, *w_args = NULL;
>>> - int ret, i;
>>> + int ret, input_ret, i;
>>>
>>> init_dynload();
>>>
>>> @@ -4156,10 +4156,14 @@ int main(int argc, char **argv)
>>> show_error(wctx, ret);
>>> }
>>> + input_ret = ret;
>>> +
>>> writer_print_section_footer(wctx);
>>> ret = writer_close(&wctx);
>>> if (ret < 0)
>>> av_log(NULL, AV_LOG_ERROR, "Writing output failed: %s\n",
>>> av_err2str(ret));
>>> +
>>> + ret = FFMIN(ret, input_ret);
>> maybe we should give priority to input_ret in case they are both
>> negative?
>>
>> return input_ret < 0 ? input_ret : ret;
>
> Scripts usually depend on exit code being 0 or something else. Also, error is
> logged for both input failure and writer_close failure, so it doesn't matter.
> Finally, input_ret is not initialized if writer_open fails, so we shouldn't
> be referencing it outside.
I would do something like
ret2 = writer_close();
ret = FFMIN(ret2, ret);
But fine either way, please push any version you prefer, this has been
broken for too long. Sorry about that.
Thanks,
Marton
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [FFmpeg-devel] [PATCH] ffprobe: restore reporting error code for failed inputs
2022-08-17 7:07 ` Marton Balint
@ 2022-08-17 7:13 ` Gyan Doshi
2022-08-17 11:19 ` Gyan Doshi
0 siblings, 1 reply; 6+ messages in thread
From: Gyan Doshi @ 2022-08-17 7:13 UTC (permalink / raw)
To: ffmpeg-devel
On 2022-08-17 12:37 pm, Marton Balint wrote:
>
>
> On Wed, 17 Aug 2022, Gyan Doshi wrote:
>
>>
>>
>> On 2022-08-17 04:36 am, Stefano Sabatini wrote:
>>> On date Tuesday 2022-08-16 00:04:10 +0530, Gyan Doshi wrote:
>>>> c11fb46731 led to a regression whereby the return code for missing
>>>> input or input probe is overridden by writer close return code and
>>>> hence not conveyed in the exit code.
>>>> ---
>>>> fftools/ffprobe.c | 6 +++++-
>>>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>>>
>>>> Affects 5.1 so will need to be backported there.
>>>>
>>>> diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c
>>>> index ad633ccc44..8983dc28cc 100644
>>>> --- a/fftools/ffprobe.c
>>>> +++ b/fftools/ffprobe.c
>>>> @@ -4032,7 +4032,7 @@ int main(int argc, char **argv)
>>>> WriterContext *wctx;
>>>> char *buf;
>>>> char *w_name = NULL, *w_args = NULL;
>>>> - int ret, i;
>>>> + int ret, input_ret, i;
>>>>
>>>> init_dynload();
>>>>
>>>> @@ -4156,10 +4156,14 @@ int main(int argc, char **argv)
>>>> show_error(wctx, ret);
>>>> }
>>>> + input_ret = ret;
>>>> +
>>>> writer_print_section_footer(wctx);
>>>> ret = writer_close(&wctx);
>>>> if (ret < 0)
>>>> av_log(NULL, AV_LOG_ERROR, "Writing output failed:
>>>> %s\n",
>>>> av_err2str(ret));
>>>> +
>>>> + ret = FFMIN(ret, input_ret);
>>> maybe we should give priority to input_ret in case they are both
>>> negative?
>>>
>>> return input_ret < 0 ? input_ret : ret;
>>
>> Scripts usually depend on exit code being 0 or something else. Also,
>> error is logged for both input failure and writer_close failure, so
>> it doesn't matter.
>> Finally, input_ret is not initialized if writer_open fails, so we
>> shouldn't be referencing it outside.
>
> I would do something like
>
> ret2 = writer_close();
> ret = FFMIN(ret2, ret);
>
> But fine either way, please push any version you prefer, this has been
> broken for too long. Sorry about that.
The only diff is which return does the new variable store. I like my var
name better :/
Will push in a couple of hours, 5.1 as well
Regards,
Gyan
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [FFmpeg-devel] [PATCH] ffprobe: restore reporting error code for failed inputs
2022-08-17 7:13 ` Gyan Doshi
@ 2022-08-17 11:19 ` Gyan Doshi
0 siblings, 0 replies; 6+ messages in thread
From: Gyan Doshi @ 2022-08-17 11:19 UTC (permalink / raw)
To: ffmpeg-devel
Pushed as
d5544f6457ad06987311beda0e4b7c08bf52915c to master
and
4e4cc6e56a899f6b4302e80dbcd6b4462f340905 to 5.1
On 2022-08-17 12:43 pm, Gyan Doshi wrote:
>
>
> On 2022-08-17 12:37 pm, Marton Balint wrote:
>>
>>
>> On Wed, 17 Aug 2022, Gyan Doshi wrote:
>>
>>>
>>>
>>> On 2022-08-17 04:36 am, Stefano Sabatini wrote:
>>>> On date Tuesday 2022-08-16 00:04:10 +0530, Gyan Doshi wrote:
>>>>> c11fb46731 led to a regression whereby the return code for missing
>>>>> input or input probe is overridden by writer close return code and
>>>>> hence not conveyed in the exit code.
>>>>> ---
>>>>> fftools/ffprobe.c | 6 +++++-
>>>>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>>>>
>>>>> Affects 5.1 so will need to be backported there.
>>>>>
>>>>> diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c
>>>>> index ad633ccc44..8983dc28cc 100644
>>>>> --- a/fftools/ffprobe.c
>>>>> +++ b/fftools/ffprobe.c
>>>>> @@ -4032,7 +4032,7 @@ int main(int argc, char **argv)
>>>>> WriterContext *wctx;
>>>>> char *buf;
>>>>> char *w_name = NULL, *w_args = NULL;
>>>>> - int ret, i;
>>>>> + int ret, input_ret, i;
>>>>>
>>>>> init_dynload();
>>>>>
>>>>> @@ -4156,10 +4156,14 @@ int main(int argc, char **argv)
>>>>> show_error(wctx, ret);
>>>>> }
>>>>> + input_ret = ret;
>>>>> +
>>>>> writer_print_section_footer(wctx);
>>>>> ret = writer_close(&wctx);
>>>>> if (ret < 0)
>>>>> av_log(NULL, AV_LOG_ERROR, "Writing output failed:
>>>>> %s\n",
>>>>> av_err2str(ret));
>>>>> +
>>>>> + ret = FFMIN(ret, input_ret);
>>>> maybe we should give priority to input_ret in case they are both
>>>> negative?
>>>>
>>>> return input_ret < 0 ? input_ret : ret;
>>>
>>> Scripts usually depend on exit code being 0 or something else. Also,
>>> error is logged for both input failure and writer_close failure, so
>>> it doesn't matter.
>>> Finally, input_ret is not initialized if writer_open fails, so we
>>> shouldn't be referencing it outside.
>>
>> I would do something like
>>
>> ret2 = writer_close();
>> ret = FFMIN(ret2, ret);
>>
>> But fine either way, please push any version you prefer, this has
>> been broken for too long. Sorry about that.
>
> The only diff is which return does the new variable store. I like my
> var name better :/
>
> Will push in a couple of hours, 5.1 as well
>
> Regards,
> Gyan
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-08-17 11:20 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-15 18:34 [FFmpeg-devel] [PATCH] ffprobe: restore reporting error code for failed inputs Gyan Doshi
2022-08-16 23:06 ` Stefano Sabatini
2022-08-17 4:12 ` Gyan Doshi
2022-08-17 7:07 ` Marton Balint
2022-08-17 7:13 ` Gyan Doshi
2022-08-17 11:19 ` Gyan Doshi
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
This inbox may be cloned and mirrored by anyone:
git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git
# If you have public-inbox 1.1+ installed, you may
# initialize and index your mirror using the following commands:
public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
ffmpegdev@gitmailbox.com
public-inbox-index ffmpegdev
Example config snippet for mirrors.
AGPL code for this site: git clone https://public-inbox.org/public-inbox.git