[FFmpeg-devel] [PATCH] dvbsub fix transcoding

[Anshul]

On June 4, 2014 9:12:13 PM IST, Michael Niedermayer <michaelni at gmx.at> wrote:
>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.
>
If I got u correct then,
We need to check whether this subtitle is going to be encoded or not and set avoption for this.
This would be special case for dvb and add ugly hack like in encode_subtitle.
If people agree to add special case for dvb decoder in ffmpeg, I will do that way.
If we already have some flags for this situation then let me know.

Please take a look at my previous fix.  there no change is done in decoder but while encoding dvb decoded subtitle are treated as special case. If you find that worth then we can go with that.

>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 
>
>[...]
Agree with you. I thought empty frame is sent at last to flush remains.

-Anshul


-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.