[FFmpeg-devel] [PATCH] Add support for Display Definition Segment to DVB Subtitle encoder

[Jernej]

To answer your questions...

1.) Yes, 2 is the version. To be honest I didn't really know what it meant.
I had a working sample stream & used the same values. But after reading the
spec I can see it's only used to signal changes in the display definition
segment which means it could be anything between 0 and 15 as it's going to
stay constant anyways. I can set it to 0 or 1 if you that's what you prefer.

2.) I saw that I could just pass avctx & read the priv_data inside
encode_dvb_subtitles function. I was just trying to make the least amount
of changes. But I'll change this and push a new patch tomorrow.

Regards,
Jernej

On Thu, Jul 11, 2019 at 10:01 PM Marton Balint <cus at passwd.hu> wrote:

>
>
> On Thu, 11 Jul 2019, mikrohard at gmail.com wrote:
>
> > Current version of dvbsub encoder doesn't support HD DVB subtitles. The
> high resolution bitmaps are muxed into the stream but without the DDS
> (display definition segment) the players asume that the DVB subtitles are
> in SD (720x576) resolution which causes them to either render the subtitles
> too large and misplaced or don't render them at all. By including the DDS
> as defined in section 7.7.1 of ETSI EN 300 743 (V1.3.1) this problem is
> fixed.
> >
> > 7.2.1 Display definition segment
> > The display definition for a subtitle service may be defined by the
> display definition segment if present in the stream.
> > Absence of a DDS implies that the stream is coded in accordance with EN
> 300 743 (V1.2.1) [5] and that a display width of 720 pixels and a display
> height of 576 lines may be assumed.
> >
> >
> https://www.etsi.org/deliver/etsi_en/300700_300799/300743/01.03.01_60/en_300743v010301p.pdf
> >
> > Signed-off-by: Jernej Fijacko <mikrohard at gmail.com>
> > ---
> > libavcodec/dvbsub.c | 18 ++++++++++++++++--
> > 1 file changed, 16 insertions(+), 2 deletions(-)
> >
> > diff --git a/libavcodec/dvbsub.c b/libavcodec/dvbsub.c
> > index 8cce702..c838567 100644
> > --- a/libavcodec/dvbsub.c
> > +++ b/libavcodec/dvbsub.c
> > @@ -247,7 +247,8 @@ static void dvb_encode_rle8(uint8_t **pq,
> >     *pq = q;
> > }
> >
> > -static int encode_dvb_subtitles(DVBSubtitleContext *s,
> > +static int encode_dvb_subtitles(AVCodecContext *avctx,
> > +                                DVBSubtitleContext *s,
> >                                 uint8_t *outbuf, const AVSubtitle *h)
> > {
> >     uint8_t *q, *pseg_len;
> > @@ -261,6 +262,19 @@ static int encode_dvb_subtitles(DVBSubtitleContext
> *s,
> >     if (h->num_rects && !h->rects)
> >         return -1;
> >
> > +    if (avctx->width > 0 && avctx->height > 0) {
> > +        /* display definition segment */
> > +        *q++ = 0x0f; /* sync_byte */
> > +        *q++ = 0x14; /* segment_type */
> > +        bytestream_put_be16(&q, page_id);
> > +        pseg_len = q;
> > +        q += 2; /* segment length */
> > +        *q++ = 0x20; /* dds version number & display window flag */
>
> I guess 2 is the version number, right? Why 2?
>
> > +        bytestream_put_be16(&q, avctx->width - 1); /* display width */
> > +        bytestream_put_be16(&q, avctx->height - 1); /* display height */
> > +        bytestream_put_be16(&pseg_len, q - pseg_len - 2);
> > +    }
> > +
> >     /* page composition segment */
> >
> >     *q++ = 0x0f; /* sync_byte */
> > @@ -449,7 +463,7 @@ static int dvbsub_encode(AVCodecContext *avctx,
> >     DVBSubtitleContext *s = avctx->priv_data;
> >     int ret;
> >
> > -    ret = encode_dvb_subtitles(s, buf, sub);
> > +    ret = encode_dvb_subtitles(avctx, s, buf, sub);
>
> You could pass avctx only and assign s the same way as it is assigned
> here...
>
> >     return ret;
>
> Otherwise looks good.
>
> Thanks,
> Marton
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".