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

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

[ffmpegagent]

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 (4):
  avcodec/dvbsubdec: don't dump images to disk based on DEBUG define
  avcodec/dvbsubdec: fix writing ppm
  avcodec/dvdsubdec: don't dump images to disk based on DEBUG define
  avcodec/dvdsubdec: fix writing ppm

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


base-commit: 242ed971cb005157488b9a21942d9fb4be4d0347
Published-As: https://github.com/ffstaging/FFmpeg/releases/tag/pr-ffstaging-17%2Fsoftworkz%2Fsubmit_dvb_subs-v1
Fetch-It-Via: git fetch https://github.com/ffstaging/FFmpeg pr-ffstaging-17/softworkz/submit_dvb_subs-v1
Pull-Request: https://github.com/ffstaging/FFmpeg/pull/17
-- 
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] [PATCH 1/4] avcodec/dvbsubdec: don't dump images to disk based on DEBUG define

[ffmpegagent]

From: softworkz <softworkz@hotmail.com>

It's been a regular annoyance.
Introduce a debug-only parameter for this.

Signed-off-by: softworkz <softworkz@hotmail.com>
---
 libavcodec/dvbsubdec.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/libavcodec/dvbsubdec.c b/libavcodec/dvbsubdec.c
index bc741a1de6..8db9963fda 100644
--- a/libavcodec/dvbsubdec.c
+++ b/libavcodec/dvbsubdec.c
@@ -137,6 +137,9 @@ typedef struct DVBSubContext {
 
     DVBSubRegionDisplay *display_list;
     DVBSubDisplayDefinition *display_definition;
+#ifdef DEBUG
+  int dump_imgs;
+#endif
 } DVBSubContext;
 
 
@@ -1534,11 +1537,11 @@ static int save_display_set(DVBSubContext *ctx)
 
         }
 
-        snprintf(filename, sizeof(filename), "dvbs.%d", fileno_index);
-
-        png_save(ctx, filename, pbuf, width, height);
-
-        av_freep(&pbuf);
+        if (ctx->dump_imgs) {
+            snprintf(filename, sizeof(filename), "dvbs.%d", fileno_index);
+            png_save(ctx, filename, pbuf, width, height);
+            av_freep(&pbuf);
+        }
     }
 
     fileno_index++;
@@ -1730,6 +1733,9 @@ static const AVOption options[] = {
     {"compute_edt", "compute end of time using pts or timeout", OFFSET(compute_edt), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, DS},
     {"compute_clut", "compute clut when not available(-1) or only once (-2) or always(1) or never(0)", OFFSET(compute_clut), AV_OPT_TYPE_BOOL, {.i64 = -1}, -2, 1, DS},
     {"dvb_substream", "", OFFSET(substream), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 63, DS},
+#ifdef DEBUG
+    { "dump_imgs", "Dump subtitle images to disk", OFFSET(dump_imgs), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, DS},
+#endif
     {NULL}
 };
 static const AVClass dvbsubdec_class = {
-- 
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] [PATCH 2/4] avcodec/dvbsubdec: fix writing ppm

[ffmpegagent]

From: softworkz <softworkz@hotmail.com>

fopen needs (b)inary mode

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

diff --git a/libavcodec/dvbsubdec.c b/libavcodec/dvbsubdec.c
index 8db9963fda..f65bf960a4 100644
--- a/libavcodec/dvbsubdec.c
+++ b/libavcodec/dvbsubdec.c
@@ -1394,7 +1394,7 @@ static void png_save(DVBSubContext *ctx, const char *filename, uint32_t *bitmap,
 
     snprintf(fname, sizeof(fname), "%s.ppm", filename);
 
-    f = fopen(fname, "w");
+    f = fopen(fname, "wb");
     if (!f) {
         perror(fname);
         return;
@@ -1416,7 +1416,7 @@ static void png_save(DVBSubContext *ctx, const char *filename, uint32_t *bitmap,
 
     snprintf(fname2, sizeof(fname2), "%s-a.pgm", filename);
 
-    f = fopen(fname2, "w");
+    f = fopen(fname2, "wb");
     if (!f) {
         perror(fname2);
         return;
-- 
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] [PATCH 3/4] avcodec/dvdsubdec: don't dump images to disk based on DEBUG define

[ffmpegagent]

From: softworkz <softworkz@hotmail.com>

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(-)

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 = {
-- 
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] [PATCH 4/4] avcodec/dvdsubdec: fix writing ppm

[ffmpegagent]

From: softworkz <softworkz@hotmail.com>

fopen needs (b)inary mode

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

diff --git a/libavcodec/dvdsubdec.c b/libavcodec/dvdsubdec.c
index c0f796068e..f3d1a4e2fc 100644
--- a/libavcodec/dvdsubdec.c
+++ b/libavcodec/dvdsubdec.c
@@ -509,7 +509,7 @@ 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;
-- 
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".

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

[Hendrik Leppkes]

On Fri, Jan 7, 2022 at 5:50 AM ffmpegagent <ffmpegagent@gmail.com> wrote:
>
> 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 (4):
>   avcodec/dvbsubdec: don't dump images to disk based on DEBUG define
>   avcodec/dvbsubdec: fix writing ppm
>   avcodec/dvdsubdec: don't dump images to disk based on DEBUG define
>   avcodec/dvdsubdec: fix writing ppm
>
>  libavcodec/dvbsubdec.c | 20 +++++++++++++-------
>  libavcodec/dvdsubdec.c | 11 ++++++++---
>  2 files changed, 21 insertions(+), 10 deletions(-)
>
>

These patches need some squasing. It should've been obvious from them
being duplicated.

- Hendrik
_______________________________________________
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".

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

[Hendrik Leppkes]

On Fri, Jan 7, 2022 at 11:20 AM Hendrik Leppkes <h.leppkes@gmail.com> wrote:
>
> On Fri, Jan 7, 2022 at 5:50 AM ffmpegagent <ffmpegagent@gmail.com> wrote:
> >
> > 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 (4):
> >   avcodec/dvbsubdec: don't dump images to disk based on DEBUG define
> >   avcodec/dvbsubdec: fix writing ppm
> >   avcodec/dvdsubdec: don't dump images to disk based on DEBUG define
> >   avcodec/dvdsubdec: fix writing ppm
> >
> >  libavcodec/dvbsubdec.c | 20 +++++++++++++-------
> >  libavcodec/dvdsubdec.c | 11 ++++++++---
> >  2 files changed, 21 insertions(+), 10 deletions(-)
> >
> >
>
> These patches need some squasing. It should've been obvious from them
> being duplicated.
>

Actually I fell into the dvb dvd trap, but regardless the patches are
basically identical, so I would still squash them, personally.

- Hendrik
_______________________________________________
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".

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

[Soft Works]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Hendrik
> Leppkes
> Sent: Friday, January 7, 2022 11:32 AM
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH 0/4] avcodec/dvbsubdec, dvdsubdec: don't
> dump images to disk based on DEBUG define
> 
> On Fri, Jan 7, 2022 at 11:20 AM Hendrik Leppkes <h.leppkes@gmail.com> wrote:
> >
> > On Fri, Jan 7, 2022 at 5:50 AM ffmpegagent <ffmpegagent@gmail.com> wrote:
> > >
> > > 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 (4):
> > >   avcodec/dvbsubdec: don't dump images to disk based on DEBUG define
> > >   avcodec/dvbsubdec: fix writing ppm
> > >   avcodec/dvdsubdec: don't dump images to disk based on DEBUG define
> > >   avcodec/dvdsubdec: fix writing ppm
> > >
> > >  libavcodec/dvbsubdec.c | 20 +++++++++++++-------
> > >  libavcodec/dvdsubdec.c | 11 ++++++++---
> > >  2 files changed, 21 insertions(+), 10 deletions(-)
> > >
> > >
> >
> > These patches need some squasing. It should've been obvious from them
> > being duplicated.
> >
> 
> Actually I fell into the dvb dvd trap, but regardless the patches are
> basically identical, so I would still squash them, personally.

No problem, but in which way?

1. All 4 commits into a single one
2. 4 >> 2 horizontally (a single one for the debug commits and a single 
   one for the fopen commits)
3. 4 >> 2 vertically (a single commit for dvb and a single one for dvd)

Thanks,
softworkz

_______________________________________________
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".

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

[Hendrik Leppkes]

On Fri, Jan 7, 2022 at 5:14 PM Soft Works <softworkz@hotmail.com> wrote:
>
>
>
> > -----Original Message-----
> > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Hendrik
> > Leppkes
> > Sent: Friday, January 7, 2022 11:32 AM
> > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> > Subject: Re: [FFmpeg-devel] [PATCH 0/4] avcodec/dvbsubdec, dvdsubdec: don't
> > dump images to disk based on DEBUG define
> >
> > On Fri, Jan 7, 2022 at 11:20 AM Hendrik Leppkes <h.leppkes@gmail.com> wrote:
> > >
> > > On Fri, Jan 7, 2022 at 5:50 AM ffmpegagent <ffmpegagent@gmail.com> wrote:
> > > >
> > > > 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 (4):
> > > >   avcodec/dvbsubdec: don't dump images to disk based on DEBUG define
> > > >   avcodec/dvbsubdec: fix writing ppm
> > > >   avcodec/dvdsubdec: don't dump images to disk based on DEBUG define
> > > >   avcodec/dvdsubdec: fix writing ppm
> > > >
> > > >  libavcodec/dvbsubdec.c | 20 +++++++++++++-------
> > > >  libavcodec/dvdsubdec.c | 11 ++++++++---
> > > >  2 files changed, 21 insertions(+), 10 deletions(-)
> > > >
> > > >
> > >
> > > These patches need some squasing. It should've been obvious from them
> > > being duplicated.
> > >
> >
> > Actually I fell into the dvb dvd trap, but regardless the patches are
> > basically identical, so I would still squash them, personally.
>
> No problem, but in which way?
>
> 1. All 4 commits into a single one
> 2. 4 >> 2 horizontally (a single one for the debug commits and a single
>    one for the fopen commits)
> 3. 4 >> 2 vertically (a single commit for dvb and a single one for dvd)
>

I figured combining the patches that do the exact same thing for the
two different decoders.

- Hendrik
_______________________________________________
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".

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

[Soft Works]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Hendrik
> Leppkes
> Sent: Monday, January 10, 2022 11:31 AM
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH 0/4] avcodec/dvbsubdec, dvdsubdec: don't
> dump images to disk based on DEBUG define
> 
> On Fri, Jan 7, 2022 at 5:14 PM Soft Works <softworkz@hotmail.com> wrote:
> >
> >
> >
> > > -----Original Message-----
> > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Hendrik
> > > Leppkes
> > > Sent: Friday, January 7, 2022 11:32 AM
> > > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> > > Subject: Re: [FFmpeg-devel] [PATCH 0/4] avcodec/dvbsubdec, dvdsubdec:
> don't
> > > dump images to disk based on DEBUG define
> > >
> > > On Fri, Jan 7, 2022 at 11:20 AM Hendrik Leppkes <h.leppkes@gmail.com>
> wrote:
> > > >
> > > > On Fri, Jan 7, 2022 at 5:50 AM ffmpegagent <ffmpegagent@gmail.com>
> wrote:
> > > > >
> > > > > 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 (4):
> > > > >   avcodec/dvbsubdec: don't dump images to disk based on DEBUG define
> > > > >   avcodec/dvbsubdec: fix writing ppm
> > > > >   avcodec/dvdsubdec: don't dump images to disk based on DEBUG define
> > > > >   avcodec/dvdsubdec: fix writing ppm
> > > > >
> > > > >  libavcodec/dvbsubdec.c | 20 +++++++++++++-------
> > > > >  libavcodec/dvdsubdec.c | 11 ++++++++---
> > > > >  2 files changed, 21 insertions(+), 10 deletions(-)
> > > > >
> > > > >
> > > >
> > > > These patches need some squasing. It should've been obvious from them
> > > > being duplicated.
> > > >
> > >
> > > Actually I fell into the dvb dvd trap, but regardless the patches are
> > > basically identical, so I would still squash them, personally.
> >
> > No problem, but in which way?
> >
> > 1. All 4 commits into a single one
> > 2. 4 >> 2 horizontally (a single one for the debug commits and a single
> >    one for the fopen commits)
> > 3. 4 >> 2 vertically (a single commit for dvb and a single one for dvd)
> >
> 
> I figured combining the patches that do the exact same thing for the
> two different decoders.

OK, will do.

Thank you,
softworkz
_______________________________________________
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".

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

[Marton Balint]

On Fri, 7 Jan 2022, ffmpegagent wrote:

> 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.

Well, I kind of agree with Anton, this a debug feature and it should not 
be added as an option, but simply should be removed from the codebase.

Regards,
Marton
_______________________________________________
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".

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

[Soft Works]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Marton
> Balint
> Sent: Friday, January 7, 2022 8:57 PM
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH 0/4] avcodec/dvbsubdec, dvdsubdec: don't
> dump images to disk based on DEBUG define
> 
> 
> 
> On Fri, 7 Jan 2022, ffmpegagent wrote:
> 
> > 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.
> 
> Well, I kind of agree with Anton, this a debug feature and it should not
> be added as an option, but simply should be removed from the codebase.

It's not added as a regular option. It's an option that is only available 
when you compile with DEBUG defined.

Isn't this an acceptable compromise?

softworkz
_______________________________________________
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".

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

[Marton Balint]

On Fri, 7 Jan 2022, Soft Works wrote:

>
>
>> -----Original Message-----
>> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Marton
>> Balint
>> Sent: Friday, January 7, 2022 8:57 PM
>> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
>> Subject: Re: [FFmpeg-devel] [PATCH 0/4] avcodec/dvbsubdec, dvdsubdec: don't
>> dump images to disk based on DEBUG define
>>
>>
>>
>> On Fri, 7 Jan 2022, ffmpegagent wrote:
>>
>>> 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.
>>
>> Well, I kind of agree with Anton, this a debug feature and it should not
>> be added as an option, but simply should be removed from the codebase.
>
> It's not added as a regular option. It's an option that is only available
> when you compile with DEBUG defined.

Ah, OK.

>
> Isn't this an acceptable compromise?

Well, I am not a fan of leaving DEBUG chunks in the codebase to be 
honest. But if somebody applies it, then fine with me.

Thanks,
Marton
_______________________________________________
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".

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

[Soft Works]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Marton
> Balint
> Sent: Friday, January 7, 2022 10:53 PM
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH 0/4] avcodec/dvbsubdec, dvdsubdec: don't
> dump images to disk based on DEBUG define
> 
> 
> 
> On Fri, 7 Jan 2022, Soft Works wrote:
> 
> >
> >
> >> -----Original Message-----
> >> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Marton
> >> Balint
> >> Sent: Friday, January 7, 2022 8:57 PM
> >> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> >> Subject: Re: [FFmpeg-devel] [PATCH 0/4] avcodec/dvbsubdec, dvdsubdec:
> don't
> >> dump images to disk based on DEBUG define
> >>
> >>
> >>
> >> On Fri, 7 Jan 2022, ffmpegagent wrote:
> >>
> >>> 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.
> >>
> >> Well, I kind of agree with Anton, this a debug feature and it should not
> >> be added as an option, but simply should be removed from the codebase.
> >
> > It's not added as a regular option. It's an option that is only available
> > when you compile with DEBUG defined.
> 
> Ah, OK.
> 
> >
> > Isn't this an acceptable compromise?
> 
> Well, I am not a fan of leaving DEBUG chunks in the codebase to be
> honest. But if somebody applies it, then fine with me.

The code exists in the code base for 6 years..
I was about to agree to the removal, but just few days ago I was glad
that I could use it, and it might be useful in the near future in the context 
of subtitle filtering for troubleshooting.

Also there's a bugfix patch where it could be useful:

avcodec/dvdsubdec: fix incorrect yellow appearance of dvd subtitles
https://github.com/ffstaging/FFmpeg/pull/16
https://master.gitmailbox.com/ffmpegdev/pull.16.ffstaging.FFmpeg.1641262759164.ffmpegagent@gmail.com

Eventually, I'll submit a 'dumpgraphicsubs' filter for this purpose,
and then that code can be removed anyway, but for the time being,
I think this PR still improves the situation (no dump files written
to disk unexpectedly), sufficiently enough to merge it even without
a full removal.

Thanks again,
softworkz


_______________________________________________
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] [PATCH v2 0/2] avcodec/dvbsubdec, dvdsubdec: don't dump images to disk based on DEBUG define

[ffmpegagent]

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
_______________________________________________
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] [PATCH v2 1/2] avcodec/dvdsubdec, dvbsubdec: don't dump images to disk based on DEBUG define

[ffmpegagent]

From: softworkz <softworkz@hotmail.com>

It's been a regular annoyance.
Introduce a debug-only parameter for this.

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

diff --git a/libavcodec/dvbsubdec.c b/libavcodec/dvbsubdec.c
index bc741a1de6..8db9963fda 100644
--- a/libavcodec/dvbsubdec.c
+++ b/libavcodec/dvbsubdec.c
@@ -137,6 +137,9 @@ typedef struct DVBSubContext {
 
     DVBSubRegionDisplay *display_list;
     DVBSubDisplayDefinition *display_definition;
+#ifdef DEBUG
+  int dump_imgs;
+#endif
 } DVBSubContext;
 
 
@@ -1534,11 +1537,11 @@ static int save_display_set(DVBSubContext *ctx)
 
         }
 
-        snprintf(filename, sizeof(filename), "dvbs.%d", fileno_index);
-
-        png_save(ctx, filename, pbuf, width, height);
-
-        av_freep(&pbuf);
+        if (ctx->dump_imgs) {
+            snprintf(filename, sizeof(filename), "dvbs.%d", fileno_index);
+            png_save(ctx, filename, pbuf, width, height);
+            av_freep(&pbuf);
+        }
     }
 
     fileno_index++;
@@ -1730,6 +1733,9 @@ static const AVOption options[] = {
     {"compute_edt", "compute end of time using pts or timeout", OFFSET(compute_edt), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, DS},
     {"compute_clut", "compute clut when not available(-1) or only once (-2) or always(1) or never(0)", OFFSET(compute_clut), AV_OPT_TYPE_BOOL, {.i64 = -1}, -2, 1, DS},
     {"dvb_substream", "", OFFSET(substream), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 63, DS},
+#ifdef DEBUG
+    { "dump_imgs", "Dump subtitle images to disk", OFFSET(dump_imgs), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, DS},
+#endif
     {NULL}
 };
 static const AVClass dvbsubdec_class = {
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 = {
-- 
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] [PATCH v2 2/2] avcodec/dvdsubdec, dvbsubdec: fix writing ppm

[ffmpegagent]

From: softworkz <softworkz@hotmail.com>

fopen needs (b)inary mode

Signed-off-by: softworkz <softworkz@hotmail.com>
---
 libavcodec/dvbsubdec.c | 4 ++--
 libavcodec/dvdsubdec.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/libavcodec/dvbsubdec.c b/libavcodec/dvbsubdec.c
index 8db9963fda..f65bf960a4 100644
--- a/libavcodec/dvbsubdec.c
+++ b/libavcodec/dvbsubdec.c
@@ -1394,7 +1394,7 @@ static void png_save(DVBSubContext *ctx, const char *filename, uint32_t *bitmap,
 
     snprintf(fname, sizeof(fname), "%s.ppm", filename);
 
-    f = fopen(fname, "w");
+    f = fopen(fname, "wb");
     if (!f) {
         perror(fname);
         return;
@@ -1416,7 +1416,7 @@ static void png_save(DVBSubContext *ctx, const char *filename, uint32_t *bitmap,
 
     snprintf(fname2, sizeof(fname2), "%s-a.pgm", filename);
 
-    f = fopen(fname2, "w");
+    f = fopen(fname2, "wb");
     if (!f) {
         perror(fname2);
         return;
diff --git a/libavcodec/dvdsubdec.c b/libavcodec/dvdsubdec.c
index c0f796068e..f3d1a4e2fc 100644
--- a/libavcodec/dvdsubdec.c
+++ b/libavcodec/dvdsubdec.c
@@ -509,7 +509,7 @@ 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;
-- 
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".

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

[Soft Works]

> -----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)


_______________________________________________
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] [PATCH v3] avcodec/dvdsubdec, dvbsubdec: remove bitmap dumping in DEBUG builds

[softworkz]

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.

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".

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

[Marton Balint]

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".