[FFmpeg-devel] [PATCH 1/4] lavc/dnxhdenc: eliminate dead code

[FFmpeg-devel] [PATCH 1/4] lavc/dnxhdenc: eliminate dead code

[Rémi Denis-Courmont]

dct_quantize cannot be NULL here. The call to ff_dct_encode_init() would
initialized it to a non-NULL value already.
---
 libavcodec/dnxhdenc.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/libavcodec/dnxhdenc.c b/libavcodec/dnxhdenc.c
index 0cb25d7714..681fb738c0 100644
--- a/libavcodec/dnxhdenc.c
+++ b/libavcodec/dnxhdenc.c
@@ -430,9 +430,6 @@ static av_cold int dnxhd_encode_init(AVCodecContext *avctx)
     if (ctx->profile != AV_PROFILE_DNXHD)
         ff_videodsp_init(&ctx->m.vdsp, ctx->bit_depth);
 
-    if (!ctx->m.dct_quantize)
-        ctx->m.dct_quantize = ff_dct_quantize_c;
-
     if (ctx->is_444 || ctx->profile == AV_PROFILE_DNXHR_HQX) {
         ctx->m.dct_quantize     = dnxhd_10bit_dct_quantize_444;
         ctx->get_pixels_8x4_sym = dnxhd_10bit_get_pixels_8x4_sym;
-- 
2.45.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/4] lavc/mpvenc: reorder code

[Rémi Denis-Courmont]

No functional changes.
---
 libavcodec/mpegvideo_enc.c | 166 ++++++++++++++++++-------------------
 1 file changed, 83 insertions(+), 83 deletions(-)

diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c
index 82bab43e14..dd92f0a3af 100644
--- a/libavcodec/mpegvideo_enc.c
+++ b/libavcodec/mpegvideo_enc.c
@@ -246,6 +246,89 @@ void ff_init_qscale_tab(MpegEncContext *s)
     }
 }
 
+int ff_dct_quantize_c(MpegEncContext *s,
+                        int16_t *block, int n,
+                        int qscale, int *overflow)
+{
+    int i, j, level, last_non_zero, q, start_i;
+    const int *qmat;
+    const uint8_t *scantable;
+    int bias;
+    int max=0;
+    unsigned int threshold1, threshold2;
+
+    s->fdsp.fdct(block);
+
+    if(s->dct_error_sum)
+        s->denoise_dct(s, block);
+
+    if (s->mb_intra) {
+        scantable= s->intra_scantable.scantable;
+        if (!s->h263_aic) {
+            if (n < 4)
+                q = s->y_dc_scale;
+            else
+                q = s->c_dc_scale;
+            q = q << 3;
+        } else
+            /* For AIC we skip quant/dequant of INTRADC */
+            q = 1 << 3;
+
+        /* note: block[0] is assumed to be positive */
+        block[0] = (block[0] + (q >> 1)) / q;
+        start_i = 1;
+        last_non_zero = 0;
+        qmat = n < 4 ? s->q_intra_matrix[qscale] : s->q_chroma_intra_matrix[qscale];
+        bias= s->intra_quant_bias*(1<<(QMAT_SHIFT - QUANT_BIAS_SHIFT));
+    } else {
+        scantable= s->inter_scantable.scantable;
+        start_i = 0;
+        last_non_zero = -1;
+        qmat = s->q_inter_matrix[qscale];
+        bias= s->inter_quant_bias*(1<<(QMAT_SHIFT - QUANT_BIAS_SHIFT));
+    }
+    threshold1= (1<<QMAT_SHIFT) - bias - 1;
+    threshold2= (threshold1<<1);
+    for(i=63;i>=start_i;i--) {
+        j = scantable[i];
+        level = block[j] * qmat[j];
+
+        if(((unsigned)(level+threshold1))>threshold2){
+            last_non_zero = i;
+            break;
+        }else{
+            block[j]=0;
+        }
+    }
+    for(i=start_i; i<=last_non_zero; i++) {
+        j = scantable[i];
+        level = block[j] * qmat[j];
+
+//        if(   bias+level >= (1<<QMAT_SHIFT)
+//           || bias-level >= (1<<QMAT_SHIFT)){
+        if(((unsigned)(level+threshold1))>threshold2){
+            if(level>0){
+                level= (bias + level)>>QMAT_SHIFT;
+                block[j]= level;
+            }else{
+                level= (bias - level)>>QMAT_SHIFT;
+                block[j]= -level;
+            }
+            max |=level;
+        }else{
+            block[j]=0;
+        }
+    }
+    *overflow= s->max_qcoeff < max; //overflow might have happened
+
+    /* we need this permutation so that we correct the IDCT, we only permute the !=0 elements */
+    if (s->idsp.perm_type != FF_IDCT_PERM_NONE)
+        ff_block_permute(block, s->idsp.idct_permutation,
+                      scantable, last_non_zero);
+
+    return last_non_zero;
+}
+
 static void update_duplicate_context_after_me(MpegEncContext *dst,
                                               const MpegEncContext *src)
 {
@@ -4565,86 +4648,3 @@ void ff_block_permute(int16_t *block, uint8_t *permutation,
         block[perm_j] = temp[j];
     }
 }
-
-int ff_dct_quantize_c(MpegEncContext *s,
-                        int16_t *block, int n,
-                        int qscale, int *overflow)
-{
-    int i, j, level, last_non_zero, q, start_i;
-    const int *qmat;
-    const uint8_t *scantable;
-    int bias;
-    int max=0;
-    unsigned int threshold1, threshold2;
-
-    s->fdsp.fdct(block);
-
-    if(s->dct_error_sum)
-        s->denoise_dct(s, block);
-
-    if (s->mb_intra) {
-        scantable= s->intra_scantable.scantable;
-        if (!s->h263_aic) {
-            if (n < 4)
-                q = s->y_dc_scale;
-            else
-                q = s->c_dc_scale;
-            q = q << 3;
-        } else
-            /* For AIC we skip quant/dequant of INTRADC */
-            q = 1 << 3;
-
-        /* note: block[0] is assumed to be positive */
-        block[0] = (block[0] + (q >> 1)) / q;
-        start_i = 1;
-        last_non_zero = 0;
-        qmat = n < 4 ? s->q_intra_matrix[qscale] : s->q_chroma_intra_matrix[qscale];
-        bias= s->intra_quant_bias*(1<<(QMAT_SHIFT - QUANT_BIAS_SHIFT));
-    } else {
-        scantable= s->inter_scantable.scantable;
-        start_i = 0;
-        last_non_zero = -1;
-        qmat = s->q_inter_matrix[qscale];
-        bias= s->inter_quant_bias*(1<<(QMAT_SHIFT - QUANT_BIAS_SHIFT));
-    }
-    threshold1= (1<<QMAT_SHIFT) - bias - 1;
-    threshold2= (threshold1<<1);
-    for(i=63;i>=start_i;i--) {
-        j = scantable[i];
-        level = block[j] * qmat[j];
-
-        if(((unsigned)(level+threshold1))>threshold2){
-            last_non_zero = i;
-            break;
-        }else{
-            block[j]=0;
-        }
-    }
-    for(i=start_i; i<=last_non_zero; i++) {
-        j = scantable[i];
-        level = block[j] * qmat[j];
-
-//        if(   bias+level >= (1<<QMAT_SHIFT)
-//           || bias-level >= (1<<QMAT_SHIFT)){
-        if(((unsigned)(level+threshold1))>threshold2){
-            if(level>0){
-                level= (bias + level)>>QMAT_SHIFT;
-                block[j]= level;
-            }else{
-                level= (bias - level)>>QMAT_SHIFT;
-                block[j]= -level;
-            }
-            max |=level;
-        }else{
-            block[j]=0;
-        }
-    }
-    *overflow= s->max_qcoeff < max; //overflow might have happened
-
-    /* we need this permutation so that we correct the IDCT, we only permute the !=0 elements */
-    if (s->idsp.perm_type != FF_IDCT_PERM_NONE)
-        ff_block_permute(block, s->idsp.idct_permutation,
-                      scantable, last_non_zero);
-
-    return last_non_zero;
-}
-- 
2.45.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/4] lavc/mpv_enc: privatize ff_dct_quantize_c

[Rémi Denis-Courmont]

---
 libavcodec/mpegvideo_enc.c | 9 ++++-----
 libavcodec/mpegvideoenc.h  | 1 -
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c
index dd92f0a3af..af04db70d8 100644
--- a/libavcodec/mpegvideo_enc.c
+++ b/libavcodec/mpegvideo_enc.c
@@ -246,9 +246,8 @@ void ff_init_qscale_tab(MpegEncContext *s)
     }
 }
 
-int ff_dct_quantize_c(MpegEncContext *s,
-                        int16_t *block, int n,
-                        int qscale, int *overflow)
+static int dct_quantize_c(MpegEncContext *s, int16_t *block, int n,
+                          int qscale, int *overflow)
 {
     int i, j, level, last_non_zero, q, start_i;
     const int *qmat;
@@ -379,7 +378,7 @@ av_cold int ff_dct_encode_init(MpegEncContext *s)
     if (CONFIG_H263_ENCODER)
         ff_h263dsp_init(&s->h263dsp);
     if (!s->dct_quantize)
-        s->dct_quantize = ff_dct_quantize_c;
+        s->dct_quantize = dct_quantize_c;
     if (!s->denoise_dct)
         s->denoise_dct  = denoise_dct_c;
     s->fast_dct_quantize = s->dct_quantize;
@@ -2527,7 +2526,7 @@ static av_always_inline void encode_mb_internal(MpegEncContext *s,
     }
 
     // non c quantize code returns incorrect block_last_index FIXME
-    if (s->alternate_scan && s->dct_quantize != ff_dct_quantize_c) {
+    if (s->alternate_scan && s->dct_quantize != dct_quantize_c) {
         for (i = 0; i < mb_block_count; i++) {
             int j;
             if (s->block_last_index[i] > 0) {
diff --git a/libavcodec/mpegvideoenc.h b/libavcodec/mpegvideoenc.h
index c20ea500eb..169ed1dc3b 100644
--- a/libavcodec/mpegvideoenc.h
+++ b/libavcodec/mpegvideoenc.h
@@ -148,7 +148,6 @@ void ff_write_quant_matrix(PutBitContext *pb, uint16_t *matrix);
 int ff_dct_encode_init(MpegEncContext *s);
 void ff_dct_encode_init_x86(MpegEncContext *s);
 
-int ff_dct_quantize_c(MpegEncContext *s, int16_t *block, int n, int qscale, int *overflow);
 void ff_convert_matrix(MpegEncContext *s, int (*qmat)[64], uint16_t (*qmat16)[2][64],
                        const uint16_t *quant_matrix, int bias, int qmin, int qmax, int intra);
 
-- 
2.45.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/4] lavc/mpv_enc: rationalize dct_quantize init

[Rémi Denis-Courmont]

On entry the function pointer is always NULL. We just need to set the
pointer before probing x86 CPU optimisations.

Note that there is a third code path setting this function pointer, but
it does so *after* calling this function: the DNxHD encoder.
---
 libavcodec/mpegvideo_enc.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c
index af04db70d8..49eed6301b 100644
--- a/libavcodec/mpegvideo_enc.c
+++ b/libavcodec/mpegvideo_enc.c
@@ -371,14 +371,13 @@ static void mpv_encode_defaults(MpegEncContext *s)
 
 av_cold int ff_dct_encode_init(MpegEncContext *s)
 {
+    s->dct_quantize = dct_quantize_c;
 #if ARCH_X86
     ff_dct_encode_init_x86(s);
 #endif
 
     if (CONFIG_H263_ENCODER)
         ff_h263dsp_init(&s->h263dsp);
-    if (!s->dct_quantize)
-        s->dct_quantize = dct_quantize_c;
     if (!s->denoise_dct)
         s->denoise_dct  = denoise_dct_c;
     s->fast_dct_quantize = s->dct_quantize;
-- 
2.45.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 4/4] lavc/mpv_enc: rationalize dct_quantize init

[Andreas Rheinhardt]

Rémi Denis-Courmont:
> On entry the function pointer is always NULL. We just need to set the
> pointer before probing x86 CPU optimisations.
> 

Incorrect:
https://github.com/mkver/FFmpeg/commit/d22d4ee8419788f9bb239a21e276cebce0891737
(see also
https://github.com/mkver/FFmpeg/commits/mpegvideo_pool/?after=d2dfcf8f226c3708f3df080aed043ff4aa26e7cd+34
which contains the equivalent of patches 1+2 and a better version of #4)

> Note that there is a third code path setting this function pointer, but
> it does so *after* calling this function: the DNxHD encoder.
> ---
>  libavcodec/mpegvideo_enc.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c
> index af04db70d8..49eed6301b 100644
> --- a/libavcodec/mpegvideo_enc.c
> +++ b/libavcodec/mpegvideo_enc.c
> @@ -371,14 +371,13 @@ static void mpv_encode_defaults(MpegEncContext *s)
>  
>  av_cold int ff_dct_encode_init(MpegEncContext *s)
>  {
> +    s->dct_quantize = dct_quantize_c;
>  #if ARCH_X86
>      ff_dct_encode_init_x86(s);
>  #endif
>  
>      if (CONFIG_H263_ENCODER)
>          ff_h263dsp_init(&s->h263dsp);
> -    if (!s->dct_quantize)
> -        s->dct_quantize = dct_quantize_c;
>      if (!s->denoise_dct)
>          s->denoise_dct  = denoise_dct_c;
>      s->fast_dct_quantize = s->dct_quantize;

_______________________________________________
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] lavc/mpv_enc: rationalize dct_quantize init

[Andreas Rheinhardt]

Andreas Rheinhardt:
> Rémi Denis-Courmont:
>> On entry the function pointer is always NULL. We just need to set the
>> pointer before probing x86 CPU optimisations.
>>
> 
> Incorrect:
> https://github.com/mkver/FFmpeg/commit/d22d4ee8419788f9bb239a21e276cebce0891737
> (see also
> https://github.com/mkver/FFmpeg/commits/mpegvideo_pool/?after=d2dfcf8f226c3708f3df080aed043ff4aa26e7cd+34
> which contains the equivalent of patches 1+2 and a better version of #4)

Wait, I see that you only set dct_quantize unconditionally. So your
claim that dct_quantize is always NULL on entry is correct. But setting
only one of the two in the ordinary way is insufficient.

> 
>> Note that there is a third code path setting this function pointer, but
>> it does so *after* calling this function: the DNxHD encoder.
>> ---
>>  libavcodec/mpegvideo_enc.c | 3 +--
>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c
>> index af04db70d8..49eed6301b 100644
>> --- a/libavcodec/mpegvideo_enc.c
>> +++ b/libavcodec/mpegvideo_enc.c
>> @@ -371,14 +371,13 @@ static void mpv_encode_defaults(MpegEncContext *s)
>>  
>>  av_cold int ff_dct_encode_init(MpegEncContext *s)
>>  {
>> +    s->dct_quantize = dct_quantize_c;
>>  #if ARCH_X86
>>      ff_dct_encode_init_x86(s);
>>  #endif
>>  
>>      if (CONFIG_H263_ENCODER)
>>          ff_h263dsp_init(&s->h263dsp);
>> -    if (!s->dct_quantize)
>> -        s->dct_quantize = dct_quantize_c;
>>      if (!s->denoise_dct)
>>          s->denoise_dct  = denoise_dct_c;
>>      s->fast_dct_quantize = s->dct_quantize;
> 
> _______________________________________________
> 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 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] lavc/mpv_enc: rationalize dct_quantize init

[Rémi Denis-Courmont]

Le sunnuntaina 9. kesäkuuta 2024, 11.39.55 EEST Andreas Rheinhardt a écrit :
> Andreas Rheinhardt:
> > Rémi Denis-Courmont:
> >> On entry the function pointer is always NULL. We just need to set the
> >> pointer before probing x86 CPU optimisations.
> > 
> > Incorrect:
> > https://github.com/mkver/FFmpeg/commit/d22d4ee8419788f9bb239a21e276cebce08
> > 91737 (see also
> > https://github.com/mkver/FFmpeg/commits/mpegvideo_pool/?after=d2dfcf8f226c
> > 3708f3df080aed043ff4aa26e7cd+34 which contains the equivalent of patches
> > 1+2 and a better version of #4)
> Wait, I see that you only set dct_quantize unconditionally. So your
> claim that dct_quantize is always NULL on entry is correct. But setting
> only one of the two in the ordinary way is insufficient.

In what way is it insufficient? The nullity of dct_unquantize is not used 
anywhere that I can see. And if it were used, it would behave different 
depending on the availability of MMX which would most certainly not work.

Of course it would be *better* to also clean-up the denoise_dct pointer. If 
you have a better patchset coming, I can drop this one but otherwise I don't 
get your point here.

-- 
Rémi Denis-Courmont
http://www.remlab.net/



_______________________________________________
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] lavc/mpv_enc: rationalize dct_quantize init

[Andreas Rheinhardt]

Rémi Denis-Courmont:
> Le sunnuntaina 9. kesäkuuta 2024, 11.39.55 EEST Andreas Rheinhardt a écrit :
>> Andreas Rheinhardt:
>>> Rémi Denis-Courmont:
>>>> On entry the function pointer is always NULL. We just need to set the
>>>> pointer before probing x86 CPU optimisations.
>>>
>>> Incorrect:
>>> https://github.com/mkver/FFmpeg/commit/d22d4ee8419788f9bb239a21e276cebce08
>>> 91737 (see also
>>> https://github.com/mkver/FFmpeg/commits/mpegvideo_pool/?after=d2dfcf8f226c
>>> 3708f3df080aed043ff4aa26e7cd+34 which contains the equivalent of patches
>>> 1+2 and a better version of #4)
>> Wait, I see that you only set dct_quantize unconditionally. So your
>> claim that dct_quantize is always NULL on entry is correct. But setting
>> only one of the two in the ordinary way is insufficient.
> 
> In what way is it insufficient? The nullity of dct_unquantize is not used 
> anywhere that I can see. And if it were used, it would behave different 
> depending on the availability of MMX which would most certainly not work.
> 

The two refers to the functions that are set by ff_dct_encode_init():
dct_quantize and denoise_dct (the other one). It does not involve
dct_unquantize at all.

> Of course it would be *better* to also clean-up the denoise_dct pointer. If 
> you have a better patchset coming, I can drop this one but otherwise I don't 
> get your point here.
> 

Ok. Will send my patchset (it is currently in a branch on top of my
other mpegvideo patchset, but actually it is not logically dependent on
the rest).

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

Re: [FFmpeg-devel] [PATCH 4/4] lavc/mpv_enc: rationalize dct_quantize init

[Rémi Denis-Courmont]

Le sunnuntaina 9. kesäkuuta 2024, 12.26.50 EEST Andreas Rheinhardt a écrit :
> The two refers to the functions that are set by ff_dct_encode_init():
> dct_quantize and denoise_dct (the other one). It does not involve
> dct_unquantize at all.

I don't think that the patchset implies any involvement of dct_unquantize 
though. There just so happens to be a contemporary patchset involving H.263 
DCT unquantize. So I still don't get your point.

-- 
レミ・デニ-クールモン
http://www.remlab.net/



_______________________________________________
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] lavc/mpv_enc: rationalize dct_quantize init

[Andreas Rheinhardt]

Rémi Denis-Courmont:
> Le sunnuntaina 9. kesäkuuta 2024, 12.26.50 EEST Andreas Rheinhardt a écrit :
>> The two refers to the functions that are set by ff_dct_encode_init():
>> dct_quantize and denoise_dct (the other one). It does not involve
>> dct_unquantize at all.
> 
> I don't think that the patchset implies any involvement of dct_unquantize 
> though. There just so happens to be a contemporary patchset involving H.263 
> DCT unquantize. So I still don't get your point.
> 

You wrote about the "nullity of dct_unquantize" and it not being used at
all. So I inferred that you believe that my "the two" (as in "one of the
two") are dct_quantize and dct_unquantize. Which is not what I meant. I
meant dct_quantize and denoise_dct. So I don't get your latest answer at
all, given that I already stated that I did not refer to dct_unquantize
at all.

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