> On 8 Mar 2025, at 08:02, Ronan Waide <wai...@waider.ie> wrote:
>
>
>
>> On 2 Mar 2025, at 20:38, Soft Works <softworkz-at-hotmail....@ffmpeg.org>
>> wrote:
>>
>>
>>
>>> -----Original Message-----
>>> From: ffmpeg-devel <ffmpeg-devel-boun...@ffmpeg.org> On Behalf Of Ronan
>>> Waide
>>> Sent: Sonntag, 2. März 2025 18:24
>>> To: ffmpeg-devel@ffmpeg.org
>>> Cc: Ronan Waide <wai...@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 <wai...@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
> wai...@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
wai...@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-requ...@ffmpeg.org with subject "unsubscribe".
