[FFmpeg-devel] [PATCH] dvbsub fix transcoding

[Anshul]

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.

-Anshul


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