[FFmpeg-devel] [PATCH] dvbsub fix transcoding

[Michael Niedermayer]

On Sun, Jun 01, 2014 at 10:44:48PM +0530, Anshul Maheshwari wrote:
> On Fri, May 23, 2014 at 3:50 PM, anshul <anshul.ffmpeg at gmail.com> wrote:
> > On 05/22/2014 06:43 PM, Wim Vander Schelden wrote:
> >>
> >> On Thu, May 22, 2014 at 3:01 PM, Wim Vander Schelden
> >> <lists at fixnum.org>wrote:
> >>
> >>> These subtitles not disappearing is exactly the issue that's introduced
> >>> by
> >>> sending the second (empty, "clearing") packet (by removing the nb = 2).
> >
> > you mean not sending second packet here.
> >>>
> >>> Note that you are now relying on the DVB subtitle timeout value, which
> is
> >>>
> >>> only for "safety" purposes (ie, the encoder crashed), and is only
> >>> required
> >>> by the specification to be followed with a 5 second accuracy (so it is
> >>> completely useless for functioning subtitles)!
> >>>
> > agree its for safety purpose, but as you see -0/+5 accuracy means that it
> > might be possible that subtitle
> > are on screen +5 second ie more then required time. so using timeout does
> > not have any harm.
> > though relying only on it, is harmful.
> > following is copy pasted from ETSI EN 300 743 V1.3.1 (2006-11)
> > page_time_out: The period, expressed in seconds, after which a page
> instance
> > is no longer valid and consequently shall
> > be erased from the screen, should it not have been redefined before that.
> > The time-out period starts when the page
> > instance is first displayed. The page_time_out value applies to each page
> > instance until its value is redefined. The
> > purpose of the time-out period is to avoid a page instance remaining on
> the
> > screen "for ever" if the IRD happens to have
> > missed the redefinition or deletion of the page instance. The time-out
> > period does not need to be counted very
> > accurately by the IRD: a reaction accuracy of -0/+5 s is accurate enough.
> >
> >> I seem to have misformulated my ordering issue in my earlier mail: the
> >> problem occurs when subtitle X only disappears after subtitle X+1 should
> >> start being displayed, as the "clear" packet for X is emitted immediately
> >> after the "show" packet for X, so before the "show" packet for X+1.
> >>
> > yes agree :)
> >
> >
> > So problem is
> > end_display_time is not reliable in case of dvbsub decoder,
> > since end_display_time in dvbsub decoder is calculated using
> page_time_out.
> > which is only for safety purpose.
> >
> > I am trying to fix end_display_time in dvbsub decoder.
> > I want to know should i cache the subtitle till i get clear_display
> > region/overlap region
> > or should i try to overwrite end_display_time when i have recieved some
> new
> > region.
> >
> > just now i have attached patch which stop second loop only when stream was
> > decoded by
> > dvbsubtile.
> >
> > -Anshul
> 
> Hi
> 
> I have attached new patch which fix end_display_time in subtitle.
> 
> > 7.2.6
> >  End of display set segment
> > The end_of_display_set_segment provides an explicit indication to the
> decoder that transmission of a display set is
> > complete. The end_of_display_set_segment shall be inserted into the
> stream immediately after the last
> > object_data_segment for each display set. It shall be present for each
> subtitle service in a subtitle stream,
> *although> decoders need not take advantage of this segment* and may apply
> other strategies to determine when they have sufficient
> > information from a display set to commence decoding.
> As spec say not to use end of display segment for any purpose. I removed
> use of eod segment
> 
> Thanks
> Anshul

>  dvbsubdec.c |  162 +++++++++++++++++++++++++++++-------------------------------
>  1 file changed, 79 insertions(+), 83 deletions(-)
> d7dc4775cac095f0f31a4879e763fd1dba365605  0001-fix-transcoding-dvbsub-to-dvbsub.patch
> From 35f48dd9d8929989b2ca130f72c66cef9a2f3a95 Mon Sep 17 00:00:00 2001
> From: Anshul Maheshwari <anshul.ffmpeg at gmail.com>
> Date: Sun, 1 Jun 2014 22:37:28 +0530
> Subject: [PATCH] fix transcoding dvbsub to dvbsub
> 
> fix ticket #2024
> ---
>  libavcodec/dvbsubdec.c | 162 ++++++++++++++++++++++++-------------------------
>  1 file changed, 79 insertions(+), 83 deletions(-)
> 
> diff --git a/libavcodec/dvbsubdec.c b/libavcodec/dvbsubdec.c
> index 9d74b1a..76ed389 100644
> --- a/libavcodec/dvbsubdec.c
> +++ b/libavcodec/dvbsubdec.c
> @@ -232,6 +232,7 @@ typedef struct DVBSubContext {
>  
>      int version;
>      int time_out;
> +    int64_t prev_start;
>      DVBSubRegion *region_list;
>      DVBSubCLUT   *clut_list;
>      DVBSubObject *object_list;
> @@ -1148,11 +1149,18 @@ static void dvbsub_parse_region_segment(AVCodecContext *avctx,
>  }
>  
>  static void dvbsub_parse_page_segment(AVCodecContext *avctx,
> -                                        const uint8_t *buf, int buf_size)
> +                                        const uint8_t *buf, int buf_size, AVSubtitle *sub)
>  {
>      DVBSubContext *ctx = avctx->priv_data;
>      DVBSubRegionDisplay *display;
>      DVBSubRegionDisplay *tmp_display_list, **tmp_ptr;
> +    DVBSubDisplayDefinition *display_def = ctx->display_definition;
> +    DVBSubRegion *region;
> +    AVSubtitleRect *rect;
> +    DVBSubCLUT *clut;
> +    uint32_t *clut_table;
> +    int i;
> +    int offset_x=0, offset_y=0;
>  
>      const uint8_t *buf_end = buf + buf_size;
>      int region_id;
> @@ -1167,12 +1175,80 @@ static void dvbsub_parse_page_segment(AVCodecContext *avctx,
>      version = ((*buf)>>4) & 15;
>      page_state = ((*buf++) >> 2) & 3;
>  
> -    if (ctx->version != version) {
> +    if (ctx->version == version) {
> +        return;
> +    }
>  
>      ctx->time_out = timeout;
>      ctx->version = version;
>  
>      av_dlog(avctx, "Page time out %ds, state %d\n", ctx->time_out, page_state);
> +    if (display_def) {
> +        offset_x = display_def->x;
> +        offset_y = display_def->y;
> +    }
> +    sub->num_rects = 0;
> +    for (display = ctx->display_list; display; display = display->next) {
> +        region = get_region(ctx, display->region_id);
> +        if (region && region->dirty)
> +            sub->num_rects++;
> +    }
> +
> +    if (sub->num_rects > 0) {
> +        if(ctx->prev_start)
> +            sub->end_display_time = av_rescale_q((sub->pts - ctx->prev_start )/90, AV_TIME_BASE_Q, avctx->time_base) - 1;

this delays the subtitle packets, they do have correct timestamps maybe
but each subtitle will be output only when the next subtitle becomes
available, which would require a player to buffer several seconds
of video to be able to combine subtitles and video.

also i think this patch adds several bugs beyond that.

if you want to delay packets till the next packet is received this
could be done more explicitly and also needs the very last packet that
has no subsequent packet to be handled. Also it has to be optional
as players will likely prefer to get the data immediately without
duration information 

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If you think the mosad wants you dead since a long time then you are either
wrong or dead since a long time.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140604/0598ad90/attachment.asc>