| 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 | expand |
| 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 |
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.
> -----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 --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 = {
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(-)