mbox series

[FFmpeg-devel,v2,0/2] avcodec/dvbsubdec, dvdsubdec: don't dump images to disk based on DEBUG define

Message ID pull.17.v2.ffstaging.FFmpeg.1641844533.ffmpegagent@gmail.com
Headers show
Series avcodec/dvbsubdec, dvdsubdec: don't dump images to disk based on DEBUG define | expand

Message

softworkz Jan. 10, 2022, 7:55 p.m. UTC
It's annoying and unexpected, but still useful at times (as I've realized
just recently).

This is a follow-up to the earlier submission here:
https://www.mail-archive.com/ffmpeg-devel@ffmpeg.org/msg128080.html

There has been a comment from Anton, questioning whether the dump-feature is
useful. Meanwhile I came to the conclusion that it can be useful in-fact. It
just shouldn't happen automatically when DEBUG is defined. That's what these
patches do.

I also added fixes for the fopen() call.

softworkz (2):
  avcodec/dvdsubdec,dvbsubdec: don't dump images to disk based on DEBUG
    define
  avcodec/dvdsubdec,dvbsubdec: fix writing ppm

 libavcodec/dvbsubdec.c | 20 +++++++++++++-------
 libavcodec/dvdsubdec.c | 11 ++++++++---
 2 files changed, 21 insertions(+), 10 deletions(-)


base-commit: 6c4074e4234edacfb3f37184fd68771df3cb2b7f
Published-As: https://github.com/ffstaging/FFmpeg/releases/tag/pr-ffstaging-17%2Fsoftworkz%2Fsubmit_dvb_subs-v2
Fetch-It-Via: git fetch https://github.com/ffstaging/FFmpeg pr-ffstaging-17/softworkz/submit_dvb_subs-v2
Pull-Request: https://github.com/ffstaging/FFmpeg/pull/17

Range-diff vs v1:

 1:  6ca8905c3d ! 1:  2f12ac7f1f avcodec/dvbsubdec: don't dump images to disk based on DEBUG define
     @@ Metadata
      Author: softworkz <softworkz@hotmail.com>
      
       ## Commit message ##
     -    avcodec/dvbsubdec: don't dump images to disk based on DEBUG define
     +    avcodec/dvdsubdec,dvbsubdec: don't dump images to disk based on DEBUG define
      
          It's been a regular annoyance.
          Introduce a debug-only parameter for this.
     @@ libavcodec/dvbsubdec.c: static const AVOption options[] = {
           {NULL}
       };
       static const AVClass dvbsubdec_class = {
     +
     + ## libavcodec/dvdsubdec.c ##
     +@@ libavcodec/dvdsubdec.c: typedef struct DVDSubContext
     +   uint8_t  used_color[256];
     + #ifdef DEBUG
     +   int sub_id;
     ++  int dump_imgs;
     + #endif
     + } DVDSubContext;
     + 
     +@@ libavcodec/dvdsubdec.c: 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
     + 
     +@@ libavcodec/dvdsubdec.c: 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 = {
 2:  8da6e4ab17 ! 2:  0cd2c77f31 avcodec/dvbsubdec: fix writing ppm
     @@ Metadata
      Author: softworkz <softworkz@hotmail.com>
      
       ## Commit message ##
     -    avcodec/dvbsubdec: fix writing ppm
     +    avcodec/dvdsubdec,dvbsubdec: fix writing ppm
      
          fopen needs (b)inary mode
      
     @@ libavcodec/dvbsubdec.c: static void png_save(DVBSubContext *ctx, const char *fil
           if (!f) {
               perror(fname2);
               return;
     +
     + ## libavcodec/dvdsubdec.c ##
     +@@ libavcodec/dvdsubdec.c: static void ppm_save(const char *filename, uint8_t *bitmap, int w, int h,
     +     int back[3] = {0, 255, 0};  /* green background */
     +     FILE *f;
     + 
     +-    f = fopen(filename, "w");
     ++    f = fopen(filename, "wb");
     +     if (!f) {
     +         perror(filename);
     +         return;
 3:  9186ff48ec < -:  ---------- avcodec/dvdsubdec: don't dump images to disk based on DEBUG define
 4:  341474e338 < -:  ---------- avcodec/dvdsubdec: fix writing ppm

Comments

Soft Works Feb. 3, 2022, 10:08 p.m. UTC | #1
> -----Original Message-----
> From: ffmpegagent <ffmpegagent@gmail.com>
> Sent: Monday, January 10, 2022 8:56 PM
> To: ffmpeg-devel@ffmpeg.org
> Cc: Hendrik Leppkes <h.leppkes@gmail.com>; Soft Works
> <softworkz@hotmail.com>; Marton Balint <cus@passwd.hu>; softworkz
> <softworkz@hotmail.com>
> Subject: [PATCH v2 0/2] avcodec/dvbsubdec,dvdsubdec: don't dump images
> to disk based on DEBUG define
> 
> It's annoying and unexpected, but still useful at times (as I've
> realized
> just recently).
> 
> This is a follow-up to the earlier submission here:
> https://www.mail-archive.com/ffmpeg-devel@ffmpeg.org/msg128080.html
> 
> There has been a comment from Anton, questioning whether the dump-
> feature is
> useful. Meanwhile I came to the conclusion that it can be useful in-
> fact. It
> just shouldn't happen automatically when DEBUG is defined. That's what
> these
> patches do.
> 
> I also added fixes for the fopen() call.
> 
> softworkz (2):
>   avcodec/dvdsubdec,dvbsubdec: don't dump images to disk based on
> DEBUG
>     define
>   avcodec/dvdsubdec,dvbsubdec: fix writing ppm
> 
>  libavcodec/dvbsubdec.c | 20 +++++++++++++-------
>  libavcodec/dvdsubdec.c | 11 ++++++++---
>  2 files changed, 21 insertions(+), 10 deletions(-)
> 
> 
> base-commit: 6c4074e4234edacfb3f37184fd68771df3cb2b7f
> Published-As: https://github.com/ffstaging/FFmpeg/releases/tag/pr-
> ffstaging-17%2Fsoftworkz%2Fsubmit_dvb_subs-v2
> Fetch-It-Via: git fetch https://github.com/ffstaging/FFmpeg pr-
> ffstaging-17/softworkz/submit_dvb_subs-v2
> Pull-Request: https://github.com/ffstaging/FFmpeg/pull/17
> 
> Range-diff vs v1:
> 
>  1:  6ca8905c3d ! 1:  2f12ac7f1f avcodec/dvbsubdec: don't dump images
> to disk based on DEBUG define
>      @@ Metadata
>       Author: softworkz <softworkz@hotmail.com>
> 
>        ## Commit message ##
>      -    avcodec/dvbsubdec: don't dump images to disk based on DEBUG
> define
>      +    avcodec/dvdsubdec,dvbsubdec: don't dump images to disk based
> on DEBUG define
> 
>           It's been a regular annoyance.
>           Introduce a debug-only parameter for this.
>      @@ libavcodec/dvbsubdec.c: static const AVOption options[] = {
>            {NULL}
>        };
>        static const AVClass dvbsubdec_class = {
>      +
>      + ## libavcodec/dvdsubdec.c ##
>      +@@ libavcodec/dvdsubdec.c: typedef struct DVDSubContext
>      +   uint8_t  used_color[256];
>      + #ifdef DEBUG
>      +   int sub_id;
>      ++  int dump_imgs;
>      + #endif
>      + } DVDSubContext;
>      +
>      +@@ libavcodec/dvdsubdec.c: 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
>      +
>      +@@ libavcodec/dvdsubdec.c: 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 = {
>  2:  8da6e4ab17 ! 2:  0cd2c77f31 avcodec/dvbsubdec: fix writing ppm
>      @@ Metadata
>       Author: softworkz <softworkz@hotmail.com>
> 
>        ## Commit message ##
>      -    avcodec/dvbsubdec: fix writing ppm
>      +    avcodec/dvdsubdec,dvbsubdec: fix writing ppm
> 
>           fopen needs (b)inary mode
> 
>      @@ libavcodec/dvbsubdec.c: static void png_save(DVBSubContext
> *ctx, const char *fil
>            if (!f) {
>                perror(fname2);
>                return;
>      +
>      + ## libavcodec/dvdsubdec.c ##
>      +@@ libavcodec/dvdsubdec.c: static void ppm_save(const char
> *filename, uint8_t *bitmap, int w, int h,
>      +     int back[3] = {0, 255, 0};  /* green background */
>      +     FILE *f;
>      +
>      +-    f = fopen(filename, "w");
>      ++    f = fopen(filename, "wb");
>      +     if (!f) {
>      +         perror(filename);
>      +         return;
>  3:  9186ff48ec < -:  ---------- avcodec/dvdsubdec: don't dump images
> to disk based on DEBUG define
>  4:  341474e338 < -:  ---------- avcodec/dvdsubdec: fix writing ppm
> 
> --
> ffmpeg-codebot

Ping. (this v2 has squashed the commits as requested)