| Message ID | AS8P250MB07448FF502A0A661A49A94628FF6A@AS8P250MB0744.EURP250.PROD.OUTLOOK.COM |
|---|---|
| State | Accepted |
| Commit | 3614f7e1ccb70bdb21cf3cb326df2764996055c5 |
| Headers | show |
| Series | [FFmpeg-devel,1/3] avcodec/vp3: Move work after ff_thread_finish_setup | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
Andreas Rheinhardt: > When decoding a keyframe, last_frame and golden_frame are > not used at all and (at least when starting decoding) > are not set at all. But due to code sharing pointer arithmetic > on the NULL data-pointers of these frames has nevertheless > been performed. This is undefined behaviour and causes e.g. > "runtime error: applying non-zero offset 173440 to null pointer" > from UBSan in the vp31, vp4, theora-coeff-level64 and theora-offset > FATE-tests. > > Fix this by reusing the current frame for unavailable frames. > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> > --- > libavcodec/vp3.c | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/libavcodec/vp3.c b/libavcodec/vp3.c > index 33c120a58e..5ce1ecfce7 100644 > --- a/libavcodec/vp3.c > +++ b/libavcodec/vp3.c > @@ -2056,6 +2056,14 @@ static void render_slice(Vp3DecodeContext *s, int slice) > { > int16_t *block = s->block; > int motion_x = 0xdeadbeef, motion_y = 0xdeadbeef; > + /* When decoding keyframes, the earlier frames may not be available, > + * so to avoid using undefined pointer arithmetic on them we just > + * use the current frame instead. Nothing is ever read from these > + * frames in case of a keyframe. */ > + const AVFrame *last_frame = s->last_frame.f->data[0] ? > + s->last_frame.f : s->current_frame.f; > + const AVFrame *golden_frame = s->golden_frame.f->data[0] ? > + s->golden_frame.f : s->current_frame.f; > int motion_halfpel_index; > int first_pixel; > > @@ -2065,9 +2073,9 @@ static void render_slice(Vp3DecodeContext *s, int slice) > for (int plane = 0; plane < 3; plane++) { > uint8_t *output_plane = s->current_frame.f->data[plane] + > s->data_offset[plane]; > - const uint8_t *last_plane = s->last_frame.f->data[plane] + > + const uint8_t *last_plane = last_frame->data[plane] + > s->data_offset[plane]; > - const uint8_t *golden_plane = s->golden_frame.f->data[plane] + > + const uint8_t *golden_plane = golden_frame->data[plane] + > s->data_offset[plane]; > ptrdiff_t stride = s->current_frame.f->linesize[plane]; > int plane_width = s->width >> (plane && s->chroma_x_shift); Will apply the remaining patches of this patchset tomorrow unless there are objections. - Andreas
diff --git a/libavcodec/vp3.c b/libavcodec/vp3.c index 33c120a58e..5ce1ecfce7 100644 --- a/libavcodec/vp3.c +++ b/libavcodec/vp3.c @@ -2056,6 +2056,14 @@ static void render_slice(Vp3DecodeContext *s, int slice) { int16_t *block = s->block; int motion_x = 0xdeadbeef, motion_y = 0xdeadbeef; + /* When decoding keyframes, the earlier frames may not be available, + * so to avoid using undefined pointer arithmetic on them we just + * use the current frame instead. Nothing is ever read from these + * frames in case of a keyframe. */ + const AVFrame *last_frame = s->last_frame.f->data[0] ? + s->last_frame.f : s->current_frame.f; + const AVFrame *golden_frame = s->golden_frame.f->data[0] ? + s->golden_frame.f : s->current_frame.f; int motion_halfpel_index; int first_pixel; @@ -2065,9 +2073,9 @@ static void render_slice(Vp3DecodeContext *s, int slice) for (int plane = 0; plane < 3; plane++) { uint8_t *output_plane = s->current_frame.f->data[plane] + s->data_offset[plane]; - const uint8_t *last_plane = s->last_frame.f->data[plane] + + const uint8_t *last_plane = last_frame->data[plane] + s->data_offset[plane]; - const uint8_t *golden_plane = s->golden_frame.f->data[plane] + + const uint8_t *golden_plane = golden_frame->data[plane] + s->data_offset[plane]; ptrdiff_t stride = s->current_frame.f->linesize[plane]; int plane_width = s->width >> (plane && s->chroma_x_shift);
When decoding a keyframe, last_frame and golden_frame are not used at all and (at least when starting decoding) are not set at all. But due to code sharing pointer arithmetic on the NULL data-pointers of these frames has nevertheless been performed. This is undefined behaviour and causes e.g. "runtime error: applying non-zero offset 173440 to null pointer" from UBSan in the vp31, vp4, theora-coeff-level64 and theora-offset FATE-tests. Fix this by reusing the current frame for unavailable frames. Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> --- libavcodec/vp3.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)