Re: [FFmpeg-devel] [PATCH v5] libavcodec/dvbsubenc.c: add a disable_2bpp option to work around some decoders.

[Ronan Waide <waider@waider.ie>]

> On 26 May 2025, at 19:41, Ronan Waide <waider@waider.ie> wrote:
> 
>> 
>> On 14 Apr 2025, at 16:53, softworkz . <softworkz-at-hotmail.com@ffmpeg.org> wrote:
>> 
>> 
>> 
>>> -----Original Message-----
>>> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
>>> Ronan Waide
>>> Sent: Sonntag, 30. März 2025 12:18
>>> To: FFmpeg development discussions and patches <ffmpeg-
>>> devel@ffmpeg.org>
>>> Subject: Re: [FFmpeg-devel] [PATCH v5] libavcodec/dvbsubenc.c: add a
>>> disable_2bpp option to work around some decoders.
>>> 
>>> 
>>> 
>>>> On 8 Mar 2025, at 08:02, Ronan Waide <waider@waider.ie> wrote:
>>>> 
>>>> 
>>>> 
>>>>> On 2 Mar 2025, at 20:38, 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: 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!
>>>> 
>>>> Hello,
>>>> 
>>>> nudging on this - does it need any more work, or will it be merged?
>>>> 
>>>> Cheers,
>>>> Waider.
>>>> --
>>>> Ronan Waide
>>>> waider@waider.ie
>>>> 
>>>> 
>>>> 
>>>> 
>>> 
>>> Hello again,
>>> 
>>> this still applies cleanly to the current HEAD, i.e. no conflicts on
>>> rebase. Would it be possible to merge it, or is there additional work
>>> required?
>>> 
>>> cheers,
>>> Waider.
>>> --
>>> Ronan Waide
>>> waider@waider.ie
>> 
>> 
>> Hello Ronan,
>> 
>> I'll try to pick this up next or 2nd-next week. Patch looked good 
>> but I want to test it locally before making a last call and applying
>> it - assuming all goes well.
> 
> nudge on this. The current patch applies to the current HEAD without changes, so I've not posted a revision.

nudging on this again. Patch continues to apply cleanly, but I can post a rebased version if required.

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