> On 14 Apr 2025, at 16:53, softworkz . <softworkz-at-hotmail....@ffmpeg.org>
> wrote:
>
>
>
>> -----Original Message-----
>> From: ffmpeg-devel <ffmpeg-devel-boun...@ffmpeg.org> On Behalf Of
>> Ronan Waide
>> Sent: Sonntag, 30. März 2025 12:18
>> To: FFmpeg development discussions and patches <ffmpeg-
>> de...@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 <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
>
>
> 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.
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".
