[FFmpeg-devel] [PATCH] dvbsub fix transcoding
Anshul
anshul.ffmpeg at gmail.com
Wed Jun 4 21:03:37 CEST 2014
On June 4, 2014 10:56:02 PM IST, Michael Niedermayer <michaelni at gmx.at> wrote:
>On Wed, Jun 04, 2014 at 10:44:25PM +0530, Anshul wrote:
>> 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.
>
>yes thats a possibility
>
>
>> This would be special case for dvb and add ugly hack like in
>encode_subtitle.
>
>yes, or it could be moved elsewhere, iam not sure which is the best
>place. But its a ugly hack no matter where its done
>If dvb is the only that needs this then it could be done inside the
>dvb decoder
>
>
If it could be done inside dvb decoder it would be best, I dont find any parameter which tells which encoder is used in decoder.
I dont know where we use AVIO_FLAG_NONBLOCK . in this situation this flag suits my need.
Should I go with this flag?
>> 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.
>
>i will take a look
>
>
Thanks
>>
>> >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.
>
>maybe it works, i ve not tested, just felt it might fail
>
>
>[...]
-Anshul
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
More information about the ffmpeg-devel
mailing list