[FFmpeg-devel] [PATCH] avcodec/hevc_refs: don't use the frame's AVBufferRef sizes when initializing planes

[FFmpeg-devel] [PATCH] avcodec/hevc_refs: don't use the frame's AVBufferRef sizes when initializing planes

[James Almer]

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]);
         } else {
             for (i = 0; frame->frame->data[i]; i++)
                 for (y = 0; y < (s->ps.sps->height >> s->ps.sps->vshift[i]); y++) {
-- 
2.35.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".

Re: [FFmpeg-devel] [PATCH] avcodec/hevc_refs: don't use the frame's AVBufferRef sizes when initializing planes

[Anton Khirnov]

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)?

-- 
Anton Khirnov
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH] avcodec/hevc_refs: don't use the frame's AVBufferRef sizes when initializing planes

[James Almer]

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.
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH] avcodec/hevc_refs: don't use the frame's AVBufferRef sizes when initializing planes

[Hendrik Leppkes]

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
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH] avcodec/hevc_refs: don't use the frame's AVBufferRef sizes when initializing planes

[Anton Khirnov]

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.

-- 
Anton Khirnov
_______________________________________________
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".