[FFmpeg-devel] [PATCH 1/7] avcodec/vc2enc: Avoid void* where possible

[FFmpeg-devel] [PATCH 1/7] avcodec/vc2enc: Avoid void* where possible

[Andreas Rheinhardt]

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

diff --git a/libavcodec/vc2enc.c b/libavcodec/vc2enc.c
index 365d43146d..b496f67d3a 100644
--- a/libavcodec/vc2enc.c
+++ b/libavcodec/vc2enc.c
@@ -106,7 +106,7 @@ typedef struct Plane {
 typedef struct SliceArgs {
     PutBitContext pb;
     int cache[DIRAC_MAX_QUANT_INDEX];
-    void *ctx;
+    struct VC2EncContext *ctx;
     int x;
     int y;
     int quant_idx;
@@ -116,7 +116,7 @@ typedef struct SliceArgs {
 } SliceArgs;
 
 typedef struct TransformArgs {
-    void *ctx;
+    struct VC2EncContext *ctx;
     Plane *plane;
     const void *idata;
     ptrdiff_t istride;
-- 
2.40.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".

[FFmpeg-devel] [PATCH 2/7] avcodec/vc2enc: Constify slices->main context pointers

[Andreas Rheinhardt]

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

diff --git a/libavcodec/vc2enc.c b/libavcodec/vc2enc.c
index b496f67d3a..4dcf423ef1 100644
--- a/libavcodec/vc2enc.c
+++ b/libavcodec/vc2enc.c
@@ -106,7 +106,7 @@ typedef struct Plane {
 typedef struct SliceArgs {
     PutBitContext pb;
     int cache[DIRAC_MAX_QUANT_INDEX];
-    struct VC2EncContext *ctx;
+    const struct VC2EncContext *ctx;
     int x;
     int y;
     int quant_idx;
@@ -116,7 +116,7 @@ typedef struct SliceArgs {
 } SliceArgs;
 
 typedef struct TransformArgs {
-    struct VC2EncContext *ctx;
+    const struct VC2EncContext *ctx;
     Plane *plane;
     const void *idata;
     ptrdiff_t istride;
@@ -527,8 +527,8 @@ static void encode_picture_start(VC2EncContext *s)
 #define QUANT(c, mul, add, shift) (((mul) * (c) + (add)) >> (shift))
 
 /* VC-2 13.5.5.2 - slice_band() */
-static void encode_subband(VC2EncContext *s, PutBitContext *pb, int sx, int sy,
-                           SubBand *b, int quant)
+static void encode_subband(const VC2EncContext *s, PutBitContext *pb,
+                           int sx, int sy, const SubBand *b, int quant)
 {
     int x, y;
 
@@ -558,7 +558,7 @@ static int count_hq_slice(SliceArgs *slice, int quant_idx)
     int x, y;
     uint8_t quants[MAX_DWT_LEVELS][4];
     int bits = 0, p, level, orientation;
-    VC2EncContext *s = slice->ctx;
+    const VC2EncContext *s = slice->ctx;
 
     if (slice->cache[quant_idx])
         return slice->cache[quant_idx];
@@ -576,7 +576,7 @@ static int count_hq_slice(SliceArgs *slice, int quant_idx)
         bits += 8;
         for (level = 0; level < s->wavelet_depth; level++) {
             for (orientation = !!level; orientation < 4; orientation++) {
-                SubBand *b = &s->plane[p].band[level][orientation];
+                const SubBand *b = &s->plane[p].band[level][orientation];
 
                 const int q_idx = quants[level][orientation];
                 const uint64_t q_m = ((uint64_t)s->qmagic_lut[q_idx][0]) << 2;
@@ -618,7 +618,7 @@ static int count_hq_slice(SliceArgs *slice, int quant_idx)
 static int rate_control(AVCodecContext *avctx, void *arg)
 {
     SliceArgs *slice_dat = arg;
-    VC2EncContext *s = slice_dat->ctx;
+    const VC2EncContext *s = slice_dat->ctx;
     const int top = slice_dat->bits_ceil;
     const int bottom = slice_dat->bits_floor;
     int quant_buf[2] = {-1, -1};
@@ -724,7 +724,7 @@ static int calc_slice_sizes(VC2EncContext *s)
 static int encode_hq_slice(AVCodecContext *avctx, void *arg)
 {
     SliceArgs *slice_dat = arg;
-    VC2EncContext *s = slice_dat->ctx;
+    const VC2EncContext *s = slice_dat->ctx;
     PutBitContext *pb = &slice_dat->pb;
     const int slice_x = slice_dat->x;
     const int slice_y = slice_dat->y;
@@ -839,7 +839,7 @@ static int encode_slices(VC2EncContext *s)
 static int dwt_plane(AVCodecContext *avctx, void *arg)
 {
     TransformArgs *transform_dat = arg;
-    VC2EncContext *s = transform_dat->ctx;
+    const VC2EncContext *s = transform_dat->ctx;
     const void *frame_data = transform_dat->idata;
     const ptrdiff_t linesize = transform_dat->istride;
     const int field = transform_dat->field;
-- 
2.40.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".

[FFmpeg-devel] [PATCH 3/7] avcodec/vc2enc: Remove superfluous error message

[Andreas Rheinhardt]

ff_get_encode_buffer() already emits an error message of its own.
While just at it, also check for ret < 0 instead of just ret != 0.

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

diff --git a/libavcodec/vc2enc.c b/libavcodec/vc2enc.c
index 4dcf423ef1..f39f368181 100644
--- a/libavcodec/vc2enc.c
+++ b/libavcodec/vc2enc.c
@@ -918,10 +918,8 @@ static int encode_frame(VC2EncContext *s, AVPacket *avpkt, const AVFrame *frame,
     if (field < 2) {
         ret = ff_get_encode_buffer(s->avctx, avpkt,
                                    max_frame_bytes << s->interlaced, 0);
-        if (ret) {
-            av_log(s->avctx, AV_LOG_ERROR, "Error getting output packet.\n");
+        if (ret < 0)
             return ret;
-        }
         init_put_bits(&s->pb, avpkt->data, avpkt->size);
     }
 
-- 
2.40.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".

[FFmpeg-devel] [PATCH 4/7] avcodec/vc2enc: Fix slice length

[Andreas Rheinhardt]

args->bytes here already includes prefix_bytes (see
SSIZE_ROUND macro), so including it here again and
forgetting it when offsetting skip seems wrong.
This only works because prefix_bytes is currently
always zero in this encoder.
(This has been added in b88be742fac7a77a8095e8155ba8790db4b77568
without any reason.)

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/vc2enc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/vc2enc.c b/libavcodec/vc2enc.c
index f39f368181..3285218724 100644
--- a/libavcodec/vc2enc.c
+++ b/libavcodec/vc2enc.c
@@ -788,7 +788,7 @@ static int encode_slices(VC2EncContext *s)
     for (slice_y = 0; slice_y < s->num_y; slice_y++) {
         for (slice_x = 0; slice_x < s->num_x; slice_x++) {
             SliceArgs *args = &enc_args[s->num_x*slice_y + slice_x];
-            init_put_bits(&args->pb, buf + skip, args->bytes+s->prefix_bytes);
+            init_put_bits(&args->pb, buf + skip, args->bytes);
             skip += args->bytes;
         }
     }
-- 
2.40.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".

[FFmpeg-devel] [PATCH 5/7] avcodec/vc2enc: Avoid relocations for short strings

[Andreas Rheinhardt]

These strings are so short that they can be put directly
into the containing structure, avoiding the pointer
and putting it into .rodata.
Also use chars for interlaced and level while at it, as
these are so small.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/vc2enc.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/libavcodec/vc2enc.c b/libavcodec/vc2enc.c
index 3285218724..8b9641916a 100644
--- a/libavcodec/vc2enc.c
+++ b/libavcodec/vc2enc.c
@@ -41,8 +41,9 @@
 typedef struct VC2BaseVideoFormat {
     enum AVPixelFormat pix_fmt;
     AVRational time_base;
-    int width, height, interlaced, level;
-    const char *name;
+    int width, height;
+    char interlaced, level;
+    char name[13];
 } VC2BaseVideoFormat;
 
 static const VC2BaseVideoFormat base_video_fmts[] = {
-- 
2.40.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 5/7] avcodec/vc2enc: Avoid relocations for short strings

[James Almer]

On 5/27/2024 11:49 PM, Andreas Rheinhardt wrote:
> These strings are so short that they can be put directly
> into the containing structure, avoiding the pointer
> and putting it into .rodata.
> Also use chars for interlaced and level while at it, as
> these are so small.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
>   libavcodec/vc2enc.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/libavcodec/vc2enc.c b/libavcodec/vc2enc.c
> index 3285218724..8b9641916a 100644
> --- a/libavcodec/vc2enc.c
> +++ b/libavcodec/vc2enc.c
> @@ -41,8 +41,9 @@
>   typedef struct VC2BaseVideoFormat {
>       enum AVPixelFormat pix_fmt;
>       AVRational time_base;
> -    int width, height, interlaced, level;
> -    const char *name;
> +    int width, height;
> +    char interlaced, level;

Use a fixed size type like uint8_t and not char. Neither of these values 
are characters (interlace should strictly speaking be a bool, but afaict 
that's not portable).

> +    char name[13];
>   } VC2BaseVideoFormat;
>   
>   static const VC2BaseVideoFormat base_video_fmts[] = {
_______________________________________________
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 5/7] avcodec/vc2enc: Avoid relocations for short strings

[Rémi Denis-Courmont]

Le 28 mai 2024 06:07:41 GMT+03:00, James Almer <jamrial@gmail.com> a écrit :
>On 5/27/2024 11:49 PM, Andreas Rheinhardt wrote:
>> These strings are so short that they can be put directly
>> into the containing structure, avoiding the pointer
>> and putting it into .rodata.
>> Also use chars for interlaced and level while at it, as
>> these are so small.
>> 
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
>> ---
>>   libavcodec/vc2enc.c | 5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
>> 
>> diff --git a/libavcodec/vc2enc.c b/libavcodec/vc2enc.c
>> index 3285218724..8b9641916a 100644
>> --- a/libavcodec/vc2enc.c
>> +++ b/libavcodec/vc2enc.c
>> @@ -41,8 +41,9 @@
>>   typedef struct VC2BaseVideoFormat {
>>       enum AVPixelFormat pix_fmt;
>>       AVRational time_base;
>> -    int width, height, interlaced, level;
>> -    const char *name;
>> +    int width, height;
>> +    char interlaced, level;
>
>Use a fixed size type like uint8_t and not char.

The size of `char` is fixed to 8 bits *if* `uint8_t` exists though. It may be preferable to pick an explicitly signed or unsigned type still.
_______________________________________________
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 5/7] avcodec/vc2enc: Avoid relocations for short strings

[Lynne via ffmpeg-devel]

[-- Attachment #1.1.1.1: Type: text/plain, Size: 1056 bytes --]

On 28/05/2024 04:49, Andreas Rheinhardt wrote:
> These strings are so short that they can be put directly
> into the containing structure, avoiding the pointer
> and putting it into .rodata.
> Also use chars for interlaced and level while at it, as
> these are so small.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
>   libavcodec/vc2enc.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/libavcodec/vc2enc.c b/libavcodec/vc2enc.c
> index 3285218724..8b9641916a 100644
> --- a/libavcodec/vc2enc.c
> +++ b/libavcodec/vc2enc.c
> @@ -41,8 +41,9 @@
>   typedef struct VC2BaseVideoFormat {
>       enum AVPixelFormat pix_fmt;
>       AVRational time_base;
> -    int width, height, interlaced, level;
> -    const char *name;
> +    int width, height;
> +    char interlaced, level;
> +    char name[13];
>   } VC2BaseVideoFormat;
>   
>   static const VC2BaseVideoFormat base_video_fmts[] = {

Patchset LGTM if interlaced and level are uint8_t like other commented.

[-- Attachment #1.1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 637 bytes --]

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 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".

[FFmpeg-devel] [PATCH 6/7] avcodec/vc2enc: Move transient PutBitContext from ctx to stack

[Andreas Rheinhardt]

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

diff --git a/libavcodec/vc2enc.c b/libavcodec/vc2enc.c
index 8b9641916a..e72c15e6f2 100644
--- a/libavcodec/vc2enc.c
+++ b/libavcodec/vc2enc.c
@@ -105,9 +105,11 @@ typedef struct Plane {
 } Plane;
 
 typedef struct SliceArgs {
-    PutBitContext pb;
-    int cache[DIRAC_MAX_QUANT_INDEX];
     const struct VC2EncContext *ctx;
+    union {
+        int cache[DIRAC_MAX_QUANT_INDEX];
+        uint8_t *buf;
+    };
     int x;
     int y;
     int quant_idx;
@@ -724,9 +726,9 @@ static int calc_slice_sizes(VC2EncContext *s)
 /* VC-2 13.5.3 - hq_slice */
 static int encode_hq_slice(AVCodecContext *avctx, void *arg)
 {
-    SliceArgs *slice_dat = arg;
+    const SliceArgs *slice_dat = arg;
     const VC2EncContext *s = slice_dat->ctx;
-    PutBitContext *pb = &slice_dat->pb;
+    PutBitContext pb0, *const pb = &pb0;
     const int slice_x = slice_dat->x;
     const int slice_y = slice_dat->y;
     const int quant_idx = slice_dat->quant_idx;
@@ -735,8 +737,9 @@ static int encode_hq_slice(AVCodecContext *avctx, void *arg)
     int p, level, orientation;
 
     /* The reference decoder ignores it, and its typical length is 0 */
-    memset(put_bits_ptr(pb), 0, s->prefix_bytes);
-    skip_put_bytes(pb, s->prefix_bytes);
+    memset(slice_dat->buf, 0, s->prefix_bytes);
+
+    init_put_bits(pb, slice_dat->buf + s->prefix_bytes, slice_dat->bytes - s->prefix_bytes);
 
     put_bits(pb, 8, quant_idx);
 
@@ -789,7 +792,7 @@ static int encode_slices(VC2EncContext *s)
     for (slice_y = 0; slice_y < s->num_y; slice_y++) {
         for (slice_x = 0; slice_x < s->num_x; slice_x++) {
             SliceArgs *args = &enc_args[s->num_x*slice_y + slice_x];
-            init_put_bits(&args->pb, buf + skip, args->bytes);
+            args->buf = buf + skip;
             skip += args->bytes;
         }
     }
-- 
2.40.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".

[FFmpeg-devel] [PATCH 7/7] avcodec/vc2enc: Use already available AVPixFmtDescriptor

[Andreas Rheinhardt]

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

diff --git a/libavcodec/vc2enc.c b/libavcodec/vc2enc.c
index e72c15e6f2..1d0754c09b 100644
--- a/libavcodec/vc2enc.c
+++ b/libavcodec/vc2enc.c
@@ -1026,9 +1026,9 @@ static av_cold int vc2_encode_init(AVCodecContext *avctx)
 {
     Plane *p;
     SubBand *b;
-    int i, level, o, shift, ret;
-    const AVPixFmtDescriptor *fmt = av_pix_fmt_desc_get(avctx->pix_fmt);
-    const int depth = fmt->comp[0].depth;
+    int i, level, o, shift;
+    const AVPixFmtDescriptor *pixdesc;
+    int depth;
     VC2EncContext *s = avctx->priv_data;
 
     s->picture_number = 0;
@@ -1100,12 +1100,13 @@ static av_cold int vc2_encode_init(AVCodecContext *avctx)
                s->base_vf, base_video_fmts[s->base_vf].name);
     }
 
+    pixdesc = av_pix_fmt_desc_get(avctx->pix_fmt);
     /* Chroma subsampling */
-    ret = av_pix_fmt_get_chroma_sub_sample(avctx->pix_fmt, &s->chroma_x_shift, &s->chroma_y_shift);
-    if (ret)
-        return ret;
+    s->chroma_x_shift = pixdesc->log2_chroma_w;
+    s->chroma_y_shift = pixdesc->log2_chroma_h;
 
     /* Bit depth and color range index */
+    depth = pixdesc->comp[0].depth;
     if (depth == 8 && avctx->color_range == AVCOL_RANGE_JPEG) {
         s->bpp = 1;
         s->bpp_idx = 1;
-- 
2.40.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 1/7] avcodec/vc2enc: Avoid void* where possible

[Andreas Rheinhardt]

Andreas Rheinhardt:
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
>  libavcodec/vc2enc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/libavcodec/vc2enc.c b/libavcodec/vc2enc.c
> index 365d43146d..b496f67d3a 100644
> --- a/libavcodec/vc2enc.c
> +++ b/libavcodec/vc2enc.c
> @@ -106,7 +106,7 @@ typedef struct Plane {
>  typedef struct SliceArgs {
>      PutBitContext pb;
>      int cache[DIRAC_MAX_QUANT_INDEX];
> -    void *ctx;
> +    struct VC2EncContext *ctx;
>      int x;
>      int y;
>      int quant_idx;
> @@ -116,7 +116,7 @@ typedef struct SliceArgs {
>  } SliceArgs;
>  
>  typedef struct TransformArgs {
> -    void *ctx;
> +    struct VC2EncContext *ctx;
>      Plane *plane;
>      const void *idata;
>      ptrdiff_t istride;

Will apply the patchset with 5/7 using uint8_t tomorrow unless there are
objections.

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