| Message ID | 20220306153939.5285-1-jamrial@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] avcodec/hevc_refs: don't use the frame's AVBufferRef sizes when initializing planes | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_aarch64_jetson | success | Make finished |
| andriy/make_fate_aarch64_jetson | success | Make fate finished |
| andriy/make_armv7_RPi4 | success | Make finished |
| andriy/make_fate_armv7_RPi4 | success | Make fate finished |
Quoting James Almer (2022-03-06 16:39:39) > Use the actual plane sizes instead. > > Signed-off-by: James Almer <jamrial@gmail.com> > --- > libavcodec/hevc_refs.c | 14 +++++++++++--- > 1 file changed, 11 insertions(+), 3 deletions(-) > > diff --git a/libavcodec/hevc_refs.c b/libavcodec/hevc_refs.c > index 35b8e5e696..b0035356ac 100644 > --- a/libavcodec/hevc_refs.c > +++ b/libavcodec/hevc_refs.c > @@ -22,6 +22,7 @@ > */ > > #include "libavutil/avassert.h" > +#include "libavutil/imgutils.h" > > #include "thread.h" > #include "hevc.h" > @@ -401,9 +402,16 @@ static HEVCFrame *generate_missing_ref(HEVCContext *s, int poc) > > if (!s->avctx->hwaccel) { > if (!s->ps.sps->pixel_shift) { > - for (i = 0; frame->frame->buf[i]; i++) > - memset(frame->frame->buf[i]->data, 1 << (s->ps.sps->bit_depth - 1), > - frame->frame->buf[i]->size); > + size_t sizes[4]; > + ptrdiff_t linesizes[4]; > + for (i = 0; i < 4; i++) > + linesizes[i] = frame->frame->linesize[i]; > + if (av_image_fill_plane_sizes(sizes, frame->frame->format, frame->frame->height, linesizes) < 0) { > + ff_hevc_unref_frame(s, frame, ~0); > + return NULL; > + } > + for (i = 0; frame->frame->data[i]; i++) > + memset(frame->frame->data[i], 1 << (s->ps.sps->bit_depth - 1), sizes[i]); wouldn't it be simpler and shorter to use linesize[i] * (height >> sps->vshift)?
On 3/8/2022 9:49 AM, Anton Khirnov wrote: > Quoting James Almer (2022-03-06 16:39:39) >> Use the actual plane sizes instead. >> >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> libavcodec/hevc_refs.c | 14 +++++++++++--- >> 1 file changed, 11 insertions(+), 3 deletions(-) >> >> diff --git a/libavcodec/hevc_refs.c b/libavcodec/hevc_refs.c >> index 35b8e5e696..b0035356ac 100644 >> --- a/libavcodec/hevc_refs.c >> +++ b/libavcodec/hevc_refs.c >> @@ -22,6 +22,7 @@ >> */ >> >> #include "libavutil/avassert.h" >> +#include "libavutil/imgutils.h" >> >> #include "thread.h" >> #include "hevc.h" >> @@ -401,9 +402,16 @@ static HEVCFrame *generate_missing_ref(HEVCContext *s, int poc) >> >> if (!s->avctx->hwaccel) { >> if (!s->ps.sps->pixel_shift) { >> - for (i = 0; frame->frame->buf[i]; i++) >> - memset(frame->frame->buf[i]->data, 1 << (s->ps.sps->bit_depth - 1), >> - frame->frame->buf[i]->size); >> + size_t sizes[4]; >> + ptrdiff_t linesizes[4]; >> + for (i = 0; i < 4; i++) >> + linesizes[i] = frame->frame->linesize[i]; >> + if (av_image_fill_plane_sizes(sizes, frame->frame->format, frame->frame->height, linesizes) < 0) { >> + ff_hevc_unref_frame(s, frame, ~0); >> + return NULL; >> + } >> + for (i = 0; frame->frame->data[i]; i++) >> + memset(frame->frame->data[i], 1 << (s->ps.sps->bit_depth - 1), sizes[i]); > > wouldn't it be simpler and shorter to use linesize[i] * (height >> sps->vshift)? That's what av_image_fill_plane_sizes() does to fill sizes[]. If we do it manually we also need to ensure to do it only for i > 0, so it's not that much of a simplification. But ok, will send a patch with that change. Could we for that matter change AVFrame linesize to ptrdiff_t in the next bump? Alongside the relevant imgutils helpers, if possible. Half this patch is just doing int -> ptrdiff_t, which is why it looks so big.
On Tue, Mar 8, 2022 at 1:58 PM James Almer <jamrial@gmail.com> wrote: > > Could we for that matter change AVFrame linesize to ptrdiff_t in the > next bump? Alongside the relevant imgutils helpers, if possible. > Half this patch is just doing int -> ptrdiff_t, which is why it looks so > big. Its definitely long overdue, having half the frame API already use size_t/ptrdiff_t makes for really annoying conversion loops when trying to use them. - Hendrik
Quoting James Almer (2022-03-08 13:58:33) > > > On 3/8/2022 9:49 AM, Anton Khirnov wrote: > > Quoting James Almer (2022-03-06 16:39:39) > >> Use the actual plane sizes instead. > >> > >> Signed-off-by: James Almer <jamrial@gmail.com> > >> --- > >> libavcodec/hevc_refs.c | 14 +++++++++++--- > >> 1 file changed, 11 insertions(+), 3 deletions(-) > >> > >> diff --git a/libavcodec/hevc_refs.c b/libavcodec/hevc_refs.c > >> index 35b8e5e696..b0035356ac 100644 > >> --- a/libavcodec/hevc_refs.c > >> +++ b/libavcodec/hevc_refs.c > >> @@ -22,6 +22,7 @@ > >> */ > >> > >> #include "libavutil/avassert.h" > >> +#include "libavutil/imgutils.h" > >> > >> #include "thread.h" > >> #include "hevc.h" > >> @@ -401,9 +402,16 @@ static HEVCFrame *generate_missing_ref(HEVCContext *s, int poc) > >> > >> if (!s->avctx->hwaccel) { > >> if (!s->ps.sps->pixel_shift) { > >> - for (i = 0; frame->frame->buf[i]; i++) > >> - memset(frame->frame->buf[i]->data, 1 << (s->ps.sps->bit_depth - 1), > >> - frame->frame->buf[i]->size); > >> + size_t sizes[4]; > >> + ptrdiff_t linesizes[4]; > >> + for (i = 0; i < 4; i++) > >> + linesizes[i] = frame->frame->linesize[i]; > >> + if (av_image_fill_plane_sizes(sizes, frame->frame->format, frame->frame->height, linesizes) < 0) { > >> + ff_hevc_unref_frame(s, frame, ~0); > >> + return NULL; > >> + } > >> + for (i = 0; frame->frame->data[i]; i++) > >> + memset(frame->frame->data[i], 1 << (s->ps.sps->bit_depth - 1), sizes[i]); > > > > wouldn't it be simpler and shorter to use linesize[i] * (height >> sps->vshift)? > > That's what av_image_fill_plane_sizes() does to fill sizes[]. > If we do it manually we also need to ensure to do it only for i > 0, so > it's not that much of a simplification. But ok, will send a patch with > that change. vshift is a per-plane array, so you don't have to > > Could we for that matter change AVFrame linesize to ptrdiff_t in the > next bump? Alongside the relevant imgutils helpers, if possible. > Half this patch is just doing int -> ptrdiff_t, which is why it looks so > big. Might be a lot of work, but I'm certainly not against.
diff --git a/libavcodec/hevc_refs.c b/libavcodec/hevc_refs.c index 35b8e5e696..b0035356ac 100644 --- a/libavcodec/hevc_refs.c +++ b/libavcodec/hevc_refs.c @@ -22,6 +22,7 @@ */ #include "libavutil/avassert.h" +#include "libavutil/imgutils.h" #include "thread.h" #include "hevc.h" @@ -401,9 +402,16 @@ static HEVCFrame *generate_missing_ref(HEVCContext *s, int poc) if (!s->avctx->hwaccel) { if (!s->ps.sps->pixel_shift) { - for (i = 0; frame->frame->buf[i]; i++) - memset(frame->frame->buf[i]->data, 1 << (s->ps.sps->bit_depth - 1), - frame->frame->buf[i]->size); + size_t sizes[4]; + ptrdiff_t linesizes[4]; + for (i = 0; i < 4; i++) + linesizes[i] = frame->frame->linesize[i]; + if (av_image_fill_plane_sizes(sizes, frame->frame->format, frame->frame->height, linesizes) < 0) { + ff_hevc_unref_frame(s, frame, ~0); + return NULL; + } + for (i = 0; frame->frame->data[i]; i++) + memset(frame->frame->data[i], 1 << (s->ps.sps->bit_depth - 1), sizes[i]); } else { for (i = 0; frame->frame->data[i]; i++) for (y = 0; y < (s->ps.sps->height >> s->ps.sps->vshift[i]); y++) {
Use the actual plane sizes instead. Signed-off-by: James Almer <jamrial@gmail.com> --- libavcodec/hevc_refs.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-)