[FFmpeg-devel] [PATCH] dvbsub fix transcoding

Michael Niedermayer michaelni at gmx.at
Thu Jun 5 01:46:22 CEST 2014


On Thu, Jun 05, 2014 at 12:02:44AM +0530, Anshul wrote:
> On June 4, 2014 11:08:37 PM IST, Michael Niedermayer <michaelni at gmx.at> wrote:
> >On Fri, May 23, 2014 at 03:50:05PM +0530, anshul 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
> >
> >>  ffmpeg.c            |    4 +++-
> >>  libavcodec/dvbsub.c |    2 +-
> >>  2 files changed, 4 insertions(+), 2 deletions(-)
> >> 5d9736343922cc0aad0fe3124943e5cf7406b2c6 
> >0001-dvbsub-fix-transcoding.patch
> >> From 946dc41a84a6b17d64445acb2424e18774e7f1ac Mon Sep 17 00:00:00
> >2001
> >> From: Anshul Maheshwari <er.anshul.maheshwari at gmail.com>
> >> Date: Fri, 23 May 2014 15:48:12 +0530
> >> Subject: [PATCH] dvbsub fix transcoding
> >> 
> >> Fix Ticket #2024
> >> ---
> >>  ffmpeg.c            | 4 +++-
> >>  libavcodec/dvbsub.c | 2 +-
> >>  2 files changed, 4 insertions(+), 2 deletions(-)
> >> 
> >> diff --git a/ffmpeg.c b/ffmpeg.c
> >> index 5299f0e..d9539ee 100644
> >> --- a/ffmpeg.c
> >> +++ b/ffmpeg.c
> >> @@ -770,6 +770,7 @@ static void do_subtitle_out(AVFormatContext *s,
> >>      int subtitle_out_max_size = 1024 * 1024;
> >>      int subtitle_out_size, nb, i;
> >>      AVCodecContext *enc;
> >> +    AVCodecContext *dec;
> >>      AVPacket pkt;
> >>      int64_t pts;
> >>  
> >> @@ -781,6 +782,7 @@ static void do_subtitle_out(AVFormatContext *s,
> >>      }
> >>  
> >>      enc = ost->st->codec;
> >> +    dec = ist->st->codec;
> >>  
> >>      if (!subtitle_out) {
> >>          subtitle_out = av_malloc(subtitle_out_max_size);
> >> @@ -789,7 +791,7 @@ static void do_subtitle_out(AVFormatContext *s,
> >>      /* 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 == AV_CODEC_ID_DVB_SUBTITLE)
> >> +    if (enc->codec_id == AV_CODEC_ID_DVB_SUBTITLE && dec->codec_id
> >!= AV_CODEC_ID_DVB_SUBTITLE)
> >
> >
> >>          nb = 2;
> >
> >this case (independant of this patch) needs some kind of que
> >(either inside the encoder or elsewhere) to ensure the clear packet
> >is correctly ordered in relation to the next "add" packet
> >there was a patch about this but i think no new version was posted
> >after reviews
> >
> >
> 
> Actually there was some disscussion going on for encode_subtitle2 api, I was waiting for that since here we need to give 2 packets or frame of subtitle out. One for start time another for end time. 
> 
> >>      else
> >>          nb = 1;
> >> diff --git a/libavcodec/dvbsub.c b/libavcodec/dvbsub.c
> >> index f6b46e6..f4b4efa 100644
> >> --- a/libavcodec/dvbsub.c
> >> +++ b/libavcodec/dvbsub.c
> >> @@ -268,7 +268,7 @@ static int
> >encode_dvb_subtitles(DVBSubtitleContext *s,
> >>      bytestream_put_be16(&q, page_id);
> >>      pseg_len = q;
> >>      q += 2; /* segment length */
> >> -    *q++ = 30; /* page_timeout (seconds) */
> >> +    *q++ = ceil(h->end_display_time /1000); /* page_timeout
> >(seconds) */
> >
> >this would need a check for the value fitting in a byte (0..255)
> >
> 
> I have not seen more then 30 here,but I haven't seen all videos yet. :) 
> So if values more then 255 comes should I put 255 there. 
> 
> >also this only sort of fixes/hacks around the dvb->dvb case, there
> >still wont be correctly set durations for dvb->something else
> >I think, unless iam missing something
> >
> If you are talking about whole patch
> This fix if end time is not reliable, thats the case with dvb decoder. 
> Since nb=2 is working on basis of end time only.
> So it works for dvb-> dvb and reliable_end_time-> dvb
> 
> 
> Some thing I would like to ask that, should I work on the inputs for second patch or first one.

I think the durations need to be optionally calculated
this patch here still wont give correct dvb->dvb as is because the
end times currently arent correctm they are just the timeout values

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Concerning the gods, I have no means of knowing whether they exist or not
or of what sort they may be, because of the obscurity of the subject, and
the brevity of human life -- Protagoras
-------------- 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/20140605/24789ba6/attachment.asc>


More information about the ffmpeg-devel mailing list