diff mbox series

[FFmpeg-devel,20/20] avcodec/snowdec: Remove debug code

Message ID AS8P250MB0744BD5976AE61EAD097E9B68F209@AS8P250MB0744.EURP250.PROD.OUTLOOK.COM
State New
Headers show
Series [FFmpeg-devel,1/8] configure: Remove dependencies of inexistant rtjpeg decoder | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Andreas Rheinhardt Oct. 10, 2022, 3:13 a.m. UTC 
The Snow decoder checks two bits of AVCodecContext.debug
via numerical constants, not defines. One of these constants
(512) used to be equivalent to FF_DEBUG_PTS which has been
removed in 302554835e39b79b977ed60c9afe81b44590dfef
(merged in 6e69525e6984d51165de0b17b796bbc29f9dd6e7).

It is unlikely that 512 was intended to be FF_DEBUG_PTS,
as it has nothing do to with PTS; instead it makes
certain parts of the code behave like it does for keyframes
even if the current frame is not a keyframe.

Whatever it might have been intended for, it is almost certainly
unused now. This commit therefore removes said checks.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
Actually, I'd like to remove the other debug code (debug&2048), too;
this would entail removing the mconly_picture, yet I wonder
about the size of scratchbuf: Before
a4ce3706595edd9b537861f0e5447e31babf2100 it's size was affected
by the linesize of mconly_picture for encoders, too. Is it possible
that these codecs are built on the assumption that the linesize
of all frames stays the same if the dimensions stay the same,
so that simply removing mconly_picture would leave scratchbuf
too small?
(Of course I am aware that the pixel format check would need
that is currently performed with mconly_picture would need
to be replaced by something similar. That should be easy.)

 libavcodec/snow.h    | 2 +-
 libavcodec/snowdec.c | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

Comments

Michael Niedermayer Oct. 10, 2022, 4:26 p.m. UTC | #1 
On Mon, Oct 10, 2022 at 05:13:10AM +0200, Andreas Rheinhardt wrote:
> The Snow decoder checks two bits of AVCodecContext.debug
> via numerical constants, not defines. One of these constants
> (512) used to be equivalent to FF_DEBUG_PTS which has been
> removed in 302554835e39b79b977ed60c9afe81b44590dfef
> (merged in 6e69525e6984d51165de0b17b796bbc29f9dd6e7).
> 
> It is unlikely that 512 was intended to be FF_DEBUG_PTS,
> as it has nothing do to with PTS; instead it makes
> certain parts of the code behave like it does for keyframes
> even if the current frame is not a keyframe.

why not fix it instead of removing ?
you can check what it did by looking at the code when it was added.

thx

[...]
diff mbox series

Patch

diff --git a/libavcodec/snow.h b/libavcodec/snow.h
index 8da3d99885..0c59fb7ec4 100644
--- a/libavcodec/snow.h
+++ b/libavcodec/snow.h
@@ -418,7 +418,7 @@  static av_always_inline void predict_slice(SnowContext *s, IDWTELEM *buf, int pl
     int w= p->width;
     int h= p->height;
     av_assert2(s->chroma_h_shift == s->chroma_v_shift); // obmc params assume squares
-    if(s->keyframe || (s->avctx->debug&512)){
+    if (s->keyframe) {
         if(mb_y==mb_h)
             return;
 
diff --git a/libavcodec/snowdec.c b/libavcodec/snowdec.c
index 13948398b0..60eb9a7f52 100644
--- a/libavcodec/snowdec.c
+++ b/libavcodec/snowdec.c
@@ -191,7 +191,7 @@  static av_always_inline void predict_slice_buffered(SnowContext *s, slice_buffer
     int w= p->width;
     int h= p->height;
 
-    if(s->keyframe || (s->avctx->debug&512)){
+    if (s->keyframe) {
         if(mb_y==mb_h)
             return;
 
@@ -694,7 +694,7 @@  static int decode_frame(AVCodecContext *avctx, AVFrame *picture,
             int slice_starty = block_h*mb_y;
             int slice_h = block_h*(mb_y+1);
 
-            if (!(s->keyframe || s->avctx->debug&512)){
+            if (!s->keyframe) {
                 slice_starty = FFMAX(0, slice_starty - (block_h >> 1));
                 slice_h -= (block_h >> 1);
             }
@@ -709,7 +709,7 @@  static int decode_frame(AVCodecContext *avctx, AVFrame *picture,
                     const int extra= 3;
                     start_y = (mb_y ? ((block_h * our_mb_start) >> (s->spatial_decomposition_count - level)) + s->spatial_decomposition_count - level + extra: 0);
                     end_y = (((block_h * our_mb_end) >> (s->spatial_decomposition_count - level)) + s->spatial_decomposition_count - level + extra);
-                    if (!(s->keyframe || s->avctx->debug&512)){
+                    if (!s->keyframe) {
                         start_y = FFMAX(0, start_y - (block_h >> (1+s->spatial_decomposition_count - level)));
                         end_y = FFMAX(0, end_y - (block_h >> (1+s->spatial_decomposition_count - level)));
                     }