diff mbox series

[FFmpeg-devel,1/2] avcodec/dvdsub: Don't dump images to disk based on DEBUG define

Message ID DM8P223MB036567ED3F5798F200A3CCDDBA669@DM8P223MB0365.NAMP223.PROD.OUTLOOK.COM
State New
Headers show
Series [FFmpeg-devel,1/2] avcodec/dvdsub: Don't dump images to disk based on DEBUG define
Related show

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished
andriy/make_ppc success Make finished
andriy/make_fate_ppc success Make fate finished

Commit Message

Soft Works Nov. 29, 2021, 8:17 p.m. UTC
It's been a regular annoyance.
Introduce a debug-only parameter for this.

Signed-off-by: softworkz <softworkz@hotmail.com>
---
 libavcodec/dvdsubdec.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

Comments

Anton Khirnov Dec. 6, 2021, 8:32 a.m. UTC | #1
Quoting Soft Works (2021-11-29 21:17:32)
> It's been a regular annoyance.
> Introduce a debug-only parameter for this.
> 
> Signed-off-by: softworkz <softworkz@hotmail.com>
> ---
>  libavcodec/dvdsubdec.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)

Does anyone actually find that code useful? Apparently it's been there
since 2005, and our standards for what is acceptable have changed
somewhat.

I would just drop it.
Soft Works Dec. 6, 2021, 4:54 p.m. UTC | #2
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Anton
> Khirnov
> Sent: Monday, December 6, 2021 9:33 AM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH 1/2] avcodec/dvdsub: Don't dump images to
> disk based on DEBUG define
> 
> Quoting Soft Works (2021-11-29 21:17:32)
> > It's been a regular annoyance.
> > Introduce a debug-only parameter for this.
> >
> > Signed-off-by: softworkz <softworkz@hotmail.com>
> > ---
> >  libavcodec/dvdsubdec.c | 9 +++++++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> Does anyone actually find that code useful? Apparently it's been there
> since 2005, and our standards for what is acceptable have changed
> somewhat.
> 
> I would just drop it.`

I think that it can be useful at times to dump the individual subtitle
images to disk. My primary point is that this shouldn't happen based
on whether ffmpeg is compiled with DEBUG or not.

Actually that's functionality that can be implemented in a new subtitle 
filter, which would be an ultimate justification to drop this from 
dvdsub and dvbsub.

Maybe we can just merge these two patches as an interim solution?

Thanks,
softworkz
diff mbox series

Patch

diff --git a/libavcodec/dvdsubdec.c b/libavcodec/dvdsubdec.c
index 52259f0730..c0f796068e 100644
--- a/libavcodec/dvdsubdec.c
+++ b/libavcodec/dvdsubdec.c
@@ -44,6 +44,7 @@  typedef struct DVDSubContext
   uint8_t  used_color[256];
 #ifdef DEBUG
   int sub_id;
+  int dump_imgs;
 #endif
 } DVDSubContext;
 
@@ -597,8 +598,9 @@  static int dvdsub_decode(AVCodecContext *avctx,
     ff_dlog(NULL, "start=%d ms end =%d ms\n",
             sub->start_display_time,
             sub->end_display_time);
-    ppm_save(ppm_name, sub->rects[0]->data[0],
-             sub->rects[0]->w, sub->rects[0]->h, (uint32_t*) sub->rects[0]->data[1]);
+    if (ctx->dump_imgs)
+        ppm_save(ppm_name, sub->rects[0]->data[0],
+                 sub->rects[0]->w, sub->rects[0]->h, (uint32_t*) sub->rects[0]->data[1]);
     }
 #endif
 
@@ -745,6 +747,9 @@  static const AVOption options[] = {
     { "palette", "set the global palette", OFFSET(palette_str), AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, SD },
     { "ifo_palette", "obtain the global palette from .IFO file", OFFSET(ifo_str), AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, SD },
     { "forced_subs_only", "Only show forced subtitles", OFFSET(forced_subs_only), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, SD},
+#ifdef DEBUG
+    { "dump_imgs", "Dump subtitle images to disk", OFFSET(dump_imgs), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, SD},
+#endif
     { NULL }
 };
 static const AVClass dvdsub_class = {