Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
* [FFmpeg-devel] [PATCH] dvbsubenc: add a disable_2bpp option to work around some decoders.
@ 2025-03-01 11:43 Waider
  2025-03-01 11:48 ` Ronan Waide
  2025-03-01 12:55 ` Soft Works
  0 siblings, 2 replies; 15+ messages in thread
From: Waider @ 2025-03-01 11:43 UTC (permalink / raw)
  To: ffmpeg-devel

As noted in the code in several places, some DVB subtitle decoders
don't handle 2bpp color. This patch adds a disable_2bpp option which
disables the 2bpp format; subtitles which would use 2bpp will be bumped
up to 4bpp.

Signed-off-by: Ronan Waide <waider@waider.ie>
---
doc/encoders.texi      | 24 ++++++++++++
libavcodec/dvbsubenc.c | 85 +++++++++++++++++++++++++++++++++---------
2 files changed, 92 insertions(+), 17 deletions(-)

diff --git a/doc/encoders.texi b/doc/encoders.texi
index f3fcc1aa60..d6dc92a047 100644
--- a/doc/encoders.texi
+++ b/doc/encoders.texi
@@ -4484,6 +4484,30 @@ Reduces detail but attempts to preserve color at extremely low bitrates.
@chapter Subtitles Encoders
@c man begin SUBTITLES ENCODERS

+@section dvbsub
+
+This codec encodes the bitmap subtitle format that is used in DVB
+broadcasts and recordings. The bitmaps are typically embedded in a
+container such as MPEG-TS as a separate stream.
+
+@subsection Options
+
+@table @option
+@item disable_2bpp @var{boolean}
+Disable the 2 bits-per-pixel encoding format.
+
+DVB supports 2, 4, and 8 bits-per-pixel color lookup tables. However,
+not all players support or properly support 2 bits-per-pixel,
+resulting in unusable subtitles.
+@table @option
+@item 0
+The 2 bits-per-pixel encoding format will be used for subtitles with 4
+colors or less. (default)
+@item 1
+The 2 bits-per-pixel encoding format will be disabled, and subtitles
+with 4 colors or less will use a 4 bits-per-pixel format.
+@end table
+
@section dvdsub

This codec encodes the bitmap subtitle format that is used in DVDs.
diff --git a/libavcodec/dvbsubenc.c b/libavcodec/dvbsubenc.c
index 822e3a5309..727a75e901 100644
--- a/libavcodec/dvbsubenc.c
+++ b/libavcodec/dvbsubenc.c
@@ -22,9 +22,13 @@
#include "bytestream.h"
#include "codec_internal.h"
#include "libavutil/colorspace.h"
+#include "libavutil/opt.h"

typedef struct DVBSubtitleContext {
+    AVClass * class;
    int object_version;
+    int disable_2bpp;
+    int min_colors;
} DVBSubtitleContext;

#define PUTBITS2(val)\
@@ -38,6 +42,18 @@ typedef struct DVBSubtitleContext {
    }\
}

+static int dvbsub_init(AVCodecContext *avctx)
+{
+    DVBSubtitleContext *s = avctx->priv_data;
+    if (s->disable_2bpp) {
+        av_log(avctx, AV_LOG_DEBUG, "Disabling 2bpp subtitles.\n");
+        s->min_colors = 16;
+    } else {
+        s->min_colors = 0;
+    }
+    return 0;
+}
+
static int dvb_encode_rle2(uint8_t **pq, int buf_size,
                           const uint8_t *bitmap, int linesize,
                           int w, int h)
@@ -326,18 +342,22 @@ static int dvbsub_encode(AVCodecContext *avctx, uint8_t *outbuf, int buf_size,

    if (h->num_rects) {
        for (clut_id = 0; clut_id < h->num_rects; clut_id++) {
-            if (buf_size < 6 + h->rects[clut_id]->nb_colors * 6)
+            int nb_colors = s->min_colors > h->rects[clut_id]->nb_colors
+                            ? s->min_colors
+                            : h->rects[clut_id]->nb_colors;
+
+            if (buf_size < 6 + nb_colors * 6)
                return AVERROR_BUFFER_TOO_SMALL;

            /* CLUT segment */

-            if (h->rects[clut_id]->nb_colors <= 4) {
+            if (nb_colors <= 4) {
                /* 2 bpp, some decoders do not support it correctly */
                bpp_index = 0;
-            } else if (h->rects[clut_id]->nb_colors <= 16) {
+            } else if (nb_colors <= 16) {
                /* 4 bpp, standard encoding */
                bpp_index = 1;
-            } else if (h->rects[clut_id]->nb_colors <= 256) {
+            } else if (nb_colors <= 256) {
                /* 8 bpp, standard encoding */
                bpp_index = 2;
            } else {
@@ -354,16 +374,24 @@ static int dvbsub_encode(AVCodecContext *avctx, uint8_t *outbuf, int buf_size,
            *q++ = clut_id;
            *q++ = (0 << 4) | 0xf; /* version = 0 */

-            for(i = 0; i < h->rects[clut_id]->nb_colors; i++) {
+            for(i = 0; i < nb_colors; i++) {
                *q++ = i; /* clut_entry_id */
                *q++ = (1 << (7 - bpp_index)) | (0xf << 1) | 1; /* 2 bits/pixel full range */
                {
                    int a, r, g, b;
-                    uint32_t x= ((uint32_t*)h->rects[clut_id]->data[1])[i];
-                    a = (x >> 24) & 0xff;
-                    r = (x >> 16) & 0xff;
-                    g = (x >>  8) & 0xff;
-                    b = (x >>  0) & 0xff;
+                    if (i < h->rects[clut_id]->nb_colors) {
+                        uint32_t x= ((uint32_t*)h->rects[clut_id]->data[1])[i];
+                        a = (x >> 24) & 0xff;
+                        r = (x >> 16) & 0xff;
+                        g = (x >>  8) & 0xff;
+                        b = (x >>  0) & 0xff;
+                    } else {
+                        /* pad out the CLUT */
+                        a = 0;
+                        r = 0;
+                        g = 0;
+                        b = 0;
+                    }

                    *q++ = RGB_TO_Y_CCIR(r, g, b);
                    *q++ = RGB_TO_V_CCIR(r, g, b, 0);
@@ -373,7 +401,7 @@ static int dvbsub_encode(AVCodecContext *avctx, uint8_t *outbuf, int buf_size,
            }

            bytestream_put_be16(&pseg_len, q - pseg_len - 2);
-            buf_size -= 6 + h->rects[clut_id]->nb_colors * 6;
+            buf_size -= 6 + nb_colors * 6;
        }

        if (buf_size < h->num_rects * 22)
@@ -381,14 +409,17 @@ static int dvbsub_encode(AVCodecContext *avctx, uint8_t *outbuf, int buf_size,
        for (region_id = 0; region_id < h->num_rects; region_id++) {

            /* region composition segment */
+            int nb_colors = s->min_colors > h->rects[region_id]->nb_colors
+                            ? s->min_colors
+                            : h->rects[region_id]->nb_colors;

-            if (h->rects[region_id]->nb_colors <= 4) {
+            if (nb_colors <= 4) {
                /* 2 bpp, some decoders do not support it correctly */
                bpp_index = 0;
-            } else if (h->rects[region_id]->nb_colors <= 16) {
+            } else if (nb_colors <= 16) {
                /* 4 bpp, standard encoding */
                bpp_index = 1;
-            } else if (h->rects[region_id]->nb_colors <= 256) {
+            } else if (nb_colors <= 256) {
                /* 8 bpp, standard encoding */
                bpp_index = 2;
            } else {
@@ -424,17 +455,21 @@ static int dvbsub_encode(AVCodecContext *avctx, uint8_t *outbuf, int buf_size,
                                  const uint8_t *bitmap, int linesize,
                                  int w, int h);

+            int nb_colors = s->min_colors > h->rects[object_id]->nb_colors
+                                ? s->min_colors
+                                : h->rects[object_id]->nb_colors;
+
            if (buf_size < 13)
                return AVERROR_BUFFER_TOO_SMALL;

            /* bpp_index maths */
-            if (h->rects[object_id]->nb_colors <= 4) {
+            if (nb_colors <= 4) {
                /* 2 bpp, some decoders do not support it correctly */
                dvb_encode_rle = dvb_encode_rle2;
-            } else if (h->rects[object_id]->nb_colors <= 16) {
+            } else if (nb_colors <= 16) {
                /* 4 bpp, standard encoding */
                dvb_encode_rle = dvb_encode_rle4;
-            } else if (h->rects[object_id]->nb_colors <= 256) {
+            } else if (nb_colors <= 256) {
                /* 8 bpp, standard encoding */
                dvb_encode_rle = dvb_encode_rle8;
            } else {
@@ -506,6 +541,20 @@ static int dvbsub_encode(AVCodecContext *avctx, uint8_t *outbuf, int buf_size,
    return q - outbuf;
}

+#define OFFSET(x) offsetof(DVBSubtitleContext, x)
+#define SE AV_OPT_FLAG_SUBTITLE_PARAM | AV_OPT_FLAG_ENCODING_PARAM
+static const AVOption options[] = {
+    {"disable_2bpp", "disable the 2bpp subtitle encoder", OFFSET(disable_2bpp), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, SE},
+    { NULL },
+};
+
+static const AVClass dvbsubenc_class = {
+    .class_name = "DVBSUB subtitle encoder",
+    .item_name  = av_default_item_name,
+    .option     = options,
+    .version    = LIBAVUTIL_VERSION_INT,
+};
+
const FFCodec ff_dvbsub_encoder = {
    .p.name         = "dvbsub",
    CODEC_LONG_NAME("DVB subtitles"),
@@ -513,4 +562,6 @@ const FFCodec ff_dvbsub_encoder = {
    .p.id           = AV_CODEC_ID_DVB_SUBTITLE,
    .priv_data_size = sizeof(DVBSubtitleContext),
    FF_CODEC_ENCODE_SUB_CB(dvbsub_encode),
+    .init           = dvbsub_init,
+    .p.priv_class   = &dvbsubenc_class,
};
-- 
2.39.5 (Apple Git-154)

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [FFmpeg-devel] [PATCH] dvbsubenc: add a disable_2bpp option to work around some decoders.
  2025-03-01 11:43 [FFmpeg-devel] [PATCH] dvbsubenc: add a disable_2bpp option to work around some decoders Waider
@ 2025-03-01 11:48 ` Ronan Waide
  2025-03-01 12:44   ` Ronan Waide
  2025-03-01 12:55 ` Soft Works
  1 sibling, 1 reply; 15+ messages in thread
From: Ronan Waide @ 2025-03-01 11:48 UTC (permalink / raw)
  To: FFmpeg development discussions and patches



> On 1 Mar 2025, at 11:43, Waider <waider@waider.ie> wrote:
> 
> As noted in the code in several places, some DVB subtitle decoders
> don't handle 2bpp color. This patch adds a disable_2bpp option which
> disables the 2bpp format; subtitles which would use 2bpp will be bumped
> up to 4bpp.


I'm a first-time patcher so apologies in advance if I've missed anything here. The patch is as described: I have a TV that will not correctly render 2bpp DVB subtitles, but is happy enough with 4bpp+, and this patch has allowed me to transcode DVDs into MPEG-TS with subtitles which work on the TV. I've kept the change as small as I can without sacrificing clarity - if I'd carried the `disable_2bpp` option through as-is then I'd have to have each use of it first check if it's set and then tweak the size of the colour table, and the layout of the current code is such that it relies more on the number of colours than the bits-per-pixel so that would have been a more intrusive change.

Cheers,
Waider.
-- 
Ronan Waide
waider@waider.ie




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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [FFmpeg-devel] [PATCH] dvbsubenc: add a disable_2bpp option to work around some decoders.
  2025-03-01 11:48 ` Ronan Waide
@ 2025-03-01 12:44   ` Ronan Waide
  0 siblings, 0 replies; 15+ messages in thread
From: Ronan Waide @ 2025-03-01 12:44 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

As noted in the code in several places, some DVB subtitle decoders
don't handle 2bpp color. This patch adds a disable_2bpp option which
disables the 2bpp format; subtitles which would use 2bpp will be bumped
up to 4bpp.

Signed-off-by: Ronan Waide <waider@waider.ie>
---
doc/encoders.texi      | 27 ++++++++++++++
libavcodec/dvbsubenc.c | 85 +++++++++++++++++++++++++++++++++---------
2 files changed, 95 insertions(+), 17 deletions(-)

diff --git a/doc/encoders.texi b/doc/encoders.texi
index f3fcc1aa60..07984b6b54 100644
--- a/doc/encoders.texi
+++ b/doc/encoders.texi
@@ -4484,6 +4484,33 @@ Reduces detail but attempts to preserve color at extremely low bitrates.
@chapter Subtitles Encoders
@c man begin SUBTITLES ENCODERS

+@section dvbsub
+
+This codec encodes the bitmap subtitle format that is used in DVB
+broadcasts and recordings. The bitmaps are typically embedded in a
+container such as MPEG-TS as a separate stream.
+
+@subsection Options
+
+@table @option
+@item disable_2bpp @var{boolean}
+Disable the 2 bits-per-pixel encoding format.
+
+DVB supports 2, 4, and 8 bits-per-pixel color lookup tables. However,
+not all players support or properly support 2 bits-per-pixel,
+resulting in unusable subtitles.
+@table @option
+@item 0
+The 2 bits-per-pixel encoding format will be used for subtitles with 4
+colors or less. (default)
+
+@item 1
+The 2 bits-per-pixel encoding format will be disabled, and subtitles
+with 4 colors or less will use a 4 bits-per-pixel format.
+@end table
+
+@end table
+
@section dvdsub

This codec encodes the bitmap subtitle format that is used in DVDs.
diff --git a/libavcodec/dvbsubenc.c b/libavcodec/dvbsubenc.c
index 822e3a5309..727a75e901 100644
--- a/libavcodec/dvbsubenc.c
+++ b/libavcodec/dvbsubenc.c
@@ -22,9 +22,13 @@
#include "bytestream.h"
#include "codec_internal.h"
#include "libavutil/colorspace.h"
+#include "libavutil/opt.h"

typedef struct DVBSubtitleContext {
+    AVClass * class;
    int object_version;
+    int disable_2bpp;
+    int min_colors;
} DVBSubtitleContext;

#define PUTBITS2(val)\
@@ -38,6 +42,18 @@ typedef struct DVBSubtitleContext {
    }\
}

+static int dvbsub_init(AVCodecContext *avctx)
+{
+    DVBSubtitleContext *s = avctx->priv_data;
+    if (s->disable_2bpp) {
+        av_log(avctx, AV_LOG_DEBUG, "Disabling 2bpp subtitles.\n");
+        s->min_colors = 16;
+    } else {
+        s->min_colors = 0;
+    }
+    return 0;
+}
+
static int dvb_encode_rle2(uint8_t **pq, int buf_size,
                           const uint8_t *bitmap, int linesize,
                           int w, int h)
@@ -326,18 +342,22 @@ static int dvbsub_encode(AVCodecContext *avctx, uint8_t *outbuf, int buf_size,

    if (h->num_rects) {
        for (clut_id = 0; clut_id < h->num_rects; clut_id++) {
-            if (buf_size < 6 + h->rects[clut_id]->nb_colors * 6)
+            int nb_colors = s->min_colors > h->rects[clut_id]->nb_colors
+                            ? s->min_colors
+                            : h->rects[clut_id]->nb_colors;
+
+            if (buf_size < 6 + nb_colors * 6)
                return AVERROR_BUFFER_TOO_SMALL;

            /* CLUT segment */

-            if (h->rects[clut_id]->nb_colors <= 4) {
+            if (nb_colors <= 4) {
                /* 2 bpp, some decoders do not support it correctly */
                bpp_index = 0;
-            } else if (h->rects[clut_id]->nb_colors <= 16) {
+            } else if (nb_colors <= 16) {
                /* 4 bpp, standard encoding */
                bpp_index = 1;
-            } else if (h->rects[clut_id]->nb_colors <= 256) {
+            } else if (nb_colors <= 256) {
                /* 8 bpp, standard encoding */
                bpp_index = 2;
            } else {
@@ -354,16 +374,24 @@ static int dvbsub_encode(AVCodecContext *avctx, uint8_t *outbuf, int buf_size,
            *q++ = clut_id;
            *q++ = (0 << 4) | 0xf; /* version = 0 */

-            for(i = 0; i < h->rects[clut_id]->nb_colors; i++) {
+            for(i = 0; i < nb_colors; i++) {
                *q++ = i; /* clut_entry_id */
                *q++ = (1 << (7 - bpp_index)) | (0xf << 1) | 1; /* 2 bits/pixel full range */
                {
                    int a, r, g, b;
-                    uint32_t x= ((uint32_t*)h->rects[clut_id]->data[1])[i];
-                    a = (x >> 24) & 0xff;
-                    r = (x >> 16) & 0xff;
-                    g = (x >>  8) & 0xff;
-                    b = (x >>  0) & 0xff;
+                    if (i < h->rects[clut_id]->nb_colors) {
+                        uint32_t x= ((uint32_t*)h->rects[clut_id]->data[1])[i];
+                        a = (x >> 24) & 0xff;
+                        r = (x >> 16) & 0xff;
+                        g = (x >>  8) & 0xff;
+                        b = (x >>  0) & 0xff;
+                    } else {
+                        /* pad out the CLUT */
+                        a = 0;
+                        r = 0;
+                        g = 0;
+                        b = 0;
+                    }

                    *q++ = RGB_TO_Y_CCIR(r, g, b);
                    *q++ = RGB_TO_V_CCIR(r, g, b, 0);
@@ -373,7 +401,7 @@ static int dvbsub_encode(AVCodecContext *avctx, uint8_t *outbuf, int buf_size,
            }

            bytestream_put_be16(&pseg_len, q - pseg_len - 2);
-            buf_size -= 6 + h->rects[clut_id]->nb_colors * 6;
+            buf_size -= 6 + nb_colors * 6;
        }

        if (buf_size < h->num_rects * 22)
@@ -381,14 +409,17 @@ static int dvbsub_encode(AVCodecContext *avctx, uint8_t *outbuf, int buf_size,
        for (region_id = 0; region_id < h->num_rects; region_id++) {

            /* region composition segment */
+            int nb_colors = s->min_colors > h->rects[region_id]->nb_colors
+                            ? s->min_colors
+                            : h->rects[region_id]->nb_colors;

-            if (h->rects[region_id]->nb_colors <= 4) {
+            if (nb_colors <= 4) {
                /* 2 bpp, some decoders do not support it correctly */
                bpp_index = 0;
-            } else if (h->rects[region_id]->nb_colors <= 16) {
+            } else if (nb_colors <= 16) {
                /* 4 bpp, standard encoding */
                bpp_index = 1;
-            } else if (h->rects[region_id]->nb_colors <= 256) {
+            } else if (nb_colors <= 256) {
                /* 8 bpp, standard encoding */
                bpp_index = 2;
            } else {
@@ -424,17 +455,21 @@ static int dvbsub_encode(AVCodecContext *avctx, uint8_t *outbuf, int buf_size,
                                  const uint8_t *bitmap, int linesize,
                                  int w, int h);

+            int nb_colors = s->min_colors > h->rects[object_id]->nb_colors
+                                ? s->min_colors
+                                : h->rects[object_id]->nb_colors;
+
            if (buf_size < 13)
                return AVERROR_BUFFER_TOO_SMALL;

            /* bpp_index maths */
-            if (h->rects[object_id]->nb_colors <= 4) {
+            if (nb_colors <= 4) {
                /* 2 bpp, some decoders do not support it correctly */
                dvb_encode_rle = dvb_encode_rle2;
-            } else if (h->rects[object_id]->nb_colors <= 16) {
+            } else if (nb_colors <= 16) {
                /* 4 bpp, standard encoding */
                dvb_encode_rle = dvb_encode_rle4;
-            } else if (h->rects[object_id]->nb_colors <= 256) {
+            } else if (nb_colors <= 256) {
                /* 8 bpp, standard encoding */
                dvb_encode_rle = dvb_encode_rle8;
            } else {
@@ -506,6 +541,20 @@ static int dvbsub_encode(AVCodecContext *avctx, uint8_t *outbuf, int buf_size,
    return q - outbuf;
}

+#define OFFSET(x) offsetof(DVBSubtitleContext, x)
+#define SE AV_OPT_FLAG_SUBTITLE_PARAM | AV_OPT_FLAG_ENCODING_PARAM
+static const AVOption options[] = {
+    {"disable_2bpp", "disable the 2bpp subtitle encoder", OFFSET(disable_2bpp), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, SE},
+    { NULL },
+};
+
+static const AVClass dvbsubenc_class = {
+    .class_name = "DVBSUB subtitle encoder",
+    .item_name  = av_default_item_name,
+    .option     = options,
+    .version    = LIBAVUTIL_VERSION_INT,
+};
+
const FFCodec ff_dvbsub_encoder = {
    .p.name         = "dvbsub",
    CODEC_LONG_NAME("DVB subtitles"),
@@ -513,4 +562,6 @@ const FFCodec ff_dvbsub_encoder = {
    .p.id           = AV_CODEC_ID_DVB_SUBTITLE,
    .priv_data_size = sizeof(DVBSubtitleContext),
    FF_CODEC_ENCODE_SUB_CB(dvbsub_encode),
+    .init           = dvbsub_init,
+    .p.priv_class   = &dvbsubenc_class,
};
-- 
2.39.5 (Apple Git-154)





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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [FFmpeg-devel] [PATCH] dvbsubenc: add a disable_2bpp option to work around some decoders.
  2025-03-01 11:43 [FFmpeg-devel] [PATCH] dvbsubenc: add a disable_2bpp option to work around some decoders Waider
  2025-03-01 11:48 ` Ronan Waide
@ 2025-03-01 12:55 ` Soft Works
  2025-03-01 13:52   ` Ronan Waide
  1 sibling, 1 reply; 15+ messages in thread
From: Soft Works @ 2025-03-01 12:55 UTC (permalink / raw)
  To: FFmpeg development discussions and patches



> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Waider
> Sent: Samstag, 1. März 2025 12:43
> To: ffmpeg-devel@ffmpeg.org
> Subject: [FFmpeg-devel] [PATCH] dvbsubenc: add a disable_2bpp option to
> work around some decoders.
> 
> As noted in the code in several places, some DVB subtitle decoders
> don't handle 2bpp color. This patch adds a disable_2bpp option which
> disables the 2bpp format; subtitles which would use 2bpp will be bumped
> up to 4bpp.
> 
> Signed-off-by: Ronan Waide <waider@waider.ie>
> ---
> doc/encoders.texi      | 24 ++++++++++++
> libavcodec/dvbsubenc.c | 85 +++++++++++++++++++++++++++++++++---------
> 2 files changed, 92 insertions(+), 17 deletions(-)
> 
> diff --git a/doc/encoders.texi b/doc/encoders.texi
> index f3fcc1aa60..d6dc92a047 100644
> --- a/doc/encoders.texi
> +++ b/doc/encoders.texi
> @@ -4484,6 +4484,30 @@ Reduces detail but attempts to preserve color at
> extremely low bitrates.
> @chapter Subtitles Encoders
> @c man begin SUBTITLES ENCODERS
> 
> +@section dvbsub
> +
> +This codec encodes the bitmap subtitle format that is used in DVB
> +broadcasts and recordings. The bitmaps are typically embedded in a
> +container such as MPEG-TS as a separate stream.
> +
> +@subsection Options
> +
> +@table @option
> +@item disable_2bpp @var{boolean}
> +Disable the 2 bits-per-pixel encoding format.
> +
> +DVB supports 2, 4, and 8 bits-per-pixel color lookup tables. However,
> +not all players support or properly support 2 bits-per-pixel,
> +resulting in unusable subtitles.
> +@table @option
> +@item 0
> +The 2 bits-per-pixel encoding format will be used for subtitles with 4
> +colors or less. (default)
> +@item 1
> +The 2 bits-per-pixel encoding format will be disabled, and subtitles
> +with 4 colors or less will use a 4 bits-per-pixel format.
> +@end table
> +
> @section dvdsub
> 
> This codec encodes the bitmap subtitle format that is used in DVDs.
> diff --git a/libavcodec/dvbsubenc.c b/libavcodec/dvbsubenc.c
> index 822e3a5309..727a75e901 100644
> --- a/libavcodec/dvbsubenc.c
> +++ b/libavcodec/dvbsubenc.c
> @@ -22,9 +22,13 @@
> #include "bytestream.h"
> #include "codec_internal.h"
> #include "libavutil/colorspace.h"
> +#include "libavutil/opt.h"
> 
> typedef struct DVBSubtitleContext {
> +    AVClass * class;
>     int object_version;
> +    int disable_2bpp;
> +    int min_colors;
> } DVBSubtitleContext;
> 
> #define PUTBITS2(val)\
> @@ -38,6 +42,18 @@ typedef struct DVBSubtitleContext {
>     }\
> }
> 
> +static int dvbsub_init(AVCodecContext *avctx)
> +{
> +    DVBSubtitleContext *s = avctx->priv_data;
> +    if (s->disable_2bpp) {
> +        av_log(avctx, AV_LOG_DEBUG, "Disabling 2bpp subtitles.\n");
> +        s->min_colors = 16;
> +    } else {
> +        s->min_colors = 0;
> +    }
> +    return 0;
> +}
> +
> static int dvb_encode_rle2(uint8_t **pq, int buf_size,
>                            const uint8_t *bitmap, int linesize,
>                            int w, int h)
> @@ -326,18 +342,22 @@ static int dvbsub_encode(AVCodecContext *avctx,
> uint8_t *outbuf, int buf_size,
> 
>     if (h->num_rects) {
>         for (clut_id = 0; clut_id < h->num_rects; clut_id++) {
> -            if (buf_size < 6 + h->rects[clut_id]->nb_colors * 6)
> +            int nb_colors = s->min_colors > h->rects[clut_id]-
> >nb_colors
> +                            ? s->min_colors
> +                            : h->rects[clut_id]->nb_colors;
> +
> +            if (buf_size < 6 + nb_colors * 6)
>                 return AVERROR_BUFFER_TOO_SMALL;
> 
>             /* CLUT segment */
> 
> -            if (h->rects[clut_id]->nb_colors <= 4) {
> +            if (nb_colors <= 4) {
>                 /* 2 bpp, some decoders do not support it correctly */
>                 bpp_index = 0;
> -            } else if (h->rects[clut_id]->nb_colors <= 16) {
> +            } else if (nb_colors <= 16) {
>                 /* 4 bpp, standard encoding */
>                 bpp_index = 1;
> -            } else if (h->rects[clut_id]->nb_colors <= 256) {
> +            } else if (nb_colors <= 256) {
>                 /* 8 bpp, standard encoding */
>                 bpp_index = 2;
>             } else {
> @@ -354,16 +374,24 @@ static int dvbsub_encode(AVCodecContext *avctx,
> uint8_t *outbuf, int buf_size,
>             *q++ = clut_id;
>             *q++ = (0 << 4) | 0xf; /* version = 0 */
> 
> -            for(i = 0; i < h->rects[clut_id]->nb_colors; i++) {
> +            for(i = 0; i < nb_colors; i++) {
>                 *q++ = i; /* clut_entry_id */
>                 *q++ = (1 << (7 - bpp_index)) | (0xf << 1) | 1; /* 2
> bits/pixel full range */
>                 {
>                     int a, r, g, b;
> -                    uint32_t x= ((uint32_t*)h->rects[clut_id]-
> >data[1])[i];
> -                    a = (x >> 24) & 0xff;
> -                    r = (x >> 16) & 0xff;
> -                    g = (x >>  8) & 0xff;
> -                    b = (x >>  0) & 0xff;
> +                    if (i < h->rects[clut_id]->nb_colors) {
> +                        uint32_t x= ((uint32_t*)h->rects[clut_id]-
> >data[1])[i];
> +                        a = (x >> 24) & 0xff;
> +                        r = (x >> 16) & 0xff;
> +                        g = (x >>  8) & 0xff;
> +                        b = (x >>  0) & 0xff;
> +                    } else {
> +                        /* pad out the CLUT */
> +                        a = 0;
> +                        r = 0;
> +                        g = 0;
> +                        b = 0;
> +                    }
> 
>                     *q++ = RGB_TO_Y_CCIR(r, g, b);
>                     *q++ = RGB_TO_V_CCIR(r, g, b, 0);
> @@ -373,7 +401,7 @@ static int dvbsub_encode(AVCodecContext *avctx,
> uint8_t *outbuf, int buf_size,
>             }
> 
>             bytestream_put_be16(&pseg_len, q - pseg_len - 2);
> -            buf_size -= 6 + h->rects[clut_id]->nb_colors * 6;
> +            buf_size -= 6 + nb_colors * 6;
>         }
> 
>         if (buf_size < h->num_rects * 22)
> @@ -381,14 +409,17 @@ static int dvbsub_encode(AVCodecContext *avctx,
> uint8_t *outbuf, int buf_size,
>         for (region_id = 0; region_id < h->num_rects; region_id++) {
> 
>             /* region composition segment */
> +            int nb_colors = s->min_colors > h->rects[region_id]-
> >nb_colors
> +                            ? s->min_colors
> +                            : h->rects[region_id]->nb_colors;
> 
> -            if (h->rects[region_id]->nb_colors <= 4) {
> +            if (nb_colors <= 4) {
>                 /* 2 bpp, some decoders do not support it correctly */
>                 bpp_index = 0;
> -            } else if (h->rects[region_id]->nb_colors <= 16) {
> +            } else if (nb_colors <= 16) {
>                 /* 4 bpp, standard encoding */
>                 bpp_index = 1;
> -            } else if (h->rects[region_id]->nb_colors <= 256) {
> +            } else if (nb_colors <= 256) {
>                 /* 8 bpp, standard encoding */
>                 bpp_index = 2;
>             } else {
> @@ -424,17 +455,21 @@ static int dvbsub_encode(AVCodecContext *avctx,
> uint8_t *outbuf, int buf_size,
>                                   const uint8_t *bitmap, int linesize,
>                                   int w, int h);
> 
> +            int nb_colors = s->min_colors > h->rects[object_id]-
> >nb_colors
> +                                ? s->min_colors
> +                                : h->rects[object_id]->nb_colors;
> +
>             if (buf_size < 13)
>                 return AVERROR_BUFFER_TOO_SMALL;
> 
>             /* bpp_index maths */
> -            if (h->rects[object_id]->nb_colors <= 4) {
> +            if (nb_colors <= 4) {
>                 /* 2 bpp, some decoders do not support it correctly */
>                 dvb_encode_rle = dvb_encode_rle2;
> -            } else if (h->rects[object_id]->nb_colors <= 16) {
> +            } else if (nb_colors <= 16) {
>                 /* 4 bpp, standard encoding */
>                 dvb_encode_rle = dvb_encode_rle4;
> -            } else if (h->rects[object_id]->nb_colors <= 256) {
> +            } else if (nb_colors <= 256) {
>                 /* 8 bpp, standard encoding */
>                 dvb_encode_rle = dvb_encode_rle8;
>             } else {
> @@ -506,6 +541,20 @@ static int dvbsub_encode(AVCodecContext *avctx,
> uint8_t *outbuf, int buf_size,
>     return q - outbuf;
> }
> 
> +#define OFFSET(x) offsetof(DVBSubtitleContext, x)
> +#define SE AV_OPT_FLAG_SUBTITLE_PARAM | AV_OPT_FLAG_ENCODING_PARAM
> +static const AVOption options[] = {
> +    {"disable_2bpp", "disable the 2bpp subtitle encoder",
> OFFSET(disable_2bpp), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, SE},
> +    { NULL },
> +};
> +
> +static const AVClass dvbsubenc_class = {
> +    .class_name = "DVBSUB subtitle encoder",
> +    .item_name  = av_default_item_name,
> +    .option     = options,
> +    .version    = LIBAVUTIL_VERSION_INT,
> +};
> +
> const FFCodec ff_dvbsub_encoder = {
>     .p.name         = "dvbsub",
>     CODEC_LONG_NAME("DVB subtitles"),
> @@ -513,4 +562,6 @@ const FFCodec ff_dvbsub_encoder = {
>     .p.id           = AV_CODEC_ID_DVB_SUBTITLE,
>     .priv_data_size = sizeof(DVBSubtitleContext),
>     FF_CODEC_ENCODE_SUB_CB(dvbsub_encode),
> +    .init           = dvbsub_init,
> +    .p.priv_class   = &dvbsubenc_class,
> };
> --

Hi,

thanks for your contribution.

Your patch fails to apply:
https://patchwork.ffmpeg.org/project/ffmpeg/patch/1D1812F5-DB2A-4847-89DB-EF3A1A6C9BCF@waider.ie/

It appears to be created against an older version of the code. Patches need to be made with the current HEAD of the master branch as the base.

Best,
sw
_______________________________________________
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".

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [FFmpeg-devel] [PATCH] dvbsubenc: add a disable_2bpp option to work around some decoders.
  2025-03-01 12:55 ` Soft Works
@ 2025-03-01 13:52   ` Ronan Waide
  2025-03-01 14:10     ` [FFmpeg-devel] [PATCH v3] libavcodec/dvbsubenc.c: " Ronan Waide
  0 siblings, 1 reply; 15+ messages in thread
From: Ronan Waide @ 2025-03-01 13:52 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

> On 1 Mar 2025, at 12:55, Soft Works <softworkz-at-hotmail.com@ffmpeg.org> wrote:
> 
> 
> 
>> -----Original Message-----
>> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Waider
>> Sent: Samstag, 1. März 2025 12:43
>> To: ffmpeg-devel@ffmpeg.org
>> Subject: [FFmpeg-devel] [PATCH] dvbsubenc: add a disable_2bpp option to
>> work around some decoders.
>> 
>> As noted in the code in several places, some DVB subtitle decoders
>> don't handle 2bpp color. This patch adds a disable_2bpp option which
>> disables the 2bpp format; subtitles which would use 2bpp will be bumped
>> up to 4bpp.
>> 
>> Signed-off-by: Ronan Waide <waider@waider.ie>
>> ---
>> doc/encoders.texi      | 24 ++++++++++++
>> libavcodec/dvbsubenc.c | 85 +++++++++++++++++++++++++++++++++---------
>> 2 files changed, 92 insertions(+), 17 deletions(-)
>> 
>> diff --git a/doc/encoders.texi b/doc/encoders.texi
>> index f3fcc1aa60..d6dc92a047 100644
>> --- a/doc/encoders.texi
>> +++ b/doc/encoders.texi
>> @@ -4484,6 +4484,30 @@ Reduces detail but attempts to preserve color at
>> extremely low bitrates.
>> @chapter Subtitles Encoders
>> @c man begin SUBTITLES ENCODERS
>> 
>> +@section dvbsub
>> +
>> +This codec encodes the bitmap subtitle format that is used in DVB
>> +broadcasts and recordings. The bitmaps are typically embedded in a
>> +container such as MPEG-TS as a separate stream.
>> +
>> +@subsection Options
>> +
>> +@table @option
>> +@item disable_2bpp @var{boolean}
>> +Disable the 2 bits-per-pixel encoding format.
>> +
>> +DVB supports 2, 4, and 8 bits-per-pixel color lookup tables. However,
>> +not all players support or properly support 2 bits-per-pixel,
>> +resulting in unusable subtitles.
>> +@table @option
>> +@item 0
>> +The 2 bits-per-pixel encoding format will be used for subtitles with 4
>> +colors or less. (default)
>> +@item 1
>> +The 2 bits-per-pixel encoding format will be disabled, and subtitles
>> +with 4 colors or less will use a 4 bits-per-pixel format.
>> +@end table
>> +
>> @section dvdsub
>> 
>> This codec encodes the bitmap subtitle format that is used in DVDs.
>> diff --git a/libavcodec/dvbsubenc.c b/libavcodec/dvbsubenc.c
>> index 822e3a5309..727a75e901 100644
>> --- a/libavcodec/dvbsubenc.c
>> +++ b/libavcodec/dvbsubenc.c
>> @@ -22,9 +22,13 @@
>> #include "bytestream.h"
>> #include "codec_internal.h"
>> #include "libavutil/colorspace.h"
>> +#include "libavutil/opt.h"
>> 
>> typedef struct DVBSubtitleContext {
>> +    AVClass * class;
>>    int object_version;
>> +    int disable_2bpp;
>> +    int min_colors;
>> } DVBSubtitleContext;
>> 
>> #define PUTBITS2(val)\
>> @@ -38,6 +42,18 @@ typedef struct DVBSubtitleContext {
>>    }\
>> }
>> 
>> +static int dvbsub_init(AVCodecContext *avctx)
>> +{
>> +    DVBSubtitleContext *s = avctx->priv_data;
>> +    if (s->disable_2bpp) {
>> +        av_log(avctx, AV_LOG_DEBUG, "Disabling 2bpp subtitles.\n");
>> +        s->min_colors = 16;
>> +    } else {
>> +        s->min_colors = 0;
>> +    }
>> +    return 0;
>> +}
>> +
>> static int dvb_encode_rle2(uint8_t **pq, int buf_size,
>>                           const uint8_t *bitmap, int linesize,
>>                           int w, int h)
>> @@ -326,18 +342,22 @@ static int dvbsub_encode(AVCodecContext *avctx,
>> uint8_t *outbuf, int buf_size,
>> 
>>    if (h->num_rects) {
>>        for (clut_id = 0; clut_id < h->num_rects; clut_id++) {
>> -            if (buf_size < 6 + h->rects[clut_id]->nb_colors * 6)
>> +            int nb_colors = s->min_colors > h->rects[clut_id]-
>>> nb_colors
>> +                            ? s->min_colors
>> +                            : h->rects[clut_id]->nb_colors;
>> +
>> +            if (buf_size < 6 + nb_colors * 6)
>>                return AVERROR_BUFFER_TOO_SMALL;
>> 
>>            /* CLUT segment */
>> 
>> -            if (h->rects[clut_id]->nb_colors <= 4) {
>> +            if (nb_colors <= 4) {
>>                /* 2 bpp, some decoders do not support it correctly */
>>                bpp_index = 0;
>> -            } else if (h->rects[clut_id]->nb_colors <= 16) {
>> +            } else if (nb_colors <= 16) {
>>                /* 4 bpp, standard encoding */
>>                bpp_index = 1;
>> -            } else if (h->rects[clut_id]->nb_colors <= 256) {
>> +            } else if (nb_colors <= 256) {
>>                /* 8 bpp, standard encoding */
>>                bpp_index = 2;
>>            } else {
>> @@ -354,16 +374,24 @@ static int dvbsub_encode(AVCodecContext *avctx,
>> uint8_t *outbuf, int buf_size,
>>            *q++ = clut_id;
>>            *q++ = (0 << 4) | 0xf; /* version = 0 */
>> 
>> -            for(i = 0; i < h->rects[clut_id]->nb_colors; i++) {
>> +            for(i = 0; i < nb_colors; i++) {
>>                *q++ = i; /* clut_entry_id */
>>                *q++ = (1 << (7 - bpp_index)) | (0xf << 1) | 1; /* 2
>> bits/pixel full range */
>>                {
>>                    int a, r, g, b;
>> -                    uint32_t x= ((uint32_t*)h->rects[clut_id]-
>>> data[1])[i];
>> -                    a = (x >> 24) & 0xff;
>> -                    r = (x >> 16) & 0xff;
>> -                    g = (x >>  8) & 0xff;
>> -                    b = (x >>  0) & 0xff;
>> +                    if (i < h->rects[clut_id]->nb_colors) {
>> +                        uint32_t x= ((uint32_t*)h->rects[clut_id]-
>>> data[1])[i];
>> +                        a = (x >> 24) & 0xff;
>> +                        r = (x >> 16) & 0xff;
>> +                        g = (x >>  8) & 0xff;
>> +                        b = (x >>  0) & 0xff;
>> +                    } else {
>> +                        /* pad out the CLUT */
>> +                        a = 0;
>> +                        r = 0;
>> +                        g = 0;
>> +                        b = 0;
>> +                    }
>> 
>>                    *q++ = RGB_TO_Y_CCIR(r, g, b);
>>                    *q++ = RGB_TO_V_CCIR(r, g, b, 0);
>> @@ -373,7 +401,7 @@ static int dvbsub_encode(AVCodecContext *avctx,
>> uint8_t *outbuf, int buf_size,
>>            }
>> 
>>            bytestream_put_be16(&pseg_len, q - pseg_len - 2);
>> -            buf_size -= 6 + h->rects[clut_id]->nb_colors * 6;
>> +            buf_size -= 6 + nb_colors * 6;
>>        }
>> 
>>        if (buf_size < h->num_rects * 22)
>> @@ -381,14 +409,17 @@ static int dvbsub_encode(AVCodecContext *avctx,
>> uint8_t *outbuf, int buf_size,
>>        for (region_id = 0; region_id < h->num_rects; region_id++) {
>> 
>>            /* region composition segment */
>> +            int nb_colors = s->min_colors > h->rects[region_id]-
>>> nb_colors
>> +                            ? s->min_colors
>> +                            : h->rects[region_id]->nb_colors;
>> 
>> -            if (h->rects[region_id]->nb_colors <= 4) {
>> +            if (nb_colors <= 4) {
>>                /* 2 bpp, some decoders do not support it correctly */
>>                bpp_index = 0;
>> -            } else if (h->rects[region_id]->nb_colors <= 16) {
>> +            } else if (nb_colors <= 16) {
>>                /* 4 bpp, standard encoding */
>>                bpp_index = 1;
>> -            } else if (h->rects[region_id]->nb_colors <= 256) {
>> +            } else if (nb_colors <= 256) {
>>                /* 8 bpp, standard encoding */
>>                bpp_index = 2;
>>            } else {
>> @@ -424,17 +455,21 @@ static int dvbsub_encode(AVCodecContext *avctx,
>> uint8_t *outbuf, int buf_size,
>>                                  const uint8_t *bitmap, int linesize,
>>                                  int w, int h);
>> 
>> +            int nb_colors = s->min_colors > h->rects[object_id]-
>>> nb_colors
>> +                                ? s->min_colors
>> +                                : h->rects[object_id]->nb_colors;
>> +
>>            if (buf_size < 13)
>>                return AVERROR_BUFFER_TOO_SMALL;
>> 
>>            /* bpp_index maths */
>> -            if (h->rects[object_id]->nb_colors <= 4) {
>> +            if (nb_colors <= 4) {
>>                /* 2 bpp, some decoders do not support it correctly */
>>                dvb_encode_rle = dvb_encode_rle2;
>> -            } else if (h->rects[object_id]->nb_colors <= 16) {
>> +            } else if (nb_colors <= 16) {
>>                /* 4 bpp, standard encoding */
>>                dvb_encode_rle = dvb_encode_rle4;
>> -            } else if (h->rects[object_id]->nb_colors <= 256) {
>> +            } else if (nb_colors <= 256) {
>>                /* 8 bpp, standard encoding */
>>                dvb_encode_rle = dvb_encode_rle8;
>>            } else {
>> @@ -506,6 +541,20 @@ static int dvbsub_encode(AVCodecContext *avctx,
>> uint8_t *outbuf, int buf_size,
>>    return q - outbuf;
>> }
>> 
>> +#define OFFSET(x) offsetof(DVBSubtitleContext, x)
>> +#define SE AV_OPT_FLAG_SUBTITLE_PARAM | AV_OPT_FLAG_ENCODING_PARAM
>> +static const AVOption options[] = {
>> +    {"disable_2bpp", "disable the 2bpp subtitle encoder",
>> OFFSET(disable_2bpp), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, SE},
>> +    { NULL },
>> +};
>> +
>> +static const AVClass dvbsubenc_class = {
>> +    .class_name = "DVBSUB subtitle encoder",
>> +    .item_name  = av_default_item_name,
>> +    .option     = options,
>> +    .version    = LIBAVUTIL_VERSION_INT,
>> +};
>> +
>> const FFCodec ff_dvbsub_encoder = {
>>    .p.name         = "dvbsub",
>>    CODEC_LONG_NAME("DVB subtitles"),
>> @@ -513,4 +562,6 @@ const FFCodec ff_dvbsub_encoder = {
>>    .p.id           = AV_CODEC_ID_DVB_SUBTITLE,
>>    .priv_data_size = sizeof(DVBSubtitleContext),
>>    FF_CODEC_ENCODE_SUB_CB(dvbsub_encode),
>> +    .init           = dvbsub_init,
>> +    .p.priv_class   = &dvbsubenc_class,
>> };
>> --
> 
> Hi,
> 
> thanks for your contribution.
> 
> Your patch fails to apply:
> https://patchwork.ffmpeg.org/project/ffmpeg/patch/1D1812F5-DB2A-4847-89DB-EF3A1A6C9BCF@waider.ie/
> 
> It appears to be created against an older version of the code. Patches need to be made with the current HEAD of the master branch as the base.
> 
> Best,
> sw

Huh, weird - I thought I'd done a `git pull --rebase` before submitting. I did find an error in the doc changes I made so I need to resubmit anyway.

Cheers,
Waider.
-- 
Ronan Waide
waider@waider.ie




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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [FFmpeg-devel] [PATCH v3] libavcodec/dvbsubenc.c: add a disable_2bpp option to work around some decoders.
  2025-03-01 13:52   ` Ronan Waide
@ 2025-03-01 14:10     ` Ronan Waide
  2025-03-01 14:29       ` Ronan Waide
  0 siblings, 1 reply; 15+ messages in thread
From: Ronan Waide @ 2025-03-01 14:10 UTC (permalink / raw)
  To: ffmpeg-devel

(changes from previous attempt: fixed an error in the docfile and made sure I'm diffing against origin/master)

As noted in the code in several places, some DVB subtitle decoders
don't handle 2bpp color. This patch adds a disable_2bpp option which
disables the 2bpp format; subtitles which would use 2bpp will be bumped
up to 4bpp.

Signed-off-by: Ronan Waide <waider@waider.ie>
---
doc/encoders.texi      | 27 ++++++++++++++
libavcodec/dvbsubenc.c | 85 +++++++++++++++++++++++++++++++++---------
2 files changed, 95 insertions(+), 17 deletions(-)

diff --git a/doc/encoders.texi b/doc/encoders.texi
index f3fcc1aa60..07984b6b54 100644
--- a/doc/encoders.texi
+++ b/doc/encoders.texi
@@ -4484,6 +4484,33 @@ Reduces detail but attempts to preserve color at extremely low bitrates.
@chapter Subtitles Encoders
@c man begin SUBTITLES ENCODERS

+@section dvbsub
+
+This codec encodes the bitmap subtitle format that is used in DVB
+broadcasts and recordings. The bitmaps are typically embedded in a
+container such as MPEG-TS as a separate stream.
+
+@subsection Options
+
+@table @option
+@item disable_2bpp @var{boolean}
+Disable the 2 bits-per-pixel encoding format.
+
+DVB supports 2, 4, and 8 bits-per-pixel color lookup tables. However,
+not all players support or properly support 2 bits-per-pixel,
+resulting in unusable subtitles.
+@table @option
+@item 0
+The 2 bits-per-pixel encoding format will be used for subtitles with 4
+colors or less. (default)
+
+@item 1
+The 2 bits-per-pixel encoding format will be disabled, and subtitles
+with 4 colors or less will use a 4 bits-per-pixel format.
+@end table
+
+@end table
+
@section dvdsub

This codec encodes the bitmap subtitle format that is used in DVDs.
diff --git a/libavcodec/dvbsubenc.c b/libavcodec/dvbsubenc.c
index 822e3a5309..727a75e901 100644
--- a/libavcodec/dvbsubenc.c
+++ b/libavcodec/dvbsubenc.c
@@ -22,9 +22,13 @@
#include "bytestream.h"
#include "codec_internal.h"
#include "libavutil/colorspace.h"
+#include "libavutil/opt.h"

typedef struct DVBSubtitleContext {
+    AVClass * class;
    int object_version;
+    int disable_2bpp;
+    int min_colors;
} DVBSubtitleContext;

#define PUTBITS2(val)\
@@ -38,6 +42,18 @@ typedef struct DVBSubtitleContext {
    }\
}

+static int dvbsub_init(AVCodecContext *avctx)
+{
+    DVBSubtitleContext *s = avctx->priv_data;
+    if (s->disable_2bpp) {
+        av_log(avctx, AV_LOG_DEBUG, "Disabling 2bpp subtitles.\n");
+        s->min_colors = 16;
+    } else {
+        s->min_colors = 0;
+    }
+    return 0;
+}
+
static int dvb_encode_rle2(uint8_t **pq, int buf_size,
                           const uint8_t *bitmap, int linesize,
                           int w, int h)
@@ -326,18 +342,22 @@ static int dvbsub_encode(AVCodecContext *avctx, uint8_t *outbuf, int buf_size,

    if (h->num_rects) {
        for (clut_id = 0; clut_id < h->num_rects; clut_id++) {
-            if (buf_size < 6 + h->rects[clut_id]->nb_colors * 6)
+            int nb_colors = s->min_colors > h->rects[clut_id]->nb_colors
+                            ? s->min_colors
+                            : h->rects[clut_id]->nb_colors;
+
+            if (buf_size < 6 + nb_colors * 6)
                return AVERROR_BUFFER_TOO_SMALL;

            /* CLUT segment */

-            if (h->rects[clut_id]->nb_colors <= 4) {
+            if (nb_colors <= 4) {
                /* 2 bpp, some decoders do not support it correctly */
                bpp_index = 0;
-            } else if (h->rects[clut_id]->nb_colors <= 16) {
+            } else if (nb_colors <= 16) {
                /* 4 bpp, standard encoding */
                bpp_index = 1;
-            } else if (h->rects[clut_id]->nb_colors <= 256) {
+            } else if (nb_colors <= 256) {
                /* 8 bpp, standard encoding */
                bpp_index = 2;
            } else {
@@ -354,16 +374,24 @@ static int dvbsub_encode(AVCodecContext *avctx, uint8_t *outbuf, int buf_size,
            *q++ = clut_id;
            *q++ = (0 << 4) | 0xf; /* version = 0 */

-            for(i = 0; i < h->rects[clut_id]->nb_colors; i++) {
+            for(i = 0; i < nb_colors; i++) {
                *q++ = i; /* clut_entry_id */
                *q++ = (1 << (7 - bpp_index)) | (0xf << 1) | 1; /* 2 bits/pixel full range */
                {
                    int a, r, g, b;
-                    uint32_t x= ((uint32_t*)h->rects[clut_id]->data[1])[i];
-                    a = (x >> 24) & 0xff;
-                    r = (x >> 16) & 0xff;
-                    g = (x >>  8) & 0xff;
-                    b = (x >>  0) & 0xff;
+                    if (i < h->rects[clut_id]->nb_colors) {
+                        uint32_t x= ((uint32_t*)h->rects[clut_id]->data[1])[i];
+                        a = (x >> 24) & 0xff;
+                        r = (x >> 16) & 0xff;
+                        g = (x >>  8) & 0xff;
+                        b = (x >>  0) & 0xff;
+                    } else {
+                        /* pad out the CLUT */
+                        a = 0;
+                        r = 0;
+                        g = 0;
+                        b = 0;
+                    }

                    *q++ = RGB_TO_Y_CCIR(r, g, b);
                    *q++ = RGB_TO_V_CCIR(r, g, b, 0);
@@ -373,7 +401,7 @@ static int dvbsub_encode(AVCodecContext *avctx, uint8_t *outbuf, int buf_size,
            }

            bytestream_put_be16(&pseg_len, q - pseg_len - 2);
-            buf_size -= 6 + h->rects[clut_id]->nb_colors * 6;
+            buf_size -= 6 + nb_colors * 6;
        }

        if (buf_size < h->num_rects * 22)
@@ -381,14 +409,17 @@ static int dvbsub_encode(AVCodecContext *avctx, uint8_t *outbuf, int buf_size,
        for (region_id = 0; region_id < h->num_rects; region_id++) {

            /* region composition segment */
+            int nb_colors = s->min_colors > h->rects[region_id]->nb_colors
+                            ? s->min_colors
+                            : h->rects[region_id]->nb_colors;

-            if (h->rects[region_id]->nb_colors <= 4) {
+            if (nb_colors <= 4) {
                /* 2 bpp, some decoders do not support it correctly */
                bpp_index = 0;
-            } else if (h->rects[region_id]->nb_colors <= 16) {
+            } else if (nb_colors <= 16) {
                /* 4 bpp, standard encoding */
                bpp_index = 1;
-            } else if (h->rects[region_id]->nb_colors <= 256) {
+            } else if (nb_colors <= 256) {
                /* 8 bpp, standard encoding */
                bpp_index = 2;
            } else {
@@ -424,17 +455,21 @@ static int dvbsub_encode(AVCodecContext *avctx, uint8_t *outbuf, int buf_size,
                                  const uint8_t *bitmap, int linesize,
                                  int w, int h);

+            int nb_colors = s->min_colors > h->rects[object_id]->nb_colors
+                                ? s->min_colors
+                                : h->rects[object_id]->nb_colors;
+
            if (buf_size < 13)
                return AVERROR_BUFFER_TOO_SMALL;

            /* bpp_index maths */
-            if (h->rects[object_id]->nb_colors <= 4) {
+            if (nb_colors <= 4) {
                /* 2 bpp, some decoders do not support it correctly */
                dvb_encode_rle = dvb_encode_rle2;
-            } else if (h->rects[object_id]->nb_colors <= 16) {
+            } else if (nb_colors <= 16) {
                /* 4 bpp, standard encoding */
                dvb_encode_rle = dvb_encode_rle4;
-            } else if (h->rects[object_id]->nb_colors <= 256) {
+            } else if (nb_colors <= 256) {
                /* 8 bpp, standard encoding */
                dvb_encode_rle = dvb_encode_rle8;
            } else {
@@ -506,6 +541,20 @@ static int dvbsub_encode(AVCodecContext *avctx, uint8_t *outbuf, int buf_size,
    return q - outbuf;
}

+#define OFFSET(x) offsetof(DVBSubtitleContext, x)
+#define SE AV_OPT_FLAG_SUBTITLE_PARAM | AV_OPT_FLAG_ENCODING_PARAM
+static const AVOption options[] = {
+    {"disable_2bpp", "disable the 2bpp subtitle encoder", OFFSET(disable_2bpp), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, SE},
+    { NULL },
+};
+
+static const AVClass dvbsubenc_class = {
+    .class_name = "DVBSUB subtitle encoder",
+    .item_name  = av_default_item_name,
+    .option     = options,
+    .version    = LIBAVUTIL_VERSION_INT,
+};
+
const FFCodec ff_dvbsub_encoder = {
    .p.name         = "dvbsub",
    CODEC_LONG_NAME("DVB subtitles"),
@@ -513,4 +562,6 @@ const FFCodec ff_dvbsub_encoder = {
    .p.id           = AV_CODEC_ID_DVB_SUBTITLE,
    .priv_data_size = sizeof(DVBSubtitleContext),
    FF_CODEC_ENCODE_SUB_CB(dvbsub_encode),
+    .init           = dvbsub_init,
+    .p.priv_class   = &dvbsubenc_class,
};
-- 
2.39.5 (Apple Git-154)

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [FFmpeg-devel] [PATCH v3] libavcodec/dvbsubenc.c: add a disable_2bpp option to work around some decoders.
  2025-03-01 14:10     ` [FFmpeg-devel] [PATCH v3] libavcodec/dvbsubenc.c: " Ronan Waide
@ 2025-03-01 14:29       ` Ronan Waide
  2025-03-01 15:00         ` Soft Works
  0 siblings, 1 reply; 15+ messages in thread
From: Ronan Waide @ 2025-03-01 14:29 UTC (permalink / raw)
  To: ffmpeg-devel

> On 1 Mar 2025, at 14:10, Ronan Waide <waider@waider.ie> wrote:
> 
> (changes from previous attempt: fixed an error in the docfile and made sure I'm diffing against origin/master)
> 
> As noted in the code in several places, some DVB subtitle decoders
> don't handle 2bpp color. This patch adds a disable_2bpp option which
> disables the 2bpp format; subtitles which would use 2bpp will be bumped
> up to 4bpp.
> 
> Signed-off-by: Ronan Waide <waider@waider.ie>
> ---
> doc/encoders.texi      | 27 ++++++++++++++
> libavcodec/dvbsubenc.c | 85 +++++++++++++++++++++++++++++++++---------
> 2 files changed, 95 insertions(+), 17 deletions(-)
> 
> diff --git a/doc/encoders.texi b/doc/encoders.texi
> index f3fcc1aa60..07984b6b54 100644
> --- a/doc/encoders.texi
> +++ b/doc/encoders.texi
> @@ -4484,6 +4484,33 @@ Reduces detail but attempts to preserve color at extremely low bitrates.
> @chapter Subtitles Encoders
> @c man begin SUBTITLES ENCODERS
> 
> +@section dvbsub
> +
> +This codec encodes the bitmap subtitle format that is used in DVB
> +broadcasts and recordings. The bitmaps are typically embedded in a
> +container such as MPEG-TS as a separate stream.
> +
> +@subsection Options
> +
> +@table @option
> +@item disable_2bpp @var{boolean}
> +Disable the 2 bits-per-pixel encoding format.
> +
> +DVB supports 2, 4, and 8 bits-per-pixel color lookup tables. However,
> +not all players support or properly support 2 bits-per-pixel,
> +resulting in unusable subtitles.
> +@table @option
> +@item 0
> +The 2 bits-per-pixel encoding format will be used for subtitles with 4
> +colors or less. (default)
> +
> +@item 1
> +The 2 bits-per-pixel encoding format will be disabled, and subtitles
> +with 4 colors or less will use a 4 bits-per-pixel format.
> +@end table
> +
> +@end table
> +
> @section dvdsub
> 
> This codec encodes the bitmap subtitle format that is used in DVDs.
> diff --git a/libavcodec/dvbsubenc.c b/libavcodec/dvbsubenc.c
> index 822e3a5309..727a75e901 100644
> --- a/libavcodec/dvbsubenc.c
> +++ b/libavcodec/dvbsubenc.c
> @@ -22,9 +22,13 @@
> #include "bytestream.h"
> #include "codec_internal.h"
> #include "libavutil/colorspace.h"
> +#include "libavutil/opt.h"
> 
> typedef struct DVBSubtitleContext {
> +    AVClass * class;
>    int object_version;
> +    int disable_2bpp;
> +    int min_colors;
> } DVBSubtitleContext;
> 
> #define PUTBITS2(val)\
> @@ -38,6 +42,18 @@ typedef struct DVBSubtitleContext {
>    }\
> }
> 
> +static int dvbsub_init(AVCodecContext *avctx)
> +{
> +    DVBSubtitleContext *s = avctx->priv_data;
> +    if (s->disable_2bpp) {
> +        av_log(avctx, AV_LOG_DEBUG, "Disabling 2bpp subtitles.\n");
> +        s->min_colors = 16;
> +    } else {
> +        s->min_colors = 0;
> +    }
> +    return 0;
> +}
> +
> static int dvb_encode_rle2(uint8_t **pq, int buf_size,
>                           const uint8_t *bitmap, int linesize,
>                           int w, int h)
> @@ -326,18 +342,22 @@ static int dvbsub_encode(AVCodecContext *avctx, uint8_t *outbuf, int buf_size,
> 
>    if (h->num_rects) {
>        for (clut_id = 0; clut_id < h->num_rects; clut_id++) {
> -            if (buf_size < 6 + h->rects[clut_id]->nb_colors * 6)
> +            int nb_colors = s->min_colors > h->rects[clut_id]->nb_colors
> +                            ? s->min_colors
> +                            : h->rects[clut_id]->nb_colors;
> +
> +            if (buf_size < 6 + nb_colors * 6)
>                return AVERROR_BUFFER_TOO_SMALL;
> 
>            /* CLUT segment */
> 
> -            if (h->rects[clut_id]->nb_colors <= 4) {
> +            if (nb_colors <= 4) {
>                /* 2 bpp, some decoders do not support it correctly */
>                bpp_index = 0;
> -            } else if (h->rects[clut_id]->nb_colors <= 16) {
> +            } else if (nb_colors <= 16) {
>                /* 4 bpp, standard encoding */
>                bpp_index = 1;
> -            } else if (h->rects[clut_id]->nb_colors <= 256) {
> +            } else if (nb_colors <= 256) {
>                /* 8 bpp, standard encoding */
>                bpp_index = 2;
>            } else {
> @@ -354,16 +374,24 @@ static int dvbsub_encode(AVCodecContext *avctx, uint8_t *outbuf, int buf_size,
>            *q++ = clut_id;
>            *q++ = (0 << 4) | 0xf; /* version = 0 */
> 
> -            for(i = 0; i < h->rects[clut_id]->nb_colors; i++) {
> +            for(i = 0; i < nb_colors; i++) {
>                *q++ = i; /* clut_entry_id */
>                *q++ = (1 << (7 - bpp_index)) | (0xf << 1) | 1; /* 2 bits/pixel full range */
>                {
>                    int a, r, g, b;
> -                    uint32_t x= ((uint32_t*)h->rects[clut_id]->data[1])[i];
> -                    a = (x >> 24) & 0xff;
> -                    r = (x >> 16) & 0xff;
> -                    g = (x >>  8) & 0xff;
> -                    b = (x >>  0) & 0xff;
> +                    if (i < h->rects[clut_id]->nb_colors) {
> +                        uint32_t x= ((uint32_t*)h->rects[clut_id]->data[1])[i];
> +                        a = (x >> 24) & 0xff;
> +                        r = (x >> 16) & 0xff;
> +                        g = (x >>  8) & 0xff;
> +                        b = (x >>  0) & 0xff;
> +                    } else {
> +                        /* pad out the CLUT */
> +                        a = 0;
> +                        r = 0;
> +                        g = 0;
> +                        b = 0;
> +                    }
> 
>                    *q++ = RGB_TO_Y_CCIR(r, g, b);
>                    *q++ = RGB_TO_V_CCIR(r, g, b, 0);
> @@ -373,7 +401,7 @@ static int dvbsub_encode(AVCodecContext *avctx, uint8_t *outbuf, int buf_size,
>            }
> 
>            bytestream_put_be16(&pseg_len, q - pseg_len - 2);
> -            buf_size -= 6 + h->rects[clut_id]->nb_colors * 6;
> +            buf_size -= 6 + nb_colors * 6;
>        }
> 
>        if (buf_size < h->num_rects * 22)
> @@ -381,14 +409,17 @@ static int dvbsub_encode(AVCodecContext *avctx, uint8_t *outbuf, int buf_size,
>        for (region_id = 0; region_id < h->num_rects; region_id++) {
> 
>            /* region composition segment */
> +            int nb_colors = s->min_colors > h->rects[region_id]->nb_colors
> +                            ? s->min_colors
> +                            : h->rects[region_id]->nb_colors;
> 
> -            if (h->rects[region_id]->nb_colors <= 4) {
> +            if (nb_colors <= 4) {
>                /* 2 bpp, some decoders do not support it correctly */
>                bpp_index = 0;
> -            } else if (h->rects[region_id]->nb_colors <= 16) {
> +            } else if (nb_colors <= 16) {
>                /* 4 bpp, standard encoding */
>                bpp_index = 1;
> -            } else if (h->rects[region_id]->nb_colors <= 256) {
> +            } else if (nb_colors <= 256) {
>                /* 8 bpp, standard encoding */
>                bpp_index = 2;
>            } else {
> @@ -424,17 +455,21 @@ static int dvbsub_encode(AVCodecContext *avctx, uint8_t *outbuf, int buf_size,
>                                  const uint8_t *bitmap, int linesize,
>                                  int w, int h);
> 
> +            int nb_colors = s->min_colors > h->rects[object_id]->nb_colors
> +                                ? s->min_colors
> +                                : h->rects[object_id]->nb_colors;
> +
>            if (buf_size < 13)
>                return AVERROR_BUFFER_TOO_SMALL;
> 
>            /* bpp_index maths */
> -            if (h->rects[object_id]->nb_colors <= 4) {
> +            if (nb_colors <= 4) {
>                /* 2 bpp, some decoders do not support it correctly */
>                dvb_encode_rle = dvb_encode_rle2;
> -            } else if (h->rects[object_id]->nb_colors <= 16) {
> +            } else if (nb_colors <= 16) {
>                /* 4 bpp, standard encoding */
>                dvb_encode_rle = dvb_encode_rle4;
> -            } else if (h->rects[object_id]->nb_colors <= 256) {
> +            } else if (nb_colors <= 256) {
>                /* 8 bpp, standard encoding */
>                dvb_encode_rle = dvb_encode_rle8;
>            } else {
> @@ -506,6 +541,20 @@ static int dvbsub_encode(AVCodecContext *avctx, uint8_t *outbuf, int buf_size,
>    return q - outbuf;
> }
> 
> +#define OFFSET(x) offsetof(DVBSubtitleContext, x)
> +#define SE AV_OPT_FLAG_SUBTITLE_PARAM | AV_OPT_FLAG_ENCODING_PARAM
> +static const AVOption options[] = {
> +    {"disable_2bpp", "disable the 2bpp subtitle encoder", OFFSET(disable_2bpp), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, SE},
> +    { NULL },
> +};
> +
> +static const AVClass dvbsubenc_class = {
> +    .class_name = "DVBSUB subtitle encoder",
> +    .item_name  = av_default_item_name,
> +    .option     = options,
> +    .version    = LIBAVUTIL_VERSION_INT,
> +};
> +
> const FFCodec ff_dvbsub_encoder = {
>    .p.name         = "dvbsub",
>    CODEC_LONG_NAME("DVB subtitles"),
> @@ -513,4 +562,6 @@ const FFCodec ff_dvbsub_encoder = {
>    .p.id           = AV_CODEC_ID_DVB_SUBTITLE,
>    .priv_data_size = sizeof(DVBSubtitleContext),
>    FF_CODEC_ENCODE_SUB_CB(dvbsub_encode),
> +    .init           = dvbsub_init,
> +    .p.priv_class   = &dvbsubenc_class,
> };
> -- 
> 2.39.5 (Apple Git-154)
> 

This failed again, and I see the problem - I'm diffing against https://github.com/FFmpeg/FFmpeg.git rather than https://git.ffmpeg.org/ffmpeg.git. Apologies for the confusion on my part. Fixed fix of a fix incoming...

Cheers,
Waider.
-- 
Ronan Waide
waider@waider.ie




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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [FFmpeg-devel] [PATCH v3] libavcodec/dvbsubenc.c: add a disable_2bpp option to work around some decoders.
  2025-03-01 14:29       ` Ronan Waide
@ 2025-03-01 15:00         ` Soft Works
  2025-03-01 15:07           ` Ronan Waide
  0 siblings, 1 reply; 15+ messages in thread
From: Soft Works @ 2025-03-01 15:00 UTC (permalink / raw)
  To: FFmpeg development discussions and patches



> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Ronan
> Waide
> Sent: Samstag, 1. März 2025 15:30
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v3] libavcodec/dvbsubenc.c: add a
> disable_2bpp option to work around some decoders.
> 
> > On 1 Mar 2025, at 14:10, Ronan Waide <waider@waider.ie> wrote:
> >
> > (changes from previous attempt: fixed an error in the docfile and made
> sure I'm diffing against origin/master)
> >
> > As noted in the code in several places, some DVB subtitle decoders
> > don't handle 2bpp color. This patch adds a disable_2bpp option which
> > disables the 2bpp format; subtitles which would use 2bpp will be
> bumped
> > up to 4bpp.
> >
> > Signed-off-by: Ronan Waide <waider@waider.ie>
> > ---
> > doc/encoders.texi      | 27 ++++++++++++++
> > libavcodec/dvbsubenc.c | 85 +++++++++++++++++++++++++++++++++---------
> > 2 files changed, 95 insertions(+), 17 deletions(-)
> >


> 
> This failed again, and I see the problem - I'm diffing against
> https://github.com/FFmpeg/FFmpeg.git rather than
> https://git.ffmpeg.org/ffmpeg.git. Apologies for the confusion on my
> part. Fixed fix of a fix incoming...

Both repos are in sync. Git says your patch is corrupted at line 11, but no matter from where I start counting, it doesn't lead to anything that looks suspicious.

You can also make a PR to this GitHub repo: https://github.com/ffstaging/FFmpeg and let it send the patch to the ML automatically.

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [FFmpeg-devel] [PATCH v3] libavcodec/dvbsubenc.c: add a disable_2bpp option to work around some decoders.
  2025-03-01 15:00         ` Soft Works
@ 2025-03-01 15:07           ` Ronan Waide
  2025-03-01 15:32             ` [FFmpeg-devel] [PATCH v4] " Ronan Waide
  0 siblings, 1 reply; 15+ messages in thread
From: Ronan Waide @ 2025-03-01 15:07 UTC (permalink / raw)
  To: FFmpeg development discussions and patches



> On 1 Mar 2025, at 15:00, Soft Works <softworkz-at-hotmail.com@ffmpeg.org> wrote:
> 
> 
> 
>> -----Original Message-----
>> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Ronan
>> Waide
>> Sent: Samstag, 1. März 2025 15:30
>> To: ffmpeg-devel@ffmpeg.org
>> Subject: Re: [FFmpeg-devel] [PATCH v3] libavcodec/dvbsubenc.c: add a
>> disable_2bpp option to work around some decoders.
>> 
>>> On 1 Mar 2025, at 14:10, Ronan Waide <waider@waider.ie> wrote:
>>> 
>>> (changes from previous attempt: fixed an error in the docfile and made
>> sure I'm diffing against origin/master)
>>> 
>>> As noted in the code in several places, some DVB subtitle decoders
>>> don't handle 2bpp color. This patch adds a disable_2bpp option which
>>> disables the 2bpp format; subtitles which would use 2bpp will be
>> bumped
>>> up to 4bpp.
>>> 
>>> Signed-off-by: Ronan Waide <waider@waider.ie>
>>> ---
>>> doc/encoders.texi      | 27 ++++++++++++++
>>> libavcodec/dvbsubenc.c | 85 +++++++++++++++++++++++++++++++++---------
>>> 2 files changed, 95 insertions(+), 17 deletions(-)
>>> 
> 
> 
>> 
>> This failed again, and I see the problem - I'm diffing against
>> https://github.com/FFmpeg/FFmpeg.git rather than
>> https://git.ffmpeg.org/ffmpeg.git. Apologies for the confusion on my
>> part. Fixed fix of a fix incoming...
> 
> Both repos are in sync. Git says your patch is corrupted at line 11, but no matter from where I start counting, it doesn't lead to anything that looks suspicious.
> 
> You can also make a PR to this GitHub repo: https://github.com/ffstaging/FFmpeg and let it send the patch to the ML automatically.

I think I've identified the problem - I'm trying to use the `git format-patch` trick to generate a .eml file and then use Apple Mail's "Send Again" feature to send it, but it looks like some part of that process is corrupting the patch by removing at least some leading whitespace. I'll see if I can fix this without sending more emails to the list, and if I can't fix it (locally) I'll use the GitHub approach.

Thanks for your feedback!

Cheers,
Waider.
-- 
Ronan Waide
waider@waider.ie




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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [FFmpeg-devel] [PATCH v4] libavcodec/dvbsubenc.c: add a disable_2bpp option to work around some decoders.
  2025-03-01 15:07           ` Ronan Waide
@ 2025-03-01 15:32             ` Ronan Waide
  2025-03-02 13:13               ` Andreas Rheinhardt
  2025-03-02 15:00               ` Soft Works
  0 siblings, 2 replies; 15+ messages in thread
From: Ronan Waide @ 2025-03-01 15:32 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Ronan Waide

As noted in the code in several places, some DVB subtitle decoders
don't handle 2bpp color. This patch adds a disable_2bpp option which
disables the 2bpp format; subtitles which would use 2bpp will be bumped
up to 4bpp.

Signed-off-by: Ronan Waide <waider@waider.ie>
---
 doc/encoders.texi      | 27 ++++++++++++++
 libavcodec/dvbsubenc.c | 85 +++++++++++++++++++++++++++++++++---------
 2 files changed, 95 insertions(+), 17 deletions(-)

diff --git a/doc/encoders.texi b/doc/encoders.texi
index f3fcc1aa60..07984b6b54 100644
--- a/doc/encoders.texi
+++ b/doc/encoders.texi
@@ -4484,6 +4484,33 @@ Reduces detail but attempts to preserve color at extremely low bitrates.
 @chapter Subtitles Encoders
 @c man begin SUBTITLES ENCODERS
 
+@section dvbsub
+
+This codec encodes the bitmap subtitle format that is used in DVB
+broadcasts and recordings. The bitmaps are typically embedded in a
+container such as MPEG-TS as a separate stream.
+
+@subsection Options
+
+@table @option
+@item disable_2bpp @var{boolean}
+Disable the 2 bits-per-pixel encoding format.
+
+DVB supports 2, 4, and 8 bits-per-pixel color lookup tables. However,
+not all players support or properly support 2 bits-per-pixel,
+resulting in unusable subtitles.
+@table @option
+@item 0
+The 2 bits-per-pixel encoding format will be used for subtitles with 4
+colors or less. (default)
+
+@item 1
+The 2 bits-per-pixel encoding format will be disabled, and subtitles
+with 4 colors or less will use a 4 bits-per-pixel format.
+@end table
+
+@end table
+
 @section dvdsub
 
 This codec encodes the bitmap subtitle format that is used in DVDs.
diff --git a/libavcodec/dvbsubenc.c b/libavcodec/dvbsubenc.c
index 822e3a5309..727a75e901 100644
--- a/libavcodec/dvbsubenc.c
+++ b/libavcodec/dvbsubenc.c
@@ -22,9 +22,13 @@
 #include "bytestream.h"
 #include "codec_internal.h"
 #include "libavutil/colorspace.h"
+#include "libavutil/opt.h"
 
 typedef struct DVBSubtitleContext {
+    AVClass * class;
     int object_version;
+    int disable_2bpp;
+    int min_colors;
 } DVBSubtitleContext;
 
 #define PUTBITS2(val)\
@@ -38,6 +42,18 @@ typedef struct DVBSubtitleContext {
     }\
 }
 
+static int dvbsub_init(AVCodecContext *avctx)
+{
+    DVBSubtitleContext *s = avctx->priv_data;
+    if (s->disable_2bpp) {
+        av_log(avctx, AV_LOG_DEBUG, "Disabling 2bpp subtitles.\n");
+        s->min_colors = 16;
+    } else {
+        s->min_colors = 0;
+    }
+    return 0;
+}
+
 static int dvb_encode_rle2(uint8_t **pq, int buf_size,
                            const uint8_t *bitmap, int linesize,
                            int w, int h)
@@ -326,18 +342,22 @@ static int dvbsub_encode(AVCodecContext *avctx, uint8_t *outbuf, int buf_size,
 
     if (h->num_rects) {
         for (clut_id = 0; clut_id < h->num_rects; clut_id++) {
-            if (buf_size < 6 + h->rects[clut_id]->nb_colors * 6)
+            int nb_colors = s->min_colors > h->rects[clut_id]->nb_colors
+                            ? s->min_colors
+                            : h->rects[clut_id]->nb_colors;
+
+            if (buf_size < 6 + nb_colors * 6)
                 return AVERROR_BUFFER_TOO_SMALL;
 
             /* CLUT segment */
 
-            if (h->rects[clut_id]->nb_colors <= 4) {
+            if (nb_colors <= 4) {
                 /* 2 bpp, some decoders do not support it correctly */
                 bpp_index = 0;
-            } else if (h->rects[clut_id]->nb_colors <= 16) {
+            } else if (nb_colors <= 16) {
                 /* 4 bpp, standard encoding */
                 bpp_index = 1;
-            } else if (h->rects[clut_id]->nb_colors <= 256) {
+            } else if (nb_colors <= 256) {
                 /* 8 bpp, standard encoding */
                 bpp_index = 2;
             } else {
@@ -354,16 +374,24 @@ static int dvbsub_encode(AVCodecContext *avctx, uint8_t *outbuf, int buf_size,
             *q++ = clut_id;
             *q++ = (0 << 4) | 0xf; /* version = 0 */
 
-            for(i = 0; i < h->rects[clut_id]->nb_colors; i++) {
+            for(i = 0; i < nb_colors; i++) {
                 *q++ = i; /* clut_entry_id */
                 *q++ = (1 << (7 - bpp_index)) | (0xf << 1) | 1; /* 2 bits/pixel full range */
                 {
                     int a, r, g, b;
-                    uint32_t x= ((uint32_t*)h->rects[clut_id]->data[1])[i];
-                    a = (x >> 24) & 0xff;
-                    r = (x >> 16) & 0xff;
-                    g = (x >>  8) & 0xff;
-                    b = (x >>  0) & 0xff;
+                    if (i < h->rects[clut_id]->nb_colors) {
+                        uint32_t x= ((uint32_t*)h->rects[clut_id]->data[1])[i];
+                        a = (x >> 24) & 0xff;
+                        r = (x >> 16) & 0xff;
+                        g = (x >>  8) & 0xff;
+                        b = (x >>  0) & 0xff;
+                    } else {
+                        /* pad out the CLUT */
+                        a = 0;
+                        r = 0;
+                        g = 0;
+                        b = 0;
+                    }
 
                     *q++ = RGB_TO_Y_CCIR(r, g, b);
                     *q++ = RGB_TO_V_CCIR(r, g, b, 0);
@@ -373,7 +401,7 @@ static int dvbsub_encode(AVCodecContext *avctx, uint8_t *outbuf, int buf_size,
             }
 
             bytestream_put_be16(&pseg_len, q - pseg_len - 2);
-            buf_size -= 6 + h->rects[clut_id]->nb_colors * 6;
+            buf_size -= 6 + nb_colors * 6;
         }
 
         if (buf_size < h->num_rects * 22)
@@ -381,14 +409,17 @@ static int dvbsub_encode(AVCodecContext *avctx, uint8_t *outbuf, int buf_size,
         for (region_id = 0; region_id < h->num_rects; region_id++) {
 
             /* region composition segment */
+            int nb_colors = s->min_colors > h->rects[region_id]->nb_colors
+                            ? s->min_colors
+                            : h->rects[region_id]->nb_colors;
 
-            if (h->rects[region_id]->nb_colors <= 4) {
+            if (nb_colors <= 4) {
                 /* 2 bpp, some decoders do not support it correctly */
                 bpp_index = 0;
-            } else if (h->rects[region_id]->nb_colors <= 16) {
+            } else if (nb_colors <= 16) {
                 /* 4 bpp, standard encoding */
                 bpp_index = 1;
-            } else if (h->rects[region_id]->nb_colors <= 256) {
+            } else if (nb_colors <= 256) {
                 /* 8 bpp, standard encoding */
                 bpp_index = 2;
             } else {
@@ -424,17 +455,21 @@ static int dvbsub_encode(AVCodecContext *avctx, uint8_t *outbuf, int buf_size,
                                   const uint8_t *bitmap, int linesize,
                                   int w, int h);
 
+            int nb_colors = s->min_colors > h->rects[object_id]->nb_colors
+                                ? s->min_colors
+                                : h->rects[object_id]->nb_colors;
+
             if (buf_size < 13)
                 return AVERROR_BUFFER_TOO_SMALL;
 
             /* bpp_index maths */
-            if (h->rects[object_id]->nb_colors <= 4) {
+            if (nb_colors <= 4) {
                 /* 2 bpp, some decoders do not support it correctly */
                 dvb_encode_rle = dvb_encode_rle2;
-            } else if (h->rects[object_id]->nb_colors <= 16) {
+            } else if (nb_colors <= 16) {
                 /* 4 bpp, standard encoding */
                 dvb_encode_rle = dvb_encode_rle4;
-            } else if (h->rects[object_id]->nb_colors <= 256) {
+            } else if (nb_colors <= 256) {
                 /* 8 bpp, standard encoding */
                 dvb_encode_rle = dvb_encode_rle8;
             } else {
@@ -506,6 +541,20 @@ static int dvbsub_encode(AVCodecContext *avctx, uint8_t *outbuf, int buf_size,
     return q - outbuf;
 }
 
+#define OFFSET(x) offsetof(DVBSubtitleContext, x)
+#define SE AV_OPT_FLAG_SUBTITLE_PARAM | AV_OPT_FLAG_ENCODING_PARAM
+static const AVOption options[] = {
+    {"disable_2bpp", "disable the 2bpp subtitle encoder", OFFSET(disable_2bpp), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, SE},
+    { NULL },
+};
+
+static const AVClass dvbsubenc_class = {
+    .class_name = "DVBSUB subtitle encoder",
+    .item_name  = av_default_item_name,
+    .option     = options,
+    .version    = LIBAVUTIL_VERSION_INT,
+};
+
 const FFCodec ff_dvbsub_encoder = {
     .p.name         = "dvbsub",
     CODEC_LONG_NAME("DVB subtitles"),
@@ -513,4 +562,6 @@ const FFCodec ff_dvbsub_encoder = {
     .p.id           = AV_CODEC_ID_DVB_SUBTITLE,
     .priv_data_size = sizeof(DVBSubtitleContext),
     FF_CODEC_ENCODE_SUB_CB(dvbsub_encode),
+    .init           = dvbsub_init,
+    .p.priv_class   = &dvbsubenc_class,
 };
-- 
2.39.5 (Apple Git-154)

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [FFmpeg-devel] [PATCH v4] libavcodec/dvbsubenc.c: add a disable_2bpp option to work around some decoders.
  2025-03-01 15:32             ` [FFmpeg-devel] [PATCH v4] " Ronan Waide
@ 2025-03-02 13:13               ` Andreas Rheinhardt
  2025-03-02 15:00               ` Soft Works
  1 sibling, 0 replies; 15+ messages in thread
From: Andreas Rheinhardt @ 2025-03-02 13:13 UTC (permalink / raw)
  To: ffmpeg-devel

Ronan Waide:
> As noted in the code in several places, some DVB subtitle decoders
> don't handle 2bpp color. This patch adds a disable_2bpp option which
> disables the 2bpp format; subtitles which would use 2bpp will be bumped
> up to 4bpp.
> 
> Signed-off-by: Ronan Waide <waider@waider.ie>
> ---
>  doc/encoders.texi      | 27 ++++++++++++++
>  libavcodec/dvbsubenc.c | 85 +++++++++++++++++++++++++++++++++---------
>  2 files changed, 95 insertions(+), 17 deletions(-)
> 
> diff --git a/doc/encoders.texi b/doc/encoders.texi
> index f3fcc1aa60..07984b6b54 100644
> --- a/doc/encoders.texi
> +++ b/doc/encoders.texi
> @@ -4484,6 +4484,33 @@ Reduces detail but attempts to preserve color at extremely low bitrates.
>  @chapter Subtitles Encoders
>  @c man begin SUBTITLES ENCODERS
>  
> +@section dvbsub
> +
> +This codec encodes the bitmap subtitle format that is used in DVB
> +broadcasts and recordings. The bitmaps are typically embedded in a
> +container such as MPEG-TS as a separate stream.
> +
> +@subsection Options
> +
> +@table @option
> +@item disable_2bpp @var{boolean}
> +Disable the 2 bits-per-pixel encoding format.
> +
> +DVB supports 2, 4, and 8 bits-per-pixel color lookup tables. However,
> +not all players support or properly support 2 bits-per-pixel,
> +resulting in unusable subtitles.
> +@table @option
> +@item 0
> +The 2 bits-per-pixel encoding format will be used for subtitles with 4
> +colors or less. (default)
> +
> +@item 1
> +The 2 bits-per-pixel encoding format will be disabled, and subtitles
> +with 4 colors or less will use a 4 bits-per-pixel format.
> +@end table
> +
> +@end table
> +
>  @section dvdsub
>  
>  This codec encodes the bitmap subtitle format that is used in DVDs.
> diff --git a/libavcodec/dvbsubenc.c b/libavcodec/dvbsubenc.c
> index 822e3a5309..727a75e901 100644
> --- a/libavcodec/dvbsubenc.c
> +++ b/libavcodec/dvbsubenc.c
> @@ -22,9 +22,13 @@
>  #include "bytestream.h"
>  #include "codec_internal.h"
>  #include "libavutil/colorspace.h"
> +#include "libavutil/opt.h"
>  
>  typedef struct DVBSubtitleContext {
> +    AVClass * class;
>      int object_version;
> +    int disable_2bpp;
> +    int min_colors;
>  } DVBSubtitleContext;
>  
>  #define PUTBITS2(val)\
> @@ -38,6 +42,18 @@ typedef struct DVBSubtitleContext {
>      }\
>  }
>  
> +static int dvbsub_init(AVCodecContext *avctx)
> +{
> +    DVBSubtitleContext *s = avctx->priv_data;
> +    if (s->disable_2bpp) {
> +        av_log(avctx, AV_LOG_DEBUG, "Disabling 2bpp subtitles.\n");

Pointless log message. In fact this whole function could be eliminated
by simply adding
    const int min_colors = s->disable_2bpp ? 16 : 0;
to dvbsub_encode().

> +        s->min_colors = 16;
> +    } else {
> +        s->min_colors = 0;
> +    }
> +    return 0;
> +}
> +
>  static int dvb_encode_rle2(uint8_t **pq, int buf_size,
>                             const uint8_t *bitmap, int linesize,
>                             int w, int h)
> @@ -326,18 +342,22 @@ static int dvbsub_encode(AVCodecContext *avctx, uint8_t *outbuf, int buf_size,
>  
>      if (h->num_rects) {
>          for (clut_id = 0; clut_id < h->num_rects; clut_id++) {
> -            if (buf_size < 6 + h->rects[clut_id]->nb_colors * 6)
> +            int nb_colors = s->min_colors > h->rects[clut_id]->nb_colors
> +                            ? s->min_colors
> +                            : h->rects[clut_id]->nb_colors;
> +
> +            if (buf_size < 6 + nb_colors * 6)
>                  return AVERROR_BUFFER_TOO_SMALL;
>  
>              /* CLUT segment */
>  
> -            if (h->rects[clut_id]->nb_colors <= 4) {
> +            if (nb_colors <= 4) {
>                  /* 2 bpp, some decoders do not support it correctly */
>                  bpp_index = 0;
> -            } else if (h->rects[clut_id]->nb_colors <= 16) {
> +            } else if (nb_colors <= 16) {
>                  /* 4 bpp, standard encoding */
>                  bpp_index = 1;
> -            } else if (h->rects[clut_id]->nb_colors <= 256) {
> +            } else if (nb_colors <= 256) {
>                  /* 8 bpp, standard encoding */
>                  bpp_index = 2;
>              } else {
> @@ -354,16 +374,24 @@ static int dvbsub_encode(AVCodecContext *avctx, uint8_t *outbuf, int buf_size,
>              *q++ = clut_id;
>              *q++ = (0 << 4) | 0xf; /* version = 0 */
>  
> -            for(i = 0; i < h->rects[clut_id]->nb_colors; i++) {
> +            for(i = 0; i < nb_colors; i++) {
>                  *q++ = i; /* clut_entry_id */
>                  *q++ = (1 << (7 - bpp_index)) | (0xf << 1) | 1; /* 2 bits/pixel full range */
>                  {
>                      int a, r, g, b;
> -                    uint32_t x= ((uint32_t*)h->rects[clut_id]->data[1])[i];
> -                    a = (x >> 24) & 0xff;
> -                    r = (x >> 16) & 0xff;
> -                    g = (x >>  8) & 0xff;
> -                    b = (x >>  0) & 0xff;
> +                    if (i < h->rects[clut_id]->nb_colors) {
> +                        uint32_t x= ((uint32_t*)h->rects[clut_id]->data[1])[i];
> +                        a = (x >> 24) & 0xff;
> +                        r = (x >> 16) & 0xff;
> +                        g = (x >>  8) & 0xff;
> +                        b = (x >>  0) & 0xff;
> +                    } else {
> +                        /* pad out the CLUT */
> +                        a = 0;
> +                        r = 0;
> +                        g = 0;
> +                        b = 0;
> +                    }
>  
>                      *q++ = RGB_TO_Y_CCIR(r, g, b);
>                      *q++ = RGB_TO_V_CCIR(r, g, b, 0);
> @@ -373,7 +401,7 @@ static int dvbsub_encode(AVCodecContext *avctx, uint8_t *outbuf, int buf_size,
>              }
>  
>              bytestream_put_be16(&pseg_len, q - pseg_len - 2);
> -            buf_size -= 6 + h->rects[clut_id]->nb_colors * 6;
> +            buf_size -= 6 + nb_colors * 6;
>          }
>  
>          if (buf_size < h->num_rects * 22)
> @@ -381,14 +409,17 @@ static int dvbsub_encode(AVCodecContext *avctx, uint8_t *outbuf, int buf_size,
>          for (region_id = 0; region_id < h->num_rects; region_id++) {
>  
>              /* region composition segment */
> +            int nb_colors = s->min_colors > h->rects[region_id]->nb_colors
> +                            ? s->min_colors
> +                            : h->rects[region_id]->nb_colors;
>  
> -            if (h->rects[region_id]->nb_colors <= 4) {
> +            if (nb_colors <= 4) {
>                  /* 2 bpp, some decoders do not support it correctly */
>                  bpp_index = 0;
> -            } else if (h->rects[region_id]->nb_colors <= 16) {
> +            } else if (nb_colors <= 16) {
>                  /* 4 bpp, standard encoding */
>                  bpp_index = 1;
> -            } else if (h->rects[region_id]->nb_colors <= 256) {
> +            } else if (nb_colors <= 256) {
>                  /* 8 bpp, standard encoding */
>                  bpp_index = 2;
>              } else {
> @@ -424,17 +455,21 @@ static int dvbsub_encode(AVCodecContext *avctx, uint8_t *outbuf, int buf_size,
>                                    const uint8_t *bitmap, int linesize,
>                                    int w, int h);
>  
> +            int nb_colors = s->min_colors > h->rects[object_id]->nb_colors
> +                                ? s->min_colors
> +                                : h->rects[object_id]->nb_colors;
> +
>              if (buf_size < 13)
>                  return AVERROR_BUFFER_TOO_SMALL;
>  
>              /* bpp_index maths */
> -            if (h->rects[object_id]->nb_colors <= 4) {
> +            if (nb_colors <= 4) {
>                  /* 2 bpp, some decoders do not support it correctly */
>                  dvb_encode_rle = dvb_encode_rle2;
> -            } else if (h->rects[object_id]->nb_colors <= 16) {
> +            } else if (nb_colors <= 16) {
>                  /* 4 bpp, standard encoding */
>                  dvb_encode_rle = dvb_encode_rle4;
> -            } else if (h->rects[object_id]->nb_colors <= 256) {
> +            } else if (nb_colors <= 256) {
>                  /* 8 bpp, standard encoding */
>                  dvb_encode_rle = dvb_encode_rle8;
>              } else {
> @@ -506,6 +541,20 @@ static int dvbsub_encode(AVCodecContext *avctx, uint8_t *outbuf, int buf_size,
>      return q - outbuf;
>  }
>  
> +#define OFFSET(x) offsetof(DVBSubtitleContext, x)
> +#define SE AV_OPT_FLAG_SUBTITLE_PARAM | AV_OPT_FLAG_ENCODING_PARAM
> +static const AVOption options[] = {
> +    {"disable_2bpp", "disable the 2bpp subtitle encoder", OFFSET(disable_2bpp), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, SE},
> +    { NULL },
> +};
> +
> +static const AVClass dvbsubenc_class = {
> +    .class_name = "DVBSUB subtitle encoder",
> +    .item_name  = av_default_item_name,
> +    .option     = options,
> +    .version    = LIBAVUTIL_VERSION_INT,
> +};
> +
>  const FFCodec ff_dvbsub_encoder = {
>      .p.name         = "dvbsub",
>      CODEC_LONG_NAME("DVB subtitles"),
> @@ -513,4 +562,6 @@ const FFCodec ff_dvbsub_encoder = {
>      .p.id           = AV_CODEC_ID_DVB_SUBTITLE,
>      .priv_data_size = sizeof(DVBSubtitleContext),
>      FF_CODEC_ENCODE_SUB_CB(dvbsub_encode),
> +    .init           = dvbsub_init,
> +    .p.priv_class   = &dvbsubenc_class,
>  };

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [FFmpeg-devel] [PATCH v4] libavcodec/dvbsubenc.c: add a disable_2bpp option to work around some decoders.
  2025-03-01 15:32             ` [FFmpeg-devel] [PATCH v4] " Ronan Waide
  2025-03-02 13:13               ` Andreas Rheinhardt
@ 2025-03-02 15:00               ` Soft Works
  2025-03-02 17:23                 ` [FFmpeg-devel] [PATCH v5] " Ronan Waide
  2025-03-02 17:26                 ` [FFmpeg-devel] [PATCH v4] " Ronan Waide
  1 sibling, 2 replies; 15+ messages in thread
From: Soft Works @ 2025-03-02 15:00 UTC (permalink / raw)
  To: FFmpeg development discussions and patches; +Cc: Ronan Waide



> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Ronan
> Waide
> Sent: Samstag, 1. März 2025 16:32
> To: ffmpeg-devel@ffmpeg.org
> Cc: Ronan Waide <waider@waider.ie>
> Subject: [FFmpeg-devel] [PATCH v4] libavcodec/dvbsubenc.c: add a
> disable_2bpp option to work around some decoders.
> 
> As noted in the code in several places, some DVB subtitle decoders
> don't handle 2bpp color. This patch adds a disable_2bpp option which
> disables the 2bpp format; subtitles which would use 2bpp will be bumped
> up to 4bpp.
> 
> Signed-off-by: Ronan Waide <waider@waider.ie>
> ---
>  doc/encoders.texi      | 27 ++++++++++++++
>  libavcodec/dvbsubenc.c | 85 +++++++++++++++++++++++++++++++++---------
>  2 files changed, 95 insertions(+), 17 deletions(-)
> 
> diff --git a/doc/encoders.texi b/doc/encoders.texi
> index f3fcc1aa60..07984b6b54 100644
> --- a/doc/encoders.texi
> +++ b/doc/encoders.texi
> @@ -4484,6 +4484,33 @@ Reduces detail but attempts to preserve color at
> extremely low bitrates.
>  @chapter Subtitles Encoders
>  @c man begin SUBTITLES ENCODERS
> 
> +@section dvbsub
> +
> +This codec encodes the bitmap subtitle format that is used in DVB
> +broadcasts and recordings. The bitmaps are typically embedded in a
> +container such as MPEG-TS as a separate stream.
> +
> +@subsection Options
> +
> +@table @option
> +@item disable_2bpp @var{boolean}
> +Disable the 2 bits-per-pixel encoding format.
> +
> +DVB supports 2, 4, and 8 bits-per-pixel color lookup tables. However,
> +not all players support or properly support 2 bits-per-pixel,
> +resulting in unusable subtitles.
> +@table @option
> +@item 0
> +The 2 bits-per-pixel encoding format will be used for subtitles with 4
> +colors or less. (default)
> +
> +@item 1
> +The 2 bits-per-pixel encoding format will be disabled, and subtitles
> +with 4 colors or less will use a 4 bits-per-pixel format.
> +@end table
> +
> +@end table
> +
>  @section dvdsub
> 
>  This codec encodes the bitmap subtitle format that is used in DVDs.
> diff --git a/libavcodec/dvbsubenc.c b/libavcodec/dvbsubenc.c
> index 822e3a5309..727a75e901 100644
> --- a/libavcodec/dvbsubenc.c
> +++ b/libavcodec/dvbsubenc.c
> @@ -22,9 +22,13 @@
>  #include "bytestream.h"
>  #include "codec_internal.h"
>  #include "libavutil/colorspace.h"
> +#include "libavutil/opt.h"
> 
>  typedef struct DVBSubtitleContext {
> +    AVClass * class;
>      int object_version;
> +    int disable_2bpp;
> +    int min_colors;
>  } DVBSubtitleContext;
> 
>  #define PUTBITS2(val)\
> @@ -38,6 +42,18 @@ typedef struct DVBSubtitleContext {
>      }\
>  }
> 
> +static int dvbsub_init(AVCodecContext *avctx)
> +{
> +    DVBSubtitleContext *s = avctx->priv_data;
> +    if (s->disable_2bpp) {
> +        av_log(avctx, AV_LOG_DEBUG, "Disabling 2bpp subtitles.\n");
> +        s->min_colors = 16;
> +    } else {
> +        s->min_colors = 0;
> +    }
> +    return 0;
> +}
> +
>  static int dvb_encode_rle2(uint8_t **pq, int buf_size,
>                             const uint8_t *bitmap, int linesize,
>                             int w, int h)
> @@ -326,18 +342,22 @@ static int dvbsub_encode(AVCodecContext *avctx,
> uint8_t *outbuf, int buf_size,
> 
>      if (h->num_rects) {
>          for (clut_id = 0; clut_id < h->num_rects; clut_id++) {
> -            if (buf_size < 6 + h->rects[clut_id]->nb_colors * 6)
> +            int nb_colors = s->min_colors > h->rects[clut_id]-
> >nb_colors
> +                            ? s->min_colors
> +                            : h->rects[clut_id]->nb_colors;
> +
> +            if (buf_size < 6 + nb_colors * 6)
>                  return AVERROR_BUFFER_TOO_SMALL;
> 
>              /* CLUT segment */
> 
> -            if (h->rects[clut_id]->nb_colors <= 4) {
> +            if (nb_colors <= 4) {
>                  /* 2 bpp, some decoders do not support it correctly */
>                  bpp_index = 0;
> -            } else if (h->rects[clut_id]->nb_colors <= 16) {
> +            } else if (nb_colors <= 16) {
>                  /* 4 bpp, standard encoding */
>                  bpp_index = 1;
> -            } else if (h->rects[clut_id]->nb_colors <= 256) {
> +            } else if (nb_colors <= 256) {
>                  /* 8 bpp, standard encoding */
>                  bpp_index = 2;
>              } else {
> @@ -354,16 +374,24 @@ static int dvbsub_encode(AVCodecContext *avctx,
> uint8_t *outbuf, int buf_size,
>              *q++ = clut_id;
>              *q++ = (0 << 4) | 0xf; /* version = 0 */
> 
> -            for(i = 0; i < h->rects[clut_id]->nb_colors; i++) {
> +            for(i = 0; i < nb_colors; i++) {
>                  *q++ = i; /* clut_entry_id */
>                  *q++ = (1 << (7 - bpp_index)) | (0xf << 1) | 1; /* 2
> bits/pixel full range */
>                  {
>                      int a, r, g, b;
> -                    uint32_t x= ((uint32_t*)h->rects[clut_id]-
> >data[1])[i];
> -                    a = (x >> 24) & 0xff;
> -                    r = (x >> 16) & 0xff;
> -                    g = (x >>  8) & 0xff;
> -                    b = (x >>  0) & 0xff;
> +                    if (i < h->rects[clut_id]->nb_colors) {
> +                        uint32_t x= ((uint32_t*)h->rects[clut_id]-
> >data[1])[i];
> +                        a = (x >> 24) & 0xff;
> +                        r = (x >> 16) & 0xff;
> +                        g = (x >>  8) & 0xff;
> +                        b = (x >>  0) & 0xff;
> +                    } else {
> +                        /* pad out the CLUT */
> +                        a = 0;
> +                        r = 0;
> +                        g = 0;
> +                        b = 0;
> +                    }
> 
>                      *q++ = RGB_TO_Y_CCIR(r, g, b);
>                      *q++ = RGB_TO_V_CCIR(r, g, b, 0);
> @@ -373,7 +401,7 @@ static int dvbsub_encode(AVCodecContext *avctx,
> uint8_t *outbuf, int buf_size,
>              }
> 
>              bytestream_put_be16(&pseg_len, q - pseg_len - 2);
> -            buf_size -= 6 + h->rects[clut_id]->nb_colors * 6;
> +            buf_size -= 6 + nb_colors * 6;
>          }
> 
>          if (buf_size < h->num_rects * 22)
> @@ -381,14 +409,17 @@ static int dvbsub_encode(AVCodecContext *avctx,
> uint8_t *outbuf, int buf_size,
>          for (region_id = 0; region_id < h->num_rects; region_id++) {
> 
>              /* region composition segment */
> +            int nb_colors = s->min_colors > h->rects[region_id]-
> >nb_colors
> +                            ? s->min_colors
> +                            : h->rects[region_id]->nb_colors;
> 
> -            if (h->rects[region_id]->nb_colors <= 4) {
> +            if (nb_colors <= 4) {
>                  /* 2 bpp, some decoders do not support it correctly */
>                  bpp_index = 0;
> -            } else if (h->rects[region_id]->nb_colors <= 16) {
> +            } else if (nb_colors <= 16) {
>                  /* 4 bpp, standard encoding */
>                  bpp_index = 1;
> -            } else if (h->rects[region_id]->nb_colors <= 256) {
> +            } else if (nb_colors <= 256) {
>                  /* 8 bpp, standard encoding */
>                  bpp_index = 2;
>              } else {
> @@ -424,17 +455,21 @@ static int dvbsub_encode(AVCodecContext *avctx,
> uint8_t *outbuf, int buf_size,
>                                    const uint8_t *bitmap, int linesize,
>                                    int w, int h);
> 
> +            int nb_colors = s->min_colors > h->rects[object_id]-
> >nb_colors
> +                                ? s->min_colors
> +                                : h->rects[object_id]->nb_colors;
> +
>              if (buf_size < 13)
>                  return AVERROR_BUFFER_TOO_SMALL;
> 
>              /* bpp_index maths */
> -            if (h->rects[object_id]->nb_colors <= 4) {
> +            if (nb_colors <= 4) {
>                  /* 2 bpp, some decoders do not support it correctly */
>                  dvb_encode_rle = dvb_encode_rle2;
> -            } else if (h->rects[object_id]->nb_colors <= 16) {
> +            } else if (nb_colors <= 16) {
>                  /* 4 bpp, standard encoding */
>                  dvb_encode_rle = dvb_encode_rle4;
> -            } else if (h->rects[object_id]->nb_colors <= 256) {
> +            } else if (nb_colors <= 256) {
>                  /* 8 bpp, standard encoding */
>                  dvb_encode_rle = dvb_encode_rle8;
>              } else {
> @@ -506,6 +541,20 @@ static int dvbsub_encode(AVCodecContext *avctx,
> uint8_t *outbuf, int buf_size,
>      return q - outbuf;
>  }
> 
> +#define OFFSET(x) offsetof(DVBSubtitleContext, x)
> +#define SE AV_OPT_FLAG_SUBTITLE_PARAM | AV_OPT_FLAG_ENCODING_PARAM
> +static const AVOption options[] = {
> +    {"disable_2bpp", "disable the 2bpp subtitle encoder",
> OFFSET(disable_2bpp), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, SE},
> +    { NULL },
> +};
> +
> +static const AVClass dvbsubenc_class = {
> +    .class_name = "DVBSUB subtitle encoder",
> +    .item_name  = av_default_item_name,
> +    .option     = options,
> +    .version    = LIBAVUTIL_VERSION_INT,
> +};
> +
>  const FFCodec ff_dvbsub_encoder = {
>      .p.name         = "dvbsub",
>      CODEC_LONG_NAME("DVB subtitles"),
> @@ -513,4 +562,6 @@ const FFCodec ff_dvbsub_encoder = {
>      .p.id           = AV_CODEC_ID_DVB_SUBTITLE,
>      .priv_data_size = sizeof(DVBSubtitleContext),
>      FF_CODEC_ENCODE_SUB_CB(dvbsub_encode),
> +    .init           = dvbsub_init,
> +    .p.priv_class   = &dvbsubenc_class,
>  };
> --


Hi Ronan,

thanks for your patch, it LGTM.

This was ringing some bell, I didn't immediately know what it was, but now I remember that we had encountered the 2bpp incompatibility with certain decoders when implementing the text2graphicsub filter (demo here: https://github.com/softworkz/SubtitleFilteringDemos/tree/master/Demo1), and so we chose to use a default of 16 for the output colors of the text2graphicsub filter to avoid that situation:

https://github.com/softworkz/FFmpeg/blob/bb1179897474e20966d5cb7724877ca651fe13d8/libavfilter/sf_text2graphicsub.c#L587-L588


In the light of these experiences, I would propose to go even one step further and make 4bpp the default minimum, either by setting the default value of disable_2bpp to 1 - or by renaming it to enable_2bpp with a default of 0 and a description like "2bpp is not supported by all decoders. By default, 4bpp is used as the minimum. Set this to enable 2pbb encoding."

Did you try running FATE while disable_2bpp is hardcoded to 1? If it causes any test failures, then the FATE reference results would need to be adjusted accordingly.

sw



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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [FFmpeg-devel] [PATCH v5] libavcodec/dvbsubenc.c: add a disable_2bpp option to work around some decoders.
  2025-03-02 15:00               ` Soft Works
@ 2025-03-02 17:23                 ` Ronan Waide
  2025-03-02 20:38                   ` Soft Works
  2025-03-02 17:26                 ` [FFmpeg-devel] [PATCH v4] " Ronan Waide
  1 sibling, 1 reply; 15+ messages in thread
From: Ronan Waide @ 2025-03-02 17:23 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Ronan Waide

As noted in the code in several places, some DVB subtitle decoders
don't handle 2bpp color. This patch adds a disable_2bpp option which
disables the 2bpp format; subtitles which would use 2bpp will be bumped
up to 4bpp. Per suggestion from sw, disable_2pp defaults to true.

Signed-off-by: Ronan Waide <waider@waider.ie>
---
 doc/encoders.texi      | 27 +++++++++++++++++
 libavcodec/dvbsubenc.c | 69 +++++++++++++++++++++++++++++++-----------
 2 files changed, 78 insertions(+), 18 deletions(-)

diff --git a/doc/encoders.texi b/doc/encoders.texi
index f3fcc1aa60..6ee0065c7d 100644
--- a/doc/encoders.texi
+++ b/doc/encoders.texi
@@ -4484,6 +4484,33 @@ Reduces detail but attempts to preserve color at extremely low bitrates.
 @chapter Subtitles Encoders
 @c man begin SUBTITLES ENCODERS
 
+@section dvbsub
+
+This codec encodes the bitmap subtitle format that is used in DVB
+broadcasts and recordings. The bitmaps are typically embedded in a
+container such as MPEG-TS as a separate stream.
+
+@subsection Options
+
+@table @option
+@item disable_2bpp @var{boolean}
+Disable the 2 bits-per-pixel encoding format.
+
+DVB supports 2, 4, and 8 bits-per-pixel color lookup tables. However,
+not all players support or properly support 2 bits-per-pixel,
+resulting in unusable subtitles.
+@table @option
+@item 0
+The 2 bits-per-pixel encoding format will be used for subtitles with 4
+colors or less.
+
+@item 1
+The 2 bits-per-pixel encoding format will be disabled, and subtitles
+with 4 colors or less will use a 4 bits-per-pixel format. (default)
+@end table
+
+@end table
+
 @section dvdsub
 
 This codec encodes the bitmap subtitle format that is used in DVDs.
diff --git a/libavcodec/dvbsubenc.c b/libavcodec/dvbsubenc.c
index 822e3a5309..4db2e1ddda 100644
--- a/libavcodec/dvbsubenc.c
+++ b/libavcodec/dvbsubenc.c
@@ -22,9 +22,12 @@
 #include "bytestream.h"
 #include "codec_internal.h"
 #include "libavutil/colorspace.h"
+#include "libavutil/opt.h"
 
 typedef struct DVBSubtitleContext {
+    AVClass * class;
     int object_version;
+    int disable_2bpp;
 } DVBSubtitleContext;
 
 #define PUTBITS2(val)\
@@ -274,13 +277,15 @@ static int dvbsub_encode(AVCodecContext *avctx, uint8_t *outbuf, int buf_size,
 {
     DVBSubtitleContext *s = avctx->priv_data;
     uint8_t *q, *pseg_len;
-    int page_id, region_id, clut_id, object_id, i, bpp_index, page_state;
+    int page_id, region_id, clut_id, object_id, i, bpp_index, page_state, min_colors;
 
 
     q = outbuf;
 
     page_id = 1;
 
+    min_colors = s->disable_2bpp ? 16 : 0;
+
     if (h->num_rects && !h->rects)
         return AVERROR(EINVAL);
 
@@ -326,18 +331,20 @@ static int dvbsub_encode(AVCodecContext *avctx, uint8_t *outbuf, int buf_size,
 
     if (h->num_rects) {
         for (clut_id = 0; clut_id < h->num_rects; clut_id++) {
-            if (buf_size < 6 + h->rects[clut_id]->nb_colors * 6)
+            int nb_colors = FFMAX(min_colors, h->rects[clut_id]->nb_colors);
+
+            if (buf_size < 6 + nb_colors * 6)
                 return AVERROR_BUFFER_TOO_SMALL;
 
             /* CLUT segment */
 
-            if (h->rects[clut_id]->nb_colors <= 4) {
+            if (nb_colors <= 4) {
                 /* 2 bpp, some decoders do not support it correctly */
                 bpp_index = 0;
-            } else if (h->rects[clut_id]->nb_colors <= 16) {
+            } else if (nb_colors <= 16) {
                 /* 4 bpp, standard encoding */
                 bpp_index = 1;
-            } else if (h->rects[clut_id]->nb_colors <= 256) {
+            } else if (nb_colors <= 256) {
                 /* 8 bpp, standard encoding */
                 bpp_index = 2;
             } else {
@@ -354,16 +361,24 @@ static int dvbsub_encode(AVCodecContext *avctx, uint8_t *outbuf, int buf_size,
             *q++ = clut_id;
             *q++ = (0 << 4) | 0xf; /* version = 0 */
 
-            for(i = 0; i < h->rects[clut_id]->nb_colors; i++) {
+            for(i = 0; i < nb_colors; i++) {
                 *q++ = i; /* clut_entry_id */
                 *q++ = (1 << (7 - bpp_index)) | (0xf << 1) | 1; /* 2 bits/pixel full range */
                 {
                     int a, r, g, b;
-                    uint32_t x= ((uint32_t*)h->rects[clut_id]->data[1])[i];
-                    a = (x >> 24) & 0xff;
-                    r = (x >> 16) & 0xff;
-                    g = (x >>  8) & 0xff;
-                    b = (x >>  0) & 0xff;
+                    if (i < h->rects[clut_id]->nb_colors) {
+                        uint32_t x= ((uint32_t*)h->rects[clut_id]->data[1])[i];
+                        a = (x >> 24) & 0xff;
+                        r = (x >> 16) & 0xff;
+                        g = (x >>  8) & 0xff;
+                        b = (x >>  0) & 0xff;
+                    } else {
+                        /* pad out the CLUT */
+                        a = 0;
+                        r = 0;
+                        g = 0;
+                        b = 0;
+                    }
 
                     *q++ = RGB_TO_Y_CCIR(r, g, b);
                     *q++ = RGB_TO_V_CCIR(r, g, b, 0);
@@ -373,7 +388,7 @@ static int dvbsub_encode(AVCodecContext *avctx, uint8_t *outbuf, int buf_size,
             }
 
             bytestream_put_be16(&pseg_len, q - pseg_len - 2);
-            buf_size -= 6 + h->rects[clut_id]->nb_colors * 6;
+            buf_size -= 6 + nb_colors * 6;
         }
 
         if (buf_size < h->num_rects * 22)
@@ -381,14 +396,15 @@ static int dvbsub_encode(AVCodecContext *avctx, uint8_t *outbuf, int buf_size,
         for (region_id = 0; region_id < h->num_rects; region_id++) {
 
             /* region composition segment */
+            int nb_colors = FFMAX(min_colors, h->rects[region_id]->nb_colors);
 
-            if (h->rects[region_id]->nb_colors <= 4) {
+            if (nb_colors <= 4) {
                 /* 2 bpp, some decoders do not support it correctly */
                 bpp_index = 0;
-            } else if (h->rects[region_id]->nb_colors <= 16) {
+            } else if (nb_colors <= 16) {
                 /* 4 bpp, standard encoding */
                 bpp_index = 1;
-            } else if (h->rects[region_id]->nb_colors <= 256) {
+            } else if (nb_colors <= 256) {
                 /* 8 bpp, standard encoding */
                 bpp_index = 2;
             } else {
@@ -424,17 +440,19 @@ static int dvbsub_encode(AVCodecContext *avctx, uint8_t *outbuf, int buf_size,
                                   const uint8_t *bitmap, int linesize,
                                   int w, int h);
 
+            int nb_colors = FFMAX(min_colors, h->rects[object_id]->nb_colors);
+
             if (buf_size < 13)
                 return AVERROR_BUFFER_TOO_SMALL;
 
             /* bpp_index maths */
-            if (h->rects[object_id]->nb_colors <= 4) {
+            if (nb_colors <= 4) {
                 /* 2 bpp, some decoders do not support it correctly */
                 dvb_encode_rle = dvb_encode_rle2;
-            } else if (h->rects[object_id]->nb_colors <= 16) {
+            } else if (nb_colors <= 16) {
                 /* 4 bpp, standard encoding */
                 dvb_encode_rle = dvb_encode_rle4;
-            } else if (h->rects[object_id]->nb_colors <= 256) {
+            } else if (nb_colors <= 256) {
                 /* 8 bpp, standard encoding */
                 dvb_encode_rle = dvb_encode_rle8;
             } else {
@@ -506,6 +524,20 @@ static int dvbsub_encode(AVCodecContext *avctx, uint8_t *outbuf, int buf_size,
     return q - outbuf;
 }
 
+#define OFFSET(x) offsetof(DVBSubtitleContext, x)
+#define SE AV_OPT_FLAG_SUBTITLE_PARAM | AV_OPT_FLAG_ENCODING_PARAM
+static const AVOption options[] = {
+    {"disable_2bpp", "disable the 2bpp subtitle encoder", OFFSET(disable_2bpp), AV_OPT_TYPE_BOOL, {.i64 = 1}, 0, 1, SE},
+    { NULL },
+};
+
+static const AVClass dvbsubenc_class = {
+    .class_name = "DVBSUB subtitle encoder",
+    .item_name  = av_default_item_name,
+    .option     = options,
+    .version    = LIBAVUTIL_VERSION_INT,
+};
+
 const FFCodec ff_dvbsub_encoder = {
     .p.name         = "dvbsub",
     CODEC_LONG_NAME("DVB subtitles"),
@@ -513,4 +545,5 @@ const FFCodec ff_dvbsub_encoder = {
     .p.id           = AV_CODEC_ID_DVB_SUBTITLE,
     .priv_data_size = sizeof(DVBSubtitleContext),
     FF_CODEC_ENCODE_SUB_CB(dvbsub_encode),
+    .p.priv_class   = &dvbsubenc_class,
 };
-- 
2.39.5 (Apple Git-154)

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [FFmpeg-devel] [PATCH v4] libavcodec/dvbsubenc.c: add a disable_2bpp option to work around some decoders.
  2025-03-02 15:00               ` Soft Works
  2025-03-02 17:23                 ` [FFmpeg-devel] [PATCH v5] " Ronan Waide
@ 2025-03-02 17:26                 ` Ronan Waide
  1 sibling, 0 replies; 15+ messages in thread
From: Ronan Waide @ 2025-03-02 17:26 UTC (permalink / raw)
  To: Soft Works; +Cc: FFmpeg development discussions and patches



> On 2 Mar 2025, at 15:00, Soft Works <softworkz@hotmail.com> wrote:
> 
> 
> 
>> -----Original Message-----
>> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Ronan
>> Waide
>> Sent: Samstag, 1. März 2025 16:32
>> To: ffmpeg-devel@ffmpeg.org
>> Cc: Ronan Waide <waider@waider.ie>
>> Subject: [FFmpeg-devel] [PATCH v4] libavcodec/dvbsubenc.c: add a
>> disable_2bpp option to work around some decoders.
>> 
>> As noted in the code in several places, some DVB subtitle decoders
>> don't handle 2bpp color. This patch adds a disable_2bpp option which
>> disables the 2bpp format; subtitles which would use 2bpp will be bumped
>> up to 4bpp.
>> 
>> Signed-off-by: Ronan Waide <waider@waider.ie>
>> ---
>> doc/encoders.texi      | 27 ++++++++++++++
>> libavcodec/dvbsubenc.c | 85 +++++++++++++++++++++++++++++++++---------
>> 2 files changed, 95 insertions(+), 17 deletions(-)
>> 
>> diff --git a/doc/encoders.texi b/doc/encoders.texi
>> index f3fcc1aa60..07984b6b54 100644
>> --- a/doc/encoders.texi
>> +++ b/doc/encoders.texi
>> @@ -4484,6 +4484,33 @@ Reduces detail but attempts to preserve color at
>> extremely low bitrates.
>> @chapter Subtitles Encoders
>> @c man begin SUBTITLES ENCODERS
>> 
>> +@section dvbsub
>> +
>> +This codec encodes the bitmap subtitle format that is used in DVB
>> +broadcasts and recordings. The bitmaps are typically embedded in a
>> +container such as MPEG-TS as a separate stream.
>> +
>> +@subsection Options
>> +
>> +@table @option
>> +@item disable_2bpp @var{boolean}
>> +Disable the 2 bits-per-pixel encoding format.
>> +
>> +DVB supports 2, 4, and 8 bits-per-pixel color lookup tables. However,
>> +not all players support or properly support 2 bits-per-pixel,
>> +resulting in unusable subtitles.
>> +@table @option
>> +@item 0
>> +The 2 bits-per-pixel encoding format will be used for subtitles with 4
>> +colors or less. (default)
>> +
>> +@item 1
>> +The 2 bits-per-pixel encoding format will be disabled, and subtitles
>> +with 4 colors or less will use a 4 bits-per-pixel format.
>> +@end table
>> +
>> +@end table
>> +
>> @section dvdsub
>> 
>> This codec encodes the bitmap subtitle format that is used in DVDs.
>> diff --git a/libavcodec/dvbsubenc.c b/libavcodec/dvbsubenc.c
>> index 822e3a5309..727a75e901 100644
>> --- a/libavcodec/dvbsubenc.c
>> +++ b/libavcodec/dvbsubenc.c
>> @@ -22,9 +22,13 @@
>> #include "bytestream.h"
>> #include "codec_internal.h"
>> #include "libavutil/colorspace.h"
>> +#include "libavutil/opt.h"
>> 
>> typedef struct DVBSubtitleContext {
>> +    AVClass * class;
>>     int object_version;
>> +    int disable_2bpp;
>> +    int min_colors;
>> } DVBSubtitleContext;
>> 
>> #define PUTBITS2(val)\
>> @@ -38,6 +42,18 @@ typedef struct DVBSubtitleContext {
>>     }\
>> }
>> 
>> +static int dvbsub_init(AVCodecContext *avctx)
>> +{
>> +    DVBSubtitleContext *s = avctx->priv_data;
>> +    if (s->disable_2bpp) {
>> +        av_log(avctx, AV_LOG_DEBUG, "Disabling 2bpp subtitles.\n");
>> +        s->min_colors = 16;
>> +    } else {
>> +        s->min_colors = 0;
>> +    }
>> +    return 0;
>> +}
>> +
>> static int dvb_encode_rle2(uint8_t **pq, int buf_size,
>>                            const uint8_t *bitmap, int linesize,
>>                            int w, int h)
>> @@ -326,18 +342,22 @@ static int dvbsub_encode(AVCodecContext *avctx,
>> uint8_t *outbuf, int buf_size,
>> 
>>     if (h->num_rects) {
>>         for (clut_id = 0; clut_id < h->num_rects; clut_id++) {
>> -            if (buf_size < 6 + h->rects[clut_id]->nb_colors * 6)
>> +            int nb_colors = s->min_colors > h->rects[clut_id]-
>>> nb_colors
>> +                            ? s->min_colors
>> +                            : h->rects[clut_id]->nb_colors;
>> +
>> +            if (buf_size < 6 + nb_colors * 6)
>>                 return AVERROR_BUFFER_TOO_SMALL;
>> 
>>             /* CLUT segment */
>> 
>> -            if (h->rects[clut_id]->nb_colors <= 4) {
>> +            if (nb_colors <= 4) {
>>                 /* 2 bpp, some decoders do not support it correctly */
>>                 bpp_index = 0;
>> -            } else if (h->rects[clut_id]->nb_colors <= 16) {
>> +            } else if (nb_colors <= 16) {
>>                 /* 4 bpp, standard encoding */
>>                 bpp_index = 1;
>> -            } else if (h->rects[clut_id]->nb_colors <= 256) {
>> +            } else if (nb_colors <= 256) {
>>                 /* 8 bpp, standard encoding */
>>                 bpp_index = 2;
>>             } else {
>> @@ -354,16 +374,24 @@ static int dvbsub_encode(AVCodecContext *avctx,
>> uint8_t *outbuf, int buf_size,
>>             *q++ = clut_id;
>>             *q++ = (0 << 4) | 0xf; /* version = 0 */
>> 
>> -            for(i = 0; i < h->rects[clut_id]->nb_colors; i++) {
>> +            for(i = 0; i < nb_colors; i++) {
>>                 *q++ = i; /* clut_entry_id */
>>                 *q++ = (1 << (7 - bpp_index)) | (0xf << 1) | 1; /* 2
>> bits/pixel full range */
>>                 {
>>                     int a, r, g, b;
>> -                    uint32_t x= ((uint32_t*)h->rects[clut_id]-
>>> data[1])[i];
>> -                    a = (x >> 24) & 0xff;
>> -                    r = (x >> 16) & 0xff;
>> -                    g = (x >>  8) & 0xff;
>> -                    b = (x >>  0) & 0xff;
>> +                    if (i < h->rects[clut_id]->nb_colors) {
>> +                        uint32_t x= ((uint32_t*)h->rects[clut_id]-
>>> data[1])[i];
>> +                        a = (x >> 24) & 0xff;
>> +                        r = (x >> 16) & 0xff;
>> +                        g = (x >>  8) & 0xff;
>> +                        b = (x >>  0) & 0xff;
>> +                    } else {
>> +                        /* pad out the CLUT */
>> +                        a = 0;
>> +                        r = 0;
>> +                        g = 0;
>> +                        b = 0;
>> +                    }
>> 
>>                     *q++ = RGB_TO_Y_CCIR(r, g, b);
>>                     *q++ = RGB_TO_V_CCIR(r, g, b, 0);
>> @@ -373,7 +401,7 @@ static int dvbsub_encode(AVCodecContext *avctx,
>> uint8_t *outbuf, int buf_size,
>>             }
>> 
>>             bytestream_put_be16(&pseg_len, q - pseg_len - 2);
>> -            buf_size -= 6 + h->rects[clut_id]->nb_colors * 6;
>> +            buf_size -= 6 + nb_colors * 6;
>>         }
>> 
>>         if (buf_size < h->num_rects * 22)
>> @@ -381,14 +409,17 @@ static int dvbsub_encode(AVCodecContext *avctx,
>> uint8_t *outbuf, int buf_size,
>>         for (region_id = 0; region_id < h->num_rects; region_id++) {
>> 
>>             /* region composition segment */
>> +            int nb_colors = s->min_colors > h->rects[region_id]-
>>> nb_colors
>> +                            ? s->min_colors
>> +                            : h->rects[region_id]->nb_colors;
>> 
>> -            if (h->rects[region_id]->nb_colors <= 4) {
>> +            if (nb_colors <= 4) {
>>                 /* 2 bpp, some decoders do not support it correctly */
>>                 bpp_index = 0;
>> -            } else if (h->rects[region_id]->nb_colors <= 16) {
>> +            } else if (nb_colors <= 16) {
>>                 /* 4 bpp, standard encoding */
>>                 bpp_index = 1;
>> -            } else if (h->rects[region_id]->nb_colors <= 256) {
>> +            } else if (nb_colors <= 256) {
>>                 /* 8 bpp, standard encoding */
>>                 bpp_index = 2;
>>             } else {
>> @@ -424,17 +455,21 @@ static int dvbsub_encode(AVCodecContext *avctx,
>> uint8_t *outbuf, int buf_size,
>>                                   const uint8_t *bitmap, int linesize,
>>                                   int w, int h);
>> 
>> +            int nb_colors = s->min_colors > h->rects[object_id]-
>>> nb_colors
>> +                                ? s->min_colors
>> +                                : h->rects[object_id]->nb_colors;
>> +
>>             if (buf_size < 13)
>>                 return AVERROR_BUFFER_TOO_SMALL;
>> 
>>             /* bpp_index maths */
>> -            if (h->rects[object_id]->nb_colors <= 4) {
>> +            if (nb_colors <= 4) {
>>                 /* 2 bpp, some decoders do not support it correctly */
>>                 dvb_encode_rle = dvb_encode_rle2;
>> -            } else if (h->rects[object_id]->nb_colors <= 16) {
>> +            } else if (nb_colors <= 16) {
>>                 /* 4 bpp, standard encoding */
>>                 dvb_encode_rle = dvb_encode_rle4;
>> -            } else if (h->rects[object_id]->nb_colors <= 256) {
>> +            } else if (nb_colors <= 256) {
>>                 /* 8 bpp, standard encoding */
>>                 dvb_encode_rle = dvb_encode_rle8;
>>             } else {
>> @@ -506,6 +541,20 @@ static int dvbsub_encode(AVCodecContext *avctx,
>> uint8_t *outbuf, int buf_size,
>>     return q - outbuf;
>> }
>> 
>> +#define OFFSET(x) offsetof(DVBSubtitleContext, x)
>> +#define SE AV_OPT_FLAG_SUBTITLE_PARAM | AV_OPT_FLAG_ENCODING_PARAM
>> +static const AVOption options[] = {
>> +    {"disable_2bpp", "disable the 2bpp subtitle encoder",
>> OFFSET(disable_2bpp), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, SE},
>> +    { NULL },
>> +};
>> +
>> +static const AVClass dvbsubenc_class = {
>> +    .class_name = "DVBSUB subtitle encoder",
>> +    .item_name  = av_default_item_name,
>> +    .option     = options,
>> +    .version    = LIBAVUTIL_VERSION_INT,
>> +};
>> +
>> const FFCodec ff_dvbsub_encoder = {
>>     .p.name         = "dvbsub",
>>     CODEC_LONG_NAME("DVB subtitles"),
>> @@ -513,4 +562,6 @@ const FFCodec ff_dvbsub_encoder = {
>>     .p.id           = AV_CODEC_ID_DVB_SUBTITLE,
>>     .priv_data_size = sizeof(DVBSubtitleContext),
>>     FF_CODEC_ENCODE_SUB_CB(dvbsub_encode),
>> +    .init           = dvbsub_init,
>> +    .p.priv_class   = &dvbsubenc_class,
>> };
>> --
> 
> 
> Hi Ronan,
> 
> thanks for your patch, it LGTM.
> 
> This was ringing some bell, I didn't immediately know what it was, but now I remember that we had encountered the 2bpp incompatibility with certain decoders when implementing the text2graphicsub filter (demo here: https://github.com/softworkz/SubtitleFilteringDemos/tree/master/Demo1), and so we chose to use a default of 16 for the output colors of the text2graphicsub filter to avoid that situation:
> 
> https://github.com/softworkz/FFmpeg/blob/bb1179897474e20966d5cb7724877ca651fe13d8/libavfilter/sf_text2graphicsub.c#L587-L588
> 
> 
> In the light of these experiences, I would propose to go even one step further and make 4bpp the default minimum, either by setting the default value of disable_2bpp to 1 - or by renaming it to enable_2bpp with a default of 0 and a description like "2bpp is not supported by all decoders. By default, 4bpp is used as the minimum. Set this to enable 2pbb encoding."
> 
> Did you try running FATE while disable_2bpp is hardcoded to 1? If it causes any test failures, then the FATE reference results would need to be adjusted accordingly.
> 
> sw
> 
> 

thanks for the feedback, Andreas & sw. V5 submitted with your suggestions included. I've run it against FATE and compared the output with previous versions and it all looks good.

Cheers,
Waider.
-- 
Ronan Waide
waider@waider.ie




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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [FFmpeg-devel] [PATCH v5] libavcodec/dvbsubenc.c: add a disable_2bpp option to work around some decoders.
  2025-03-02 17:23                 ` [FFmpeg-devel] [PATCH v5] " Ronan Waide
@ 2025-03-02 20:38                   ` Soft Works
  0 siblings, 0 replies; 15+ messages in thread
From: Soft Works @ 2025-03-02 20:38 UTC (permalink / raw)
  To: FFmpeg development discussions and patches; +Cc: Ronan Waide



> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Ronan
> Waide
> Sent: Sonntag, 2. März 2025 18:24
> To: ffmpeg-devel@ffmpeg.org
> Cc: Ronan Waide <waider@waider.ie>
> Subject: [FFmpeg-devel] [PATCH v5] libavcodec/dvbsubenc.c: add a
> disable_2bpp option to work around some decoders.
> 
> As noted in the code in several places, some DVB subtitle decoders
> don't handle 2bpp color. This patch adds a disable_2bpp option which
> disables the 2bpp format; subtitles which would use 2bpp will be bumped
> up to 4bpp. Per suggestion from sw, disable_2pp defaults to true.
> 
> Signed-off-by: Ronan Waide <waider@waider.ie>
> ---
>  doc/encoders.texi      | 27 +++++++++++++++++
>  libavcodec/dvbsubenc.c | 69 +++++++++++++++++++++++++++++++-----------
>  2 files changed, 78 insertions(+), 18 deletions(-)
> 
> diff --git a/doc/encoders.texi b/doc/encoders.texi
> index f3fcc1aa60..6ee0065c7d 100644
> --- a/doc/encoders.texi
> +++ b/doc/encoders.texi
> @@ -4484,6 +4484,33 @@ Reduces detail but attempts to preserve color at
> extremely low bitrates.
>  @chapter Subtitles Encoders
>  @c man begin SUBTITLES ENCODERS
> 
> +@section dvbsub
> +
> +This codec encodes the bitmap subtitle format that is used in DVB
> +broadcasts and recordings. The bitmaps are typically embedded in a
> +container such as MPEG-TS as a separate stream.
> +
> +@subsection Options
> +
> +@table @option
> +@item disable_2bpp @var{boolean}
> +Disable the 2 bits-per-pixel encoding format.
> +
> +DVB supports 2, 4, and 8 bits-per-pixel color lookup tables. However,
> +not all players support or properly support 2 bits-per-pixel,
> +resulting in unusable subtitles.
> +@table @option
> +@item 0
> +The 2 bits-per-pixel encoding format will be used for subtitles with 4
> +colors or less.
> +
> +@item 1
> +The 2 bits-per-pixel encoding format will be disabled, and subtitles
> +with 4 colors or less will use a 4 bits-per-pixel format. (default)
> +@end table
> +
> +@end table
> +
>  @section dvdsub
> 
>  This codec encodes the bitmap subtitle format that is used in DVDs.
> diff --git a/libavcodec/dvbsubenc.c b/libavcodec/dvbsubenc.c
> index 822e3a5309..4db2e1ddda 100644
> --- a/libavcodec/dvbsubenc.c
> +++ b/libavcodec/dvbsubenc.c
> @@ -22,9 +22,12 @@
>  #include "bytestream.h"
>  #include "codec_internal.h"
>  #include "libavutil/colorspace.h"
> +#include "libavutil/opt.h"
> 
>  typedef struct DVBSubtitleContext {
> +    AVClass * class;
>      int object_version;
> +    int disable_2bpp;
>  } DVBSubtitleContext;
> 
>  #define PUTBITS2(val)\
> @@ -274,13 +277,15 @@ static int dvbsub_encode(AVCodecContext *avctx,
> uint8_t *outbuf, int buf_size,
>  {
>      DVBSubtitleContext *s = avctx->priv_data;
>      uint8_t *q, *pseg_len;
> -    int page_id, region_id, clut_id, object_id, i, bpp_index,
> page_state;
> +    int page_id, region_id, clut_id, object_id, i, bpp_index,
> page_state, min_colors;
> 
> 
>      q = outbuf;
> 
>      page_id = 1;
> 
> +    min_colors = s->disable_2bpp ? 16 : 0;
> +
>      if (h->num_rects && !h->rects)
>          return AVERROR(EINVAL);
> 
> @@ -326,18 +331,20 @@ static int dvbsub_encode(AVCodecContext *avctx,
> uint8_t *outbuf, int buf_size,
> 
>      if (h->num_rects) {
>          for (clut_id = 0; clut_id < h->num_rects; clut_id++) {
> -            if (buf_size < 6 + h->rects[clut_id]->nb_colors * 6)
> +            int nb_colors = FFMAX(min_colors, h->rects[clut_id]-
> >nb_colors);
> +
> +            if (buf_size < 6 + nb_colors * 6)
>                  return AVERROR_BUFFER_TOO_SMALL;
> 
>              /* CLUT segment */
> 
> -            if (h->rects[clut_id]->nb_colors <= 4) {
> +            if (nb_colors <= 4) {
>                  /* 2 bpp, some decoders do not support it correctly */
>                  bpp_index = 0;
> -            } else if (h->rects[clut_id]->nb_colors <= 16) {
> +            } else if (nb_colors <= 16) {
>                  /* 4 bpp, standard encoding */
>                  bpp_index = 1;
> -            } else if (h->rects[clut_id]->nb_colors <= 256) {
> +            } else if (nb_colors <= 256) {
>                  /* 8 bpp, standard encoding */
>                  bpp_index = 2;
>              } else {
> @@ -354,16 +361,24 @@ static int dvbsub_encode(AVCodecContext *avctx,
> uint8_t *outbuf, int buf_size,
>              *q++ = clut_id;
>              *q++ = (0 << 4) | 0xf; /* version = 0 */
> 
> -            for(i = 0; i < h->rects[clut_id]->nb_colors; i++) {
> +            for(i = 0; i < nb_colors; i++) {
>                  *q++ = i; /* clut_entry_id */
>                  *q++ = (1 << (7 - bpp_index)) | (0xf << 1) | 1; /* 2
> bits/pixel full range */
>                  {
>                      int a, r, g, b;
> -                    uint32_t x= ((uint32_t*)h->rects[clut_id]-
> >data[1])[i];
> -                    a = (x >> 24) & 0xff;
> -                    r = (x >> 16) & 0xff;
> -                    g = (x >>  8) & 0xff;
> -                    b = (x >>  0) & 0xff;
> +                    if (i < h->rects[clut_id]->nb_colors) {
> +                        uint32_t x= ((uint32_t*)h->rects[clut_id]-
> >data[1])[i];
> +                        a = (x >> 24) & 0xff;
> +                        r = (x >> 16) & 0xff;
> +                        g = (x >>  8) & 0xff;
> +                        b = (x >>  0) & 0xff;
> +                    } else {
> +                        /* pad out the CLUT */
> +                        a = 0;
> +                        r = 0;
> +                        g = 0;
> +                        b = 0;
> +                    }
> 
>                      *q++ = RGB_TO_Y_CCIR(r, g, b);
>                      *q++ = RGB_TO_V_CCIR(r, g, b, 0);
> @@ -373,7 +388,7 @@ static int dvbsub_encode(AVCodecContext *avctx,
> uint8_t *outbuf, int buf_size,
>              }
> 
>              bytestream_put_be16(&pseg_len, q - pseg_len - 2);
> -            buf_size -= 6 + h->rects[clut_id]->nb_colors * 6;
> +            buf_size -= 6 + nb_colors * 6;
>          }
> 
>          if (buf_size < h->num_rects * 22)
> @@ -381,14 +396,15 @@ static int dvbsub_encode(AVCodecContext *avctx,
> uint8_t *outbuf, int buf_size,
>          for (region_id = 0; region_id < h->num_rects; region_id++) {
> 
>              /* region composition segment */
> +            int nb_colors = FFMAX(min_colors, h->rects[region_id]-
> >nb_colors);
> 
> -            if (h->rects[region_id]->nb_colors <= 4) {
> +            if (nb_colors <= 4) {
>                  /* 2 bpp, some decoders do not support it correctly */
>                  bpp_index = 0;
> -            } else if (h->rects[region_id]->nb_colors <= 16) {
> +            } else if (nb_colors <= 16) {
>                  /* 4 bpp, standard encoding */
>                  bpp_index = 1;
> -            } else if (h->rects[region_id]->nb_colors <= 256) {
> +            } else if (nb_colors <= 256) {
>                  /* 8 bpp, standard encoding */
>                  bpp_index = 2;
>              } else {
> @@ -424,17 +440,19 @@ static int dvbsub_encode(AVCodecContext *avctx,
> uint8_t *outbuf, int buf_size,
>                                    const uint8_t *bitmap, int linesize,
>                                    int w, int h);
> 
> +            int nb_colors = FFMAX(min_colors, h->rects[object_id]-
> >nb_colors);
> +
>              if (buf_size < 13)
>                  return AVERROR_BUFFER_TOO_SMALL;
> 
>              /* bpp_index maths */
> -            if (h->rects[object_id]->nb_colors <= 4) {
> +            if (nb_colors <= 4) {
>                  /* 2 bpp, some decoders do not support it correctly */
>                  dvb_encode_rle = dvb_encode_rle2;
> -            } else if (h->rects[object_id]->nb_colors <= 16) {
> +            } else if (nb_colors <= 16) {
>                  /* 4 bpp, standard encoding */
>                  dvb_encode_rle = dvb_encode_rle4;
> -            } else if (h->rects[object_id]->nb_colors <= 256) {
> +            } else if (nb_colors <= 256) {
>                  /* 8 bpp, standard encoding */
>                  dvb_encode_rle = dvb_encode_rle8;
>              } else {
> @@ -506,6 +524,20 @@ static int dvbsub_encode(AVCodecContext *avctx,
> uint8_t *outbuf, int buf_size,
>      return q - outbuf;
>  }
> 
> +#define OFFSET(x) offsetof(DVBSubtitleContext, x)
> +#define SE AV_OPT_FLAG_SUBTITLE_PARAM | AV_OPT_FLAG_ENCODING_PARAM
> +static const AVOption options[] = {
> +    {"disable_2bpp", "disable the 2bpp subtitle encoder",
> OFFSET(disable_2bpp), AV_OPT_TYPE_BOOL, {.i64 = 1}, 0, 1, SE},
> +    { NULL },
> +};
> +
> +static const AVClass dvbsubenc_class = {
> +    .class_name = "DVBSUB subtitle encoder",
> +    .item_name  = av_default_item_name,
> +    .option     = options,
> +    .version    = LIBAVUTIL_VERSION_INT,
> +};
> +
>  const FFCodec ff_dvbsub_encoder = {
>      .p.name         = "dvbsub",
>      CODEC_LONG_NAME("DVB subtitles"),
> @@ -513,4 +545,5 @@ const FFCodec ff_dvbsub_encoder = {
>      .p.id           = AV_CODEC_ID_DVB_SUBTITLE,
>      .priv_data_size = sizeof(DVBSubtitleContext),
>      FF_CODEC_ENCODE_SUB_CB(dvbsub_encode),
> +    .p.priv_class   = &dvbsubenc_class,
>  };
> --

Tested & LGTM

Thanks for the patch!

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2025-03-02 20:38 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-03-01 11:43 [FFmpeg-devel] [PATCH] dvbsubenc: add a disable_2bpp option to work around some decoders Waider
2025-03-01 11:48 ` Ronan Waide
2025-03-01 12:44   ` Ronan Waide
2025-03-01 12:55 ` Soft Works
2025-03-01 13:52   ` Ronan Waide
2025-03-01 14:10     ` [FFmpeg-devel] [PATCH v3] libavcodec/dvbsubenc.c: " Ronan Waide
2025-03-01 14:29       ` Ronan Waide
2025-03-01 15:00         ` Soft Works
2025-03-01 15:07           ` Ronan Waide
2025-03-01 15:32             ` [FFmpeg-devel] [PATCH v4] " Ronan Waide
2025-03-02 13:13               ` Andreas Rheinhardt
2025-03-02 15:00               ` Soft Works
2025-03-02 17:23                 ` [FFmpeg-devel] [PATCH v5] " Ronan Waide
2025-03-02 20:38                   ` Soft Works
2025-03-02 17:26                 ` [FFmpeg-devel] [PATCH v4] " Ronan Waide

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git