[FFmpeg-devel] [PATCH 1/4] avcodec/hevcdec: Combine multiple allocations

[FFmpeg-devel] [PATCH 1/4] avcodec/hevcdec: Combine multiple allocations

[Andreas Rheinhardt]

Reduces the number of allocs and frees.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/hevcdec.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c
index 8d7a4f7147..568bdb5ab7 100644
--- a/libavcodec/hevcdec.c
+++ b/libavcodec/hevcdec.c
@@ -2621,17 +2621,15 @@ static int hls_slice_data_wpp(HEVCContext *s, const H2645NAL *nal)
     const uint8_t *data = nal->data;
     int length          = nal->size;
     HEVCLocalContext *lc = s->HEVClc;
-    int *ret = av_malloc_array(s->sh.num_entry_point_offsets + 1, sizeof(int));
-    int *arg = av_malloc_array(s->sh.num_entry_point_offsets + 1, sizeof(int));
+    int *ret = av_malloc_array(s->sh.num_entry_point_offsets + 1, 2 * sizeof(int));
+    int *arg;
     int64_t offset;
     int64_t startheader, cmpt = 0;
     int i, j, res = 0;
 
-    if (!ret || !arg) {
-        av_free(ret);
-        av_free(arg);
+    if (!ret)
         return AVERROR(ENOMEM);
-    }
+    arg = ret + s->sh.num_entry_point_offsets + 1;
 
     if (s->sh.slice_ctb_addr_rs + s->sh.num_entry_point_offsets * s->ps.sps->ctb_width >= s->ps.sps->ctb_width * s->ps.sps->ctb_height) {
         av_log(s->avctx, AV_LOG_ERROR, "WPP ctb addresses are wrong (%d %d %d %d)\n",
@@ -2716,7 +2714,6 @@ static int hls_slice_data_wpp(HEVCContext *s, const H2645NAL *nal)
         res += ret[i];
 error:
     av_free(ret);
-    av_free(arg);
     return res;
 }
 
-- 
2.32.0

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

[FFmpeg-devel] [PATCH 2/4] avcodec/hevcdec: Remove redundant frees

[Andreas Rheinhardt]

These arrays have already been freed in pic_arrays_free().

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/hevcdec.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c
index 568bdb5ab7..85a1b9f47a 100644
--- a/libavcodec/hevcdec.c
+++ b/libavcodec/hevcdec.c
@@ -3595,10 +3595,6 @@ static av_cold int hevc_decode_free(AVCodecContext *avctx)
 
     ff_hevc_ps_uninit(&s->ps);
 
-    av_freep(&s->sh.entry_point_offset);
-    av_freep(&s->sh.offset);
-    av_freep(&s->sh.size);
-
     if (s->HEVClcList && s->sList) {
         for (i = 1; i < s->threads_number; i++) {
             av_freep(&s->HEVClcList[i]);
-- 
2.32.0

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

[FFmpeg-devel] [PATCH 3/4] avcodec/hevcdec: Combine related data into structure

[Andreas Rheinhardt]

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/hevcdec.c | 28 +++++++++++++---------------
 libavcodec/hevcdec.h |  8 ++++++--
 2 files changed, 19 insertions(+), 17 deletions(-)

diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c
index 85a1b9f47a..fc0dc7a584 100644
--- a/libavcodec/hevcdec.c
+++ b/libavcodec/hevcdec.c
@@ -78,8 +78,7 @@ static void pic_arrays_free(HEVCContext *s)
     av_freep(&s->vertical_bs);
 
     av_freep(&s->sh.entry_point_offset);
-    av_freep(&s->sh.size);
-    av_freep(&s->sh.offset);
+    av_freep(&s->sh.entry_points);
 
     av_buffer_pool_uninit(&s->tab_mvf_pool);
     av_buffer_pool_uninit(&s->rpl_tab_pool);
@@ -915,12 +914,10 @@ static int hls_slice_header(HEVCContext *s)
             }
 
             av_freep(&sh->entry_point_offset);
-            av_freep(&sh->offset);
-            av_freep(&sh->size);
+            av_freep(&sh->entry_points);
             sh->entry_point_offset = av_malloc_array(sh->num_entry_point_offsets, sizeof(unsigned));
-            sh->offset = av_malloc_array(sh->num_entry_point_offsets, sizeof(int));
-            sh->size = av_malloc_array(sh->num_entry_point_offsets, sizeof(int));
-            if (!sh->entry_point_offset || !sh->offset || !sh->size) {
+            FF_ALLOC_TYPED_ARRAY(sh->entry_points, sh->num_entry_point_offsets);
+            if (!sh->entry_point_offset || !sh->entry_points) {
                 sh->num_entry_point_offsets = 0;
                 av_log(s->avctx, AV_LOG_ERROR, "Failed to allocate memory\n");
                 return AVERROR(ENOMEM);
@@ -2552,10 +2549,11 @@ static int hls_decode_entry_wpp(AVCodecContext *avctxt, void *input_ctb_row, int
     lc = s->HEVClc;
 
     if(ctb_row) {
-        ret = init_get_bits8(&lc->gb, s->data + s->sh.offset[ctb_row - 1], s->sh.size[ctb_row - 1]);
+        const EntryPoint *const entry_point = &s->sh.entry_points[ctb_row - 1];
+        ret = init_get_bits8(&lc->gb, s->data + entry_point->offset, entry_point->size);
         if (ret < 0)
             goto error;
-        ff_init_cabac_decoder(&lc->cc, s->data + s->sh.offset[(ctb_row)-1], s->sh.size[ctb_row - 1]);
+        ff_init_cabac_decoder(&lc->cc, s->data + entry_point->offset, entry_point->size);
     }
 
     while(more_data && ctb_addr_ts < s->ps.sps->ctb_size) {
@@ -2675,9 +2673,10 @@ static int hls_slice_data_wpp(HEVCContext *s, const H2645NAL *nal)
                 cmpt++;
             }
         }
-        s->sh.size[i - 1] = s->sh.entry_point_offset[i] - cmpt;
-        s->sh.offset[i - 1] = offset;
-
+        s->sh.entry_points[i - 1] = (EntryPoint){
+            .size   = s->sh.entry_point_offset[i] - cmpt,
+            .offset = offset
+        };
     }
     if (s->sh.num_entry_point_offsets != 0) {
         offset += s->sh.entry_point_offset[s->sh.num_entry_point_offsets - 1] - cmpt;
@@ -2686,9 +2685,8 @@ static int hls_slice_data_wpp(HEVCContext *s, const H2645NAL *nal)
             res = AVERROR_INVALIDDATA;
             goto error;
         }
-        s->sh.size[s->sh.num_entry_point_offsets - 1] = length - offset;
-        s->sh.offset[s->sh.num_entry_point_offsets - 1] = offset;
-
+        s->sh.entry_points[s->sh.num_entry_point_offsets - 1] =
+            (EntryPoint){ .size = length - offset, .offset = offset };
     }
     s->data = data;
 
diff --git a/libavcodec/hevcdec.h b/libavcodec/hevcdec.h
index 157bc6926f..76ad262558 100644
--- a/libavcodec/hevcdec.h
+++ b/libavcodec/hevcdec.h
@@ -246,6 +246,11 @@ typedef struct RefPicListTab {
     RefPicList refPicList[2];
 } RefPicListTab;
 
+typedef struct EntryPoint {
+    int offset;
+    int size;
+} EntryPoint;
+
 typedef struct SliceHeader {
     unsigned int pps_id;
 
@@ -300,8 +305,7 @@ typedef struct SliceHeader {
     unsigned int max_num_merge_cand; ///< 5 - 5_minus_max_num_merge_cand
 
     unsigned *entry_point_offset;
-    int * offset;
-    int * size;
+    EntryPoint *entry_points;
     int num_entry_point_offsets;
 
     int8_t slice_qp;
-- 
2.32.0

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

[FFmpeg-devel] [PATCH 4/4] avcodec/hevcdec: Avoid redundant entry_point_offsets array

[Andreas Rheinhardt]

Instead modify the offsets in place.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/hevcdec.c | 19 +++++++++----------
 libavcodec/hevcdec.h |  1 -
 2 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c
index fc0dc7a584..6aa14455d0 100644
--- a/libavcodec/hevcdec.c
+++ b/libavcodec/hevcdec.c
@@ -77,7 +77,6 @@ static void pic_arrays_free(HEVCContext *s)
     av_freep(&s->horizontal_bs);
     av_freep(&s->vertical_bs);
 
-    av_freep(&s->sh.entry_point_offset);
     av_freep(&s->sh.entry_points);
 
     av_buffer_pool_uninit(&s->tab_mvf_pool);
@@ -913,18 +912,16 @@ static int hls_slice_header(HEVCContext *s)
                 return AVERROR_INVALIDDATA;
             }
 
-            av_freep(&sh->entry_point_offset);
             av_freep(&sh->entry_points);
-            sh->entry_point_offset = av_malloc_array(sh->num_entry_point_offsets, sizeof(unsigned));
             FF_ALLOC_TYPED_ARRAY(sh->entry_points, sh->num_entry_point_offsets);
-            if (!sh->entry_point_offset || !sh->entry_points) {
+            if (!sh->entry_points) {
                 sh->num_entry_point_offsets = 0;
                 av_log(s->avctx, AV_LOG_ERROR, "Failed to allocate memory\n");
                 return AVERROR(ENOMEM);
             }
             for (i = 0; i < sh->num_entry_point_offsets; i++) {
                 unsigned val = get_bits_long(gb, offset_len);
-                sh->entry_point_offset[i] = val + 1; // +1; // +1 to get the size
+                sh->entry_points[i].offset = val + 1; // +1; // +1 to get the size
             }
             if (s->threads_number > 1 && (s->ps.pps->num_tile_rows > 1 || s->ps.pps->num_tile_columns > 1)) {
                 s->enable_parallel_tiles = 0; // TODO: you can enable tiles in parallel here
@@ -2657,29 +2654,31 @@ static int hls_slice_data_wpp(HEVCContext *s, const H2645NAL *nal)
 
     offset = (lc->gb.index >> 3);
 
-    for (j = 0, cmpt = 0, startheader = offset + s->sh.entry_point_offset[0]; j < nal->skipped_bytes; j++) {
+    for (j = 0, cmpt = 0, startheader = offset + s->sh.entry_points[0].offset; j < nal->skipped_bytes; j++) {
         if (nal->skipped_bytes_pos[j] >= offset && nal->skipped_bytes_pos[j] < startheader) {
             startheader--;
             cmpt++;
         }
     }
 
+    /* Convert the entry_points offsets from being pre-0x03-escaping
+     * to post-0x03-escaping and set sizes. */
     for (i = 1; i < s->sh.num_entry_point_offsets; i++) {
-        offset += (s->sh.entry_point_offset[i - 1] - cmpt);
+        offset += (s->sh.entry_points[i - 1].offset - cmpt);
         for (j = 0, cmpt = 0, startheader = offset
-             + s->sh.entry_point_offset[i]; j < nal->skipped_bytes; j++) {
+             + s->sh.entry_points[i].offset; j < nal->skipped_bytes; j++) {
             if (nal->skipped_bytes_pos[j] >= offset && nal->skipped_bytes_pos[j] < startheader) {
                 startheader--;
                 cmpt++;
             }
         }
         s->sh.entry_points[i - 1] = (EntryPoint){
-            .size   = s->sh.entry_point_offset[i] - cmpt,
+            .size   = s->sh.entry_points[i].offset - cmpt,
             .offset = offset
         };
     }
     if (s->sh.num_entry_point_offsets != 0) {
-        offset += s->sh.entry_point_offset[s->sh.num_entry_point_offsets - 1] - cmpt;
+        offset += s->sh.entry_points[s->sh.num_entry_point_offsets - 1].offset - cmpt;
         if (length < offset) {
             av_log(s->avctx, AV_LOG_ERROR, "entry_point_offset table is corrupted\n");
             res = AVERROR_INVALIDDATA;
diff --git a/libavcodec/hevcdec.h b/libavcodec/hevcdec.h
index 76ad262558..9ac04555f4 100644
--- a/libavcodec/hevcdec.h
+++ b/libavcodec/hevcdec.h
@@ -304,7 +304,6 @@ typedef struct SliceHeader {
 
     unsigned int max_num_merge_cand; ///< 5 - 5_minus_max_num_merge_cand
 
-    unsigned *entry_point_offset;
     EntryPoint *entry_points;
     int num_entry_point_offsets;
 
-- 
2.32.0

_______________________________________________
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 4/4] avcodec/hevcdec: Avoid redundant entry_point_offsets array

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 572 bytes --]

On Wed, Jan 05, 2022 at 10:19:08PM +0100, Andreas Rheinhardt wrote:
> Instead modify the offsets in place.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
>  libavcodec/hevcdec.c | 19 +++++++++----------
>  libavcodec/hevcdec.h |  1 -
>  2 files changed, 9 insertions(+), 11 deletions(-)

This seems to break -threads 4 -thread_type slice
ill send you the sample file privatly

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

It is what and why we do it that matters, not just one of them.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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 1/4] avcodec/hevcdec: Combine multiple allocations

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 789 bytes --]

On Wed, Jan 05, 2022 at 10:18:11PM +0100, Andreas Rheinhardt wrote:
> Reduces the number of allocs and frees.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
>  libavcodec/hevcdec.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)

something in the patches today causes a segfault in a hevc stream with slice threading
iam trying to bisect it but it but it does not replicate at all reliably ATM
so i dont even know if its from a patch today
just wanted to report what i know ATM ...

thx


[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Freedom in capitalist society always remains about the same as it was in
ancient Greek republics: Freedom for slave owners. -- Vladimir Lenin

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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 1/4] avcodec/hevcdec: Combine multiple allocations

[Andreas Rheinhardt]

Michael Niedermayer:
> On Wed, Jan 05, 2022 at 10:18:11PM +0100, Andreas Rheinhardt wrote:
>> Reduces the number of allocs and frees.
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
>> ---
>>  libavcodec/hevcdec.c | 11 ++++-------
>>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> something in the patches today causes a segfault in a hevc stream with slice threading
> iam trying to bisect it but it but it does not replicate at all reliably ATM
> so i dont even know if its from a patch today
> just wanted to report what i know ATM ...
> 
> thx

On what arch did you test this? Josh's recent patchset [1] caused
segfaults on aarch64, see also [2]. I actually tested with slice
threading (which this data is all about). But I will nevertheless look
at your sample.

- Andreas

[1]: https://ffmpeg.org/pipermail/ffmpeg-devel/2022-January/290774.html
[2]: https://ffmpeg.org/pipermail/ffmpeg-devel/2022-January/290862.html
_______________________________________________
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 1/4] avcodec/hevcdec: Combine multiple allocations

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 1170 bytes --]

On Thu, Jan 06, 2022 at 10:56:37AM +0100, Andreas Rheinhardt wrote:
> Michael Niedermayer:
> > On Wed, Jan 05, 2022 at 10:18:11PM +0100, Andreas Rheinhardt wrote:
> >> Reduces the number of allocs and frees.
> >>
> >> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> >> ---
> >>  libavcodec/hevcdec.c | 11 ++++-------
> >>  1 file changed, 4 insertions(+), 7 deletions(-)
> > 
> > something in the patches today causes a segfault in a hevc stream with slice threading
> > iam trying to bisect it but it but it does not replicate at all reliably ATM
> > so i dont even know if its from a patch today
> > just wanted to report what i know ATM ...
> > 
> > thx
> 
> On what arch did you test this? Josh's recent patchset [1] caused
> segfaults on aarch64, see also [2]. I actually tested with slice
> threading (which this data is all about). But I will nevertheless look
> at your sample.

x86-64 

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Does the universe only have a finite lifespan? No, its going to go on
forever, its just that you wont like living in it. -- Hiranya Peiri

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

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