| Message ID | 20211203094343.66139-1-young_chelsea@163.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] Exception when frame is set NULL | expand |
| Context | Check | Description |
|---|---|---|
| andriy/commit_msg_x86 | warning | The first line of the commit message must start with a context terminated by a colon and a space, for example "lavu/opt: " or "doc: ". |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
| andriy/commit_msg_ppc | warning | The first line of the commit message must start with a context terminated by a colon and a space, for example "lavu/opt: " or "doc: ". |
| andriy/make_ppc | success | Make finished |
| andriy/make_fate_ppc | success | Make fate finished |
Yu Yang: > fftools/ffmpegc When `ost->last_frame` is NULL, 'SEGV' occurs when accessing its pts. > > libavutil/framec `ost->last_frame` may be set NULL by av_frame_alloc(). In this situation， > av_frame_unref() and av_frame_free() do nothing. Frame is not released. > > ```c > // in fftools/ffmpeg.c:1145 > 1145 static void do_video_out(OutputFile *of, ...) > > 1148 { > ... > // `ost->last_frame` is NULL. > 1272 av_log(NULL, AV_LOG_VERBOSE, > 1273 "*** dropping frame %d from stream %d at ts %"PRId64"\n", > 1274 ost->frame_number, ost->st->index, ost->last_frame->pts); > ... > 1421 if (!ost->last_frame) > // `ost->last_frame` may be set NULL here. > 1422 ost->last_frame = av_frame_alloc(); > ... > > 1433 } > ``` > > coredump backtrace info: > ==7192==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000088 (pc 0x0000005e87e2 bp 0x7fff84f0ffb0 sp 0x7fff84f0f020 T0) > ==7192==The signal is caused by a READ memory access. > ==7192==Hint: address points to the zero page. > #0 0x5e87e2 in do_video_out /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg.c:1274:68 > #1 0x5df341 in reap_filters /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg.c:1548:25 > #2 0x5d08b7 in transcode_from_filter /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg.c:4644:15 > #3 0x59e557 in transcode_step /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg.c:4729:20 > #4 0x593970 in transcode /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg.c:4805:15 > #5 0x58f7a4 in main /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg.c:5010:9 > #6 0x7f0fa9d900b2 in __libc_start_main /build/glibc-eX1tMB/glibc-2.31/csu/../csu/libc-start.c:308:16 > #7 0x42033d in _start (/home/r1/ffmpeg/ffmpeg_4.4.1+0x42033d) > > Reported-by: TOTE Robot <oslab@tsinghua.edu.cn> > Signed-off-by: Yu Yang <young_chelsea@163.com> > --- > fftools/ffmpeg.c | 7 ++++--- > libavutil/frame.c | 9 ++++----- > 2 files changed, 8 insertions(+), 8 deletions(-) > > diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c > index cfb04d5eff..cade05f762 100644 > --- a/fftools/ffmpeg.c > +++ b/fftools/ffmpeg.c > @@ -1265,9 +1265,10 @@ static void do_video_out(OutputFile *of, > > if (nb0_frames == 0 && ost->last_dropped) { > nb_frames_drop++; > - av_log(NULL, AV_LOG_VERBOSE, > - "*** dropping frame %d from stream %d at ts %"PRId64"\n", > - ost->frame_number, ost->st->index, ost->last_frame->pts); > + if (ost->last_frame) > + av_log(NULL, AV_LOG_VERBOSE, > + "*** dropping frame %d from stream %d at ts %"PRId64"\n", > + ost->frame_number, ost->st->index, ost->last_frame->pts); > } > if (nb_frames > (nb0_frames && ost->last_dropped) + (nb_frames > nb0_frames)) { > if (nb_frames > dts_error_threshold * 30) { > diff --git a/libavutil/frame.c b/libavutil/frame.c > index d4d3ad6988..9c866320a7 100644 > --- a/libavutil/frame.c > +++ b/libavutil/frame.c > @@ -111,11 +111,10 @@ AVFrame *av_frame_alloc(void) > > void av_frame_free(AVFrame **frame) > { > - if (!frame || !*frame) > - return; > - > - av_frame_unref(*frame); > - av_freep(frame); > + if (*frame) > + av_frame_unref(*frame); > + if (frame) > + av_freep(frame); > } > > static int get_video_buffer(AVFrame *frame, int align) > This change to frame.c is also completely wrong; this frame should probably not be constantly allocated and freed and the code at lines 1422-1428 should actually error out in case of allocation error. - Andreas
> 2021年12月3日 下午6:04，Andreas Rheinhardt <andreas.rheinhardt@outlook.com> 写道： > > Yu Yang: >> fftools/ffmpegc When `ost->last_frame` is NULL, 'SEGV' occurs when accessing its pts. >> >> libavutil/framec `ost->last_frame` may be set NULL by av_frame_alloc(). In this situation， >> av_frame_unref() and av_frame_free() do nothing. Frame is not released. >> >> ```c >> // in fftools/ffmpeg.c:1145 >> 1145 static void do_video_out(OutputFile *of, ...) >> >> 1148 { >> ... >> // `ost->last_frame` is NULL. >> 1272 av_log(NULL, AV_LOG_VERBOSE, >> 1273 "*** dropping frame %d from stream %d at ts %"PRId64"\n", >> 1274 ost->frame_number, ost->st->index, ost->last_frame->pts); >> ... >> 1421 if (!ost->last_frame) >> // `ost->last_frame` may be set NULL here. >> 1422 ost->last_frame = av_frame_alloc(); >> ... >> >> 1433 } >> ``` >> >> coredump backtrace info: >> ==7192==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000088 (pc 0x0000005e87e2 bp 0x7fff84f0ffb0 sp 0x7fff84f0f020 T0) >> ==7192==The signal is caused by a READ memory access. >> ==7192==Hint: address points to the zero page. >> #0 0x5e87e2 in do_video_out /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg.c:1274:68 >> #1 0x5df341 in reap_filters /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg.c:1548:25 >> #2 0x5d08b7 in transcode_from_filter /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg.c:4644:15 >> #3 0x59e557 in transcode_step /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg.c:4729:20 >> #4 0x593970 in transcode /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg.c:4805:15 >> #5 0x58f7a4 in main /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg.c:5010:9 >> #6 0x7f0fa9d900b2 in __libc_start_main /build/glibc-eX1tMB/glibc-2.31/csu/../csu/libc-start.c:308:16 >> #7 0x42033d in _start (/home/r1/ffmpeg/ffmpeg_4.4.1+0x42033d) >> >> Reported-by: TOTE Robot <oslab@tsinghua.edu.cn> >> Signed-off-by: Yu Yang <young_chelsea@163.com> >> --- >> fftools/ffmpeg.c | 7 ++++--- >> libavutil/frame.c | 9 ++++----- >> 2 files changed, 8 insertions(+), 8 deletions(-) >> >> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c >> index cfb04d5eff..cade05f762 100644 >> --- a/fftools/ffmpeg.c >> +++ b/fftools/ffmpeg.c >> @@ -1265,9 +1265,10 @@ static void do_video_out(OutputFile *of, >> >> if (nb0_frames == 0 && ost->last_dropped) { >> nb_frames_drop++; >> - av_log(NULL, AV_LOG_VERBOSE, >> - "*** dropping frame %d from stream %d at ts %"PRId64"\n", >> - ost->frame_number, ost->st->index, ost->last_frame->pts); >> + if (ost->last_frame) >> + av_log(NULL, AV_LOG_VERBOSE, >> + "*** dropping frame %d from stream %d at ts %"PRId64"\n", >> + ost->frame_number, ost->st->index, ost->last_frame->pts); >> } >> if (nb_frames > (nb0_frames && ost->last_dropped) + (nb_frames > nb0_frames)) { >> if (nb_frames > dts_error_threshold * 30) { >> diff --git a/libavutil/frame.c b/libavutil/frame.c >> index d4d3ad6988..9c866320a7 100644 >> --- a/libavutil/frame.c >> +++ b/libavutil/frame.c >> @@ -111,11 +111,10 @@ AVFrame *av_frame_alloc(void) >> >> void av_frame_free(AVFrame **frame) >> { >> - if (!frame || !*frame) >> - return; >> - >> - av_frame_unref(*frame); >> - av_freep(frame); >> + if (*frame) >> + av_frame_unref(*frame); >> + if (frame) >> + av_freep(frame); >> } >> >> static int get_video_buffer(AVFrame *frame, int align) >> > > This change to frame.c is also completely wrong; this frame should > probably not be constantly allocated and freed and the code at lines > 1422-1428 should actually error out in case of allocation error. Thx, how do you think about the fix of 'ost->last_frame’? The code at lines 1266-1270 , if ost->last_frame == NULL, error when accessing its its. And at lines 1422, we can know that ost->last_frame can be NULL. In this situation，I don’t understand that It is emptied and released immediately after allocation. Is it necessary? > > - Andreas > _______________________________________________ > 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".
diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c index cfb04d5eff..cade05f762 100644 --- a/fftools/ffmpeg.c +++ b/fftools/ffmpeg.c @@ -1265,9 +1265,10 @@ static void do_video_out(OutputFile *of, if (nb0_frames == 0 && ost->last_dropped) { nb_frames_drop++; - av_log(NULL, AV_LOG_VERBOSE, - "*** dropping frame %d from stream %d at ts %"PRId64"\n", - ost->frame_number, ost->st->index, ost->last_frame->pts); + if (ost->last_frame) + av_log(NULL, AV_LOG_VERBOSE, + "*** dropping frame %d from stream %d at ts %"PRId64"\n", + ost->frame_number, ost->st->index, ost->last_frame->pts); } if (nb_frames > (nb0_frames && ost->last_dropped) + (nb_frames > nb0_frames)) { if (nb_frames > dts_error_threshold * 30) { diff --git a/libavutil/frame.c b/libavutil/frame.c index d4d3ad6988..9c866320a7 100644 --- a/libavutil/frame.c +++ b/libavutil/frame.c @@ -111,11 +111,10 @@ AVFrame *av_frame_alloc(void) void av_frame_free(AVFrame **frame) { - if (!frame || !*frame) - return; - - av_frame_unref(*frame); - av_freep(frame); + if (*frame) + av_frame_unref(*frame); + if (frame) + av_freep(frame); } static int get_video_buffer(AVFrame *frame, int align)
fftools/ffmpegc When `ost->last_frame` is NULL, 'SEGV' occurs when accessing its pts. libavutil/framec `ost->last_frame` may be set NULL by av_frame_alloc(). In this situation， av_frame_unref() and av_frame_free() do nothing. Frame is not released. ```c // in fftools/ffmpeg.c:1145 1145 static void do_video_out(OutputFile *of, ...) 1148 { ... // `ost->last_frame` is NULL. 1272 av_log(NULL, AV_LOG_VERBOSE, 1273 "*** dropping frame %d from stream %d at ts %"PRId64"\n", 1274 ost->frame_number, ost->st->index, ost->last_frame->pts); ... 1421 if (!ost->last_frame) // `ost->last_frame` may be set NULL here. 1422 ost->last_frame = av_frame_alloc(); ... 1433 } ``` coredump backtrace info: ==7192==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000088 (pc 0x0000005e87e2 bp 0x7fff84f0ffb0 sp 0x7fff84f0f020 T0) ==7192==The signal is caused by a READ memory access. ==7192==Hint: address points to the zero page. #0 0x5e87e2 in do_video_out /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg.c:1274:68 #1 0x5df341 in reap_filters /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg.c:1548:25 #2 0x5d08b7 in transcode_from_filter /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg.c:4644:15 #3 0x59e557 in transcode_step /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg.c:4729:20 #4 0x593970 in transcode /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg.c:4805:15 #5 0x58f7a4 in main /home/r1/ffmpeg/ffmpeg-4.4.1/build/src/fftools/ffmpeg.c:5010:9 #6 0x7f0fa9d900b2 in __libc_start_main /build/glibc-eX1tMB/glibc-2.31/csu/../csu/libc-start.c:308:16 #7 0x42033d in _start (/home/r1/ffmpeg/ffmpeg_4.4.1+0x42033d) Reported-by: TOTE Robot <oslab@tsinghua.edu.cn> Signed-off-by: Yu Yang <young_chelsea@163.com> --- fftools/ffmpeg.c | 7 ++++--- libavutil/frame.c | 9 ++++----- 2 files changed, 8 insertions(+), 8 deletions(-)