[FFmpeg-devel] [PATCH] DVB Subtitles Fix
Michael Niedermayer
michaelni at gmx.at
Wed May 4 19:25:47 CEST 2011
On Wed, May 04, 2011 at 04:11:33PM +0100, JULIAN GARDNER wrote:
>
>
>
>
> ----- Original Message -----
> > From: Michael Niedermayer <michaelni at gmx.at>
> > To: FFmpeg development discussions and patches <ffmpeg-devel at ffmpeg.org>
> > Cc:
> > Sent: Wednesday, 4 May 2011, 15:14
> > Subject: Re: [FFmpeg-devel] [PATCH] DVB Subtitles Fix
> >
> > On Tue, Apr 26, 2011 at 11:34:10PM +0100, JULIAN GARDNER wrote:
> >> Ok first patch to fix the decoding and re-encoding of DVB Subtitles.
> >>
> >> I went through and validated against the spec, my own engine and fixed the
> > bits that were failing or wrong.
> >
> > please explain how to reproduce the bugs that your patch fixes
> >
> > Also please explain more elaborately what and why each change is made.
> >
>
> I use LIVE and RECORDED streams from various sources which i have collected over the past years, these are put through ffmpeg and then the output stream is played out in VLC and a couple of my own receivers which are used in professional cable networks.
understood, but does this patch fix any problem with these ffmpeg
generated streams?
Iam asking because id like to reproduce the problem and the fix.
>
> >
> >>
> >> I have tested this on recorded streams and live streams and found no
> > problems except for one which i will try and explain about.
> >>
> >> Here in the UK some of our channels use a live speech convertor where you
> > see the words appear as they are spoken, now this works fine but the way the
> > decode/encode works a lot more space is taken by this encoder for the subsequent
> > encoded stream. In one test the incoming packets made up around 3k of data, but
> > the amount of data in the output stream was around 15k.
> >>
> >> joolz
> >
> >> ffmpeg.c | 11 -
> >> libavcodec/dvbsub.c | 103 ++++++++---------
> >> libavcodec/dvbsubdec.c | 296
> > +++++++++++++++++++++++++++++--------------------
> >> libavcodec/utils.c | 3
> >> 4 files changed, 233 insertions(+), 180 deletions(-)
> >> d22a3a04cf764a0b25138652ad82fe484311405f dvbsubs.diff
> >> diff --git a/ffmpeg.c b/ffmpeg.c
> >> index 2c69608..e88bcfc 100644
> >> --- a/ffmpeg.c
> >> +++ b/ffmpeg.c
> >> @@ -1092,15 +1092,8 @@ static void do_subtitle_out(AVFormatContext *s,
> >> subtitle_out = av_malloc(subtitle_out_max_size);
> >> }
> >>
> >> - /* Note: DVB subtitle need one packet to draw them and one other
> >> - packet to clear them */
> >> - /* XXX: signal it in the codec context ? */
> >> - if (enc->codec_id == CODEC_ID_DVB_SUBTITLE)
> >> - nb = 2;
> >> - else
> >> - nb = 1;
> >> -
> >> - for(i = 0; i < nb; i++) {
> >> + i = 0;
> >> + {
> >> sub->pts = av_rescale_q(pts, ist->st->time_base,
> > AV_TIME_BASE_Q);
> >> // start_display_time is required to be 0
> >> sub->pts +=
> > av_rescale_q(sub->start_display_time, (AVRational){1, 1000}, AV_TIME_BASE_Q);
> >
> > why is this changed?
> >
>
> Because only one stream is needed to process DVB Subtitles, i could not work out why you needed this. The NOTE is wrong regarding needed one to draw and one to remove
>
> What you will get from the DVB Stream is a packet with NO regions, this is the one that removes the subtitles from the picture, as per the spec, there is a timeout value, which in the ffmpeg engine is always set to 30 seconds, this would cause the receiving unit to switch off the display after 30 seconds of no valid dvb stream.
Lets assume something generates subtitles, lets say it generates one
displaying "hello world" at start time 0:10 and end time 0:50
If i understand you correctly this needs a packet muxed with time 0:10
and a empty clear packet muxed at time 0:50
And this loop generates these 2 packets from 1 subtitle with start and
end times.
You remove it and i dont see how this could still work afterwards
maybe iam missing something ?
[...]
> >
> >> @@ -387,7 +383,7 @@ static int encode_dvb_subtitles(DVBSubtitleContext *s,
> >> *q++ = 0xff; /* end of PES data */
> >>
> >> s->object_version = (s->object_version + 1) & 0xf;
> >> - s->hide_state = !s->hide_state;
> >> +
> >> return q - outbuf;
> >> }
> >>
> >> @@ -399,6 +395,11 @@ static int dvbsub_encode(AVCodecContext *avctx,
> >> int ret;
> >>
> >> ret = encode_dvb_subtitles(s, buf, sub);
> >> +
> >> +#ifdef DEBUG
> >> + av_log( avctx, AV_LOG_INFO, "dvbsub_encode %d
> > bytes\r\n", ret);
> >> +#endif
> >> +
> >> return ret;
> >> }
> >>
> >> diff --git a/libavcodec/dvbsubdec.c b/libavcodec/dvbsubdec.c
> >> index 288e6f5..d9c396a 100644
> >> --- a/libavcodec/dvbsubdec.c
> >> +++ b/libavcodec/dvbsubdec.c
> >> @@ -157,6 +157,7 @@ static void png_save2(const char *filename, uint32_t
> > *bitmap, int w, int h)
> >>
> >> typedef struct DVBSubCLUT {
> >> int id;
> >> + int version;
> >>
> >> uint32_t clut4[4];
> >> uint32_t clut16[16];
> >> @@ -183,6 +184,7 @@ typedef struct DVBSubObjectDisplay {
> >>
> >> typedef struct DVBSubObject {
> >> int id;
> >> + int version;
> >>
> >> int type;
> >>
> >> @@ -202,6 +204,7 @@ typedef struct DVBSubRegionDisplay {
> >>
> >> typedef struct DVBSubRegion {
> >> int id;
> >> + int version;
> >>
> >> int width;
> >> int height;
> >> @@ -212,6 +215,7 @@ typedef struct DVBSubRegion {
> >>
> >> uint8_t *pbuf;
> >> int buf_size;
> >> + int dirty;
> >>
> >> DVBSubObjectDisplay *display_list;
> >>
> >> @@ -231,6 +235,7 @@ typedef struct DVBSubContext {
> >> int composition_id;
> >> int ancillary_id;
> >>
> >> + int version;
> >> int time_out;
> >> DVBSubRegion *region_list;
> >> DVBSubCLUT *clut_list;
> >
> >
> >> @@ -321,21 +326,10 @@ static void delete_region_display_list(DVBSubContext
> > *ctx, DVBSubRegion *region)
> >>
> >> }
> >>
> >> -static void delete_state(DVBSubContext *ctx)
> >> +static void delete_cluts(DVBSubContext *ctx)
> >> {
> >> - DVBSubRegion *region;
> >> DVBSubCLUT *clut;
> >>
> >> - while (ctx->region_list) {
> >> - region = ctx->region_list;
> >> -
> >> - ctx->region_list = region->next;
> >> -
> >> - delete_region_display_list(ctx, region);
> >> - av_free(region->pbuf);
> >> - av_free(region);
> >> - }
> >> -
> >> while (ctx->clut_list) {
> >> clut = ctx->clut_list;
> >>
> >> @@ -343,12 +337,35 @@ static void delete_state(DVBSubContext *ctx)
> >>
> >> av_free(clut);
> >> }
> >> +}
> >>
> >> - av_freep(&ctx->display_definition);
> >> +static void delete_objects(DVBSubContext *ctx)
> >> +{
> >> + DVBSubObject *object;
> >> +
> >> + while (ctx->object_list) {
> >> + object = ctx->object_list;
> >> +
> >> + ctx->object_list = object->next;
> >> +
> >> + av_free(object);
> >> + }
> >> +}
> >>
> >> - /* Should already be null */
> >> - if (ctx->object_list)
> >> - av_log(0, AV_LOG_ERROR, "Memory deallocation
> > error!\n");
> >> +static void delete_regions(DVBSubContext *ctx)
> >> +{
> >> + DVBSubRegion *region;
> >> +
> >> + while (ctx->region_list) {
> >> + region = ctx->region_list;
> >> +
> >> + ctx->region_list = region->next;
> >> +
> >> + delete_region_display_list(ctx, region);
> >> +
> >> + av_free(region->pbuf);
> >> + av_free(region);
> >> + }
> >> }
> >>
> >> static av_cold int dvbsub_init_decoder(AVCodecContext *avctx)
> >> @@ -365,6 +382,8 @@ static av_cold int dvbsub_init_decoder(AVCodecContext
> > *avctx)
> >> ctx->ancillary_id = AV_RB16(avctx->extradata + 2);
> >> }
> >>
> >> + ctx->version = -1;
> >> +
> >> default_clut.id = -1;
> >> default_clut.next = NULL;
> >>
> >> @@ -433,7 +452,13 @@ static av_cold int dvbsub_close_decoder(AVCodecContext
> > *avctx)
> >> DVBSubContext *ctx = avctx->priv_data;
> >> DVBSubRegionDisplay *display;
> >>
> >> - delete_state(ctx);
> >> + delete_regions(ctx);
> >> +
> >> + delete_objects(ctx);
> >> +
> >> + delete_cluts(ctx);
> >> +
> >> + av_freep(&ctx->display_definition);
> >>
> >> while (ctx->display_list) {
> >> display = ctx->display_list;
> >
> > why is this changeset done?
> > this looks like just reformating code and then adding delete_objects()
> > later can be done without the reformating
>
> Because we need to be able to delete certain things depending on its version, i still have some changes to make as in, what happens when an objects version number changes, only the object needs deleting the same as with cluts.
ok
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Breaking DRM is a little like attempting to break through a door even
though the window is wide open and the only thing in the house is a bunch
of things you dont want and which you would get tomorrow for free anyway
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20110504/1d2ef1ae/attachment.asc>
More information about the ffmpeg-devel
mailing list