Re: [FFmpeg-devel] [PATCH v3] avcodec/dvdsubdec, dvbsubdec: remove bitmap dumping in DEBUG builds

[Marton Balint <cus@passwd.hu>]

On Sat, 28 May 2022, softworkz wrote:

> From: softworkz <softworkz@hotmail.com>
>
> It's been a regular annoyance and often undesired.
> There will be a subtitle filter which allows to dump individual
> subtitle bitmaps.
>
> Signed-off-by: softworkz <softworkz@hotmail.com>
> ---
>    avcodec/dvdsubdec,dvbsubdec: remove bitmap dumping in DEBUG builds
>
>    Original Message:
>
>    > 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.
>
>    v2: My previous submission wasn't liked that much and there seemed to be
>    a consensus to drop this altogether. So - here's the commit for dropping
>    it.

Will apply.

Thanks,
Marton

>
> Published-As: https://github.com/ffstaging/FFmpeg/releases/tag/pr-ffstaging-17%2Fsoftworkz%2Fsubmit_dvb_subs-v3
> Fetch-It-Via: git fetch https://github.com/ffstaging/FFmpeg pr-ffstaging-17/softworkz/submit_dvb_subs-v3
> Pull-Request: https://github.com/ffstaging/FFmpeg/pull/17
>
> Range-diff vs v2:
>
> 1:  2f12ac7f1f < -:  ---------- avcodec/dvdsubdec,dvbsubdec: don't dump images to disk based on DEBUG define
> 2:  0cd2c77f31 < -:  ---------- avcodec/dvdsubdec,dvbsubdec: fix writing ppm
> -:  ---------- > 1:  e12db0e6d7 avcodec/dvdsubdec,dvbsubdec: remove bitmap dumping in DEBUG builds
>
>
> libavcodec/dvbsubdec.c | 169 -----------------------------------------
> libavcodec/dvdsubdec.c |  48 ------------
> 2 files changed, 217 deletions(-)
>
> diff --git a/libavcodec/dvbsubdec.c b/libavcodec/dvbsubdec.c
> index 6e510d12c7..4d4007ffd9 100644
> --- a/libavcodec/dvbsubdec.c
> +++ b/libavcodec/dvbsubdec.c
> @@ -1381,172 +1381,6 @@ static int dvbsub_parse_page_segment(AVCodecContext *avctx,
>     return 0;
> }
>
> -
> -#ifdef DEBUG
> -static void png_save(DVBSubContext *ctx, const char *filename, uint32_t *bitmap, int w, int h)
> -{
> -    int x, y, v;
> -    FILE *f;
> -    char fname[40], fname2[40];
> -    char command[1024];
> -
> -    snprintf(fname, sizeof(fname), "%s.ppm", filename);
> -
> -    f = fopen(fname, "w");
> -    if (!f) {
> -        perror(fname);
> -        return;
> -    }
> -    fprintf(f, "P6\n"
> -            "%d %d\n"
> -            "%d\n",
> -            w, h, 255);
> -    for(y = 0; y < h; y++) {
> -        for(x = 0; x < w; x++) {
> -            v = bitmap[y * w + x];
> -            putc((v >> 16) & 0xff, f);
> -            putc((v >> 8) & 0xff, f);
> -            putc((v >> 0) & 0xff, f);
> -        }
> -    }
> -    fclose(f);
> -
> -
> -    snprintf(fname2, sizeof(fname2), "%s-a.pgm", filename);
> -
> -    f = fopen(fname2, "w");
> -    if (!f) {
> -        perror(fname2);
> -        return;
> -    }
> -    fprintf(f, "P5\n"
> -            "%d %d\n"
> -            "%d\n",
> -            w, h, 255);
> -    for(y = 0; y < h; y++) {
> -        for(x = 0; x < w; x++) {
> -            v = bitmap[y * w + x];
> -            putc((v >> 24) & 0xff, f);
> -        }
> -    }
> -    fclose(f);
> -
> -    snprintf(command, sizeof(command), "pnmtopng -alpha %s %s > %s.png 2> /dev/null", fname2, fname, filename);
> -    if (system(command) != 0) {
> -        av_log(ctx, AV_LOG_ERROR, "Error running pnmtopng\n");
> -        return;
> -    }
> -
> -    snprintf(command, sizeof(command), "rm %s %s", fname, fname2);
> -    if (system(command) != 0) {
> -        av_log(ctx, AV_LOG_ERROR, "Error removing %s and %s\n", fname, fname2);
> -        return;
> -    }
> -}
> -
> -static int save_display_set(DVBSubContext *ctx)
> -{
> -    DVBSubRegion *region;
> -    DVBSubRegionDisplay *display;
> -    const DVBSubCLUT *clut;
> -    const uint32_t *clut_table;
> -    int x_pos, y_pos, width, height;
> -    int x, y, y_off, x_off;
> -    uint32_t *pbuf;
> -    char filename[32];
> -    static int fileno_index = 0;
> -
> -    x_pos = -1;
> -    y_pos = -1;
> -    width = 0;
> -    height = 0;
> -
> -    for (display = ctx->display_list; display; display = display->next) {
> -        region = get_region(ctx, display->region_id);
> -
> -        if (!region)
> -            return -1;
> -
> -        if (x_pos == -1) {
> -            x_pos = display->x_pos;
> -            y_pos = display->y_pos;
> -            width = region->width;
> -            height = region->height;
> -        } else {
> -            if (display->x_pos < x_pos) {
> -                width += (x_pos - display->x_pos);
> -                x_pos = display->x_pos;
> -            }
> -
> -            if (display->y_pos < y_pos) {
> -                height += (y_pos - display->y_pos);
> -                y_pos = display->y_pos;
> -            }
> -
> -            if (display->x_pos + region->width > x_pos + width) {
> -                width = display->x_pos + region->width - x_pos;
> -            }
> -
> -            if (display->y_pos + region->height > y_pos + height) {
> -                height = display->y_pos + region->height - y_pos;
> -            }
> -        }
> -    }
> -
> -    if (x_pos >= 0) {
> -
> -        pbuf = av_malloc(width * height * 4);
> -        if (!pbuf)
> -            return -1;
> -
> -        for (display = ctx->display_list; display; display = display->next) {
> -            region = get_region(ctx, display->region_id);
> -
> -            if (!region)
> -                return -1;
> -
> -            x_off = display->x_pos - x_pos;
> -            y_off = display->y_pos - y_pos;
> -
> -            clut = get_clut(ctx, region->clut);
> -
> -            if (!clut)
> -                clut = &default_clut;
> -
> -            switch (region->depth) {
> -            case 2:
> -                clut_table = clut->clut4;
> -                break;
> -            case 8:
> -                clut_table = clut->clut256;
> -                break;
> -            case 4:
> -            default:
> -                clut_table = clut->clut16;
> -                break;
> -            }
> -
> -            for (y = 0; y < region->height; y++) {
> -                for (x = 0; x < region->width; x++) {
> -                    pbuf[((y + y_off) * width) + x_off + x] =
> -                        clut_table[region->pbuf[y * region->width + x]];
> -                }
> -            }
> -
> -        }
> -
> -        snprintf(filename, sizeof(filename), "dvbs.%d", fileno_index);
> -
> -        png_save(ctx, filename, pbuf, width, height);
> -
> -        av_freep(&pbuf);
> -    }
> -
> -    fileno_index++;
> -    return 0;
> -}
> -#endif /* DEBUG */
> -
> static int dvbsub_parse_display_definition_segment(AVCodecContext *avctx,
>                                                    const uint8_t *buf,
>                                                    int buf_size)
> @@ -1601,9 +1435,6 @@ static int dvbsub_display_end_segment(AVCodecContext *avctx, const uint8_t *buf,
>
>     if (ctx->compute_edt == 0)
>         save_subtitle_set(avctx, sub, got_output);
> -#ifdef DEBUG
> -    save_display_set(ctx);
> -#endif
>     return 0;
> }
>
> diff --git a/libavcodec/dvdsubdec.c b/libavcodec/dvdsubdec.c
> index b54073393e..97f366cc74 100644
> --- a/libavcodec/dvdsubdec.c
> +++ b/libavcodec/dvdsubdec.c
> @@ -43,9 +43,6 @@ typedef struct DVDSubContext
>   int      buf_size;
>   int      forced_subs_only;
>   uint8_t  used_color[256];
> -#ifdef DEBUG
> -  int sub_id;
> -#endif
> } DVDSubContext;
>
> static void yuv_a_to_rgba(const uint8_t *ycbcr, const uint8_t *alpha, uint32_t *rgba, int num_values)
> @@ -499,38 +496,6 @@ static int find_smallest_bounding_rectangle(DVDSubContext *ctx, AVSubtitle *s)
>     return 1;
> }
>
> -#ifdef DEBUG
> -#define ALPHA_MIX(A,BACK,FORE) (((255-(A)) * (BACK) + (A) * (FORE)) / 255)
> -static void ppm_save(const char *filename, uint8_t *bitmap, int w, int h,
> -                     uint32_t *rgba_palette)
> -{
> -    int x, y, alpha;
> -    uint32_t v;
> -    int back[3] = {0, 255, 0};  /* green background */
> -    FILE *f;
> -
> -    f = fopen(filename, "w");
> -    if (!f) {
> -        perror(filename);
> -        return;
> -    }
> -    fprintf(f, "P6\n"
> -            "%d %d\n"
> -            "%d\n",
> -            w, h, 255);
> -    for(y = 0; y < h; y++) {
> -        for(x = 0; x < w; x++) {
> -            v = rgba_palette[bitmap[y * w + x]];
> -            alpha = v >> 24;
> -            putc(ALPHA_MIX(alpha, back[0], (v >> 16) & 0xff), f);
> -            putc(ALPHA_MIX(alpha, back[1], (v >> 8) & 0xff), f);
> -            putc(ALPHA_MIX(alpha, back[2], (v >> 0) & 0xff), f);
> -        }
> -    }
> -    fclose(f);
> -}
> -#endif
> -
> static int append_to_cached_buf(AVCodecContext *avctx,
>                                 const uint8_t *buf, int buf_size)
> {
> @@ -588,19 +553,6 @@ static int dvdsub_decode(AVCodecContext *avctx, AVSubtitle *sub,
>     if (ctx->forced_subs_only && !(sub->rects[0]->flags & AV_SUBTITLE_FLAG_FORCED))
>         goto no_subtitle;
>
> -#if defined(DEBUG)
> -    {
> -    char ppm_name[32];
> -
> -    snprintf(ppm_name, sizeof(ppm_name), "/tmp/%05d.ppm", ctx->sub_id++);
> -    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]);
> -    }
> -#endif
> -
>     ctx->buf_size = 0;
>     *data_size = 1;
>     return buf_size;
>
> base-commit: 9fba0b8a8c754a012fc74c90ffb7c26a56be8ca0
> -- 
> ffmpeg-codebot
> _______________________________________________
> 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".