diff mbox

[FFmpeg-devel] ffmpeg: Check the return values of two functions

Message ID CAB0OVGqsNo1g+gUw_1s5JYL9QQdSsXcVbR17=P569kqL+SLgKw@mail.gmail.com
State Superseded
Headers show

Commit Message

Carl Eugen Hoyos Feb. 12, 2017, 4:20 p.m. UTC 
2017-02-12 17:00 GMT+01:00 Nicolas George <george@nsup.org>:
> Le quartidi 24 pluviôse, an CCXXV, Carl Eugen Hoyos a écrit :
>> Hi!
>>
>> Attached patch written yesterday silences warnings when compiling
>> ffmpeg.c, don't know how useful it is, only fate-tested.
>>
>> Please comment, Carl Eugen
>
>> From 525aff909aec50d4e1a49f289ff9069300a4b51f Mon Sep 17 00:00:00 2001
>> From: Carl Eugen Hoyos <cehoyos@ag.or.at>
>> Date: Sun, 12 Feb 2017 16:41:21 +0100
>> Subject: [PATCH] ffmpeg: Check the return value of two functions declared
>>  "warn_unused_result".
>>
>> ---
>>  ffmpeg.c |   20 ++++++++++++++------
>>  1 file changed, 14 insertions(+), 6 deletions(-)
>>
>> diff --git a/ffmpeg.c b/ffmpeg.c
>> index 06570c0..9952da6 100644
>> --- a/ffmpeg.c
>> +++ b/ffmpeg.c
>> @@ -215,14 +215,18 @@ static void sub2video_copy_rect(uint8_t *dst, int dst_linesize, int w, int h,
>>  static void sub2video_push_ref(InputStream *ist, int64_t pts)
>>  {
>>      AVFrame *frame = ist->sub2video.frame;
>> -    int i;
>> +    int i, ret;
>>
>>      av_assert1(frame->data[0]);
>>      ist->sub2video.last_pts = frame->pts = pts;
>> -    for (i = 0; i < ist->nb_filters; i++)
>> -        av_buffersrc_add_frame_flags(ist->filters[i]->filter, frame,
>> +    for (i = 0; i < ist->nb_filters; i++) {
>
>> +        ret = av_buffersrc_add_frame_flags(ist->filters[i]->filter, frame,
>>                                       AV_BUFFERSRC_FLAG_KEEP_REF |
>>                                       AV_BUFFERSRC_FLAG_PUSH);
>
> When the changes are that small, I think it is better to fix the
> indentation at the same time rather than leave it like that or fix it in
> a separate commit.

Done.

>> +        if (ret < 0)
>> +            av_log(ist->filters[i]->filter, AV_LOG_ERROR,
>
>> +                   "Failed to add a frame to the buffer source.\n");
>
> Please mention it is a "subtitle video frame" and include the error
> message itself (av_err2str(ret)).

Thank you, done twice.

New patch attached, Carl Eugen

Comments

Nicolas George Feb. 12, 2017, 4:26 p.m. UTC | #1 
Le quartidi 24 pluviôse, an CCXXV, Carl Eugen Hoyos a écrit :
> New patch attached, Carl Eugen

Thanks. The new patch looks fine to me, of course.

I have said my piece on the other considerations, I do not think the
eventual decision is mine to make.

Regards,
Paul B Mahol Feb. 12, 2017, 4:45 p.m. UTC | #2 
On 2/12/17, Nicolas George <george@nsup.org> wrote:
> Le quartidi 24 pluviose, an CCXXV, Carl Eugen Hoyos a ecrit :
>> New patch attached, Carl Eugen
>
> Thanks. The new patch looks fine to me, of course.
>
> I have said my piece on the other considerations, I do not think the
> eventual decision is mine to make.
>
> Regards,
>
> --
>   Nicolas George
>


I do not see big need for this patch. It can be applied later
when merge work is finished.
diff mbox

Patch

From 267e5e9733a073b6a6f578a5eb2199d52e05248a Mon Sep 17 00:00:00 2001
From: Carl Eugen Hoyos <cehoyos@ag.or.at>
Date: Sun, 12 Feb 2017 17:17:25 +0100
Subject: [PATCH] ffmpeg: Check the return value of two functions declared
 "warn_unused_result".

---
 ffmpeg.c |   26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/ffmpeg.c b/ffmpeg.c
index 06570c0..36b8132 100644
--- a/ffmpeg.c
+++ b/ffmpeg.c
@@ -215,14 +215,19 @@  static void sub2video_copy_rect(uint8_t *dst, int dst_linesize, int w, int h,
 static void sub2video_push_ref(InputStream *ist, int64_t pts)
 {
     AVFrame *frame = ist->sub2video.frame;
-    int i;
+    int i, ret;
 
     av_assert1(frame->data[0]);
     ist->sub2video.last_pts = frame->pts = pts;
-    for (i = 0; i < ist->nb_filters; i++)
-        av_buffersrc_add_frame_flags(ist->filters[i]->filter, frame,
-                                     AV_BUFFERSRC_FLAG_KEEP_REF |
-                                     AV_BUFFERSRC_FLAG_PUSH);
+    for (i = 0; i < ist->nb_filters; i++) {
+        ret = av_buffersrc_add_frame_flags(ist->filters[i]->filter, frame,
+                                           AV_BUFFERSRC_FLAG_KEEP_REF |
+                                           AV_BUFFERSRC_FLAG_PUSH);
+        if (ret < 0)
+            av_log(ist->filters[i]->filter, AV_LOG_ERROR,
+                   "Failed to add a subtitle video frame to the buffer source: %s\n",
+                   av_err2str(ret));
+    }
 }
 
 static void sub2video_update(InputStream *ist, AVSubtitle *sub)
@@ -290,12 +295,17 @@  static void sub2video_heartbeat(InputStream *ist, int64_t pts)
 
 static void sub2video_flush(InputStream *ist)
 {
-    int i;
+    int i, ret;
 
     if (ist->sub2video.end_pts < INT64_MAX)
         sub2video_update(ist, NULL);
-    for (i = 0; i < ist->nb_filters; i++)
-        av_buffersrc_add_frame(ist->filters[i]->filter, NULL);
+    for (i = 0; i < ist->nb_filters; i++) {
+        ret = av_buffersrc_add_frame(ist->filters[i]->filter, NULL);
+        if (ret < 0)
+            av_log(ist->filters[i]->filter, AV_LOG_ERROR,
+                   "Failed to add a subtitle video frame to the buffer source: %s\n",
+                   av_err2str(ret));
+    }
 }
 
 /* end of sub2video hack */
-- 
1.7.10.4