[FFmpeg-devel] [PATCH] DVB Subtitles Fix
JULIAN GARDNER
joolzg at btinternet.com
Fri May 6 16:46:38 CEST 2011
----- Original Message -----
> From: Ivan Kalvachev <ikalvachev at gmail.com>
> To: FFmpeg development discussions and patches <ffmpeg-devel at ffmpeg.org>
> Cc:
> Sent: Friday, 6 May 2011, 15:28
> Subject: Re: [FFmpeg-devel] [PATCH] DVB Subtitles Fix
>
> On 5/6/11, JULIAN GARDNER <joolzg at btinternet.com> wrote:
>>
>>
>>
>>> ________________________________
>>> From: Ivan Kalvachev <ikalvachev at gmail.com>
>>> To: FFmpeg development discussions and patches
> <ffmpeg-devel at ffmpeg.org>
>>> Sent: Friday, 6 May 2011, 12:56
>>> Subject: Re: [FFmpeg-devel] [PATCH] DVB Subtitles Fix
>>>
>>> On 5/5/11, JULIAN GARDNER <joolzg at btinternet.com> wrote:
>>>> ----- Original Message -----
>>>>> From: Ivan Kalvachev <ikalvachev at gmail.com>
>>>>> To: FFmpeg development discussions and patches
> <ffmpeg-devel at ffmpeg.org>
>>>>> Cc:
>>>>> Sent: Wednesday, 4 May 2011, 21:32
>>>>> Subject: Re: [FFmpeg-devel] [PATCH] DVB Subtitles Fix
>>>>>
>>>>> On 5/4/11, Michael Niedermayer <michaelni at gmx.at> wrote:
>>>>>> On Wed, May 04, 2011 at 07:18:12PM +0100, JULIAN GARDNER
> wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> ----- Original Message -----
>>>>>>> > From: Michael Niedermayer
> <michaelni at gmx.at>
>>>>>>> > To: FFmpeg development discussions and patches
>>>>> <ffmpeg-devel at ffmpeg.org>
>>>>>>> > Cc:
>>>>>>> > Sent: Wednesday, 4 May 2011, 18:25
>>>>>>> > Subject: Re: [FFmpeg-devel] [PATCH] DVB Subtitles
> Fix
>>>>>>> >
>>>>>>> > On Wed, May 04, 2011 at 04:11:33PM +0100, JULIAN
> GARDNER wrote:
>>>>>>> >>
>>>>>>> >>
>>>>>>> >>
>>>>>>> >>
>>>>>>> >> ----- Original Message -----
>>>>>>> >> > From: Michael Niedermayer
> <michaelni at gmx.at>
>>>>>>> >> > To: FFmpeg development discussions and
> patches
>>>>>>> > <ffmpeg-devel at ffmpeg.org>
>>>>>>> >> > Cc:
>>>>>>> >> > Sent: Wednesday, 4 May 2011, 15:14
>>>>>>> >> > Subject: Re: [FFmpeg-devel] [PATCH] DVB
> Subtitles Fix
>>>>>>> >> >
>>>>>>> >> > On Tue, Apr 26, 2011 at 11:34:10PM
> +0100, JULIAN GARDNER
>>>>> wrote:
>>>>>>> >> >> Ok first patch to fix the decoding
> and re-encoding
>>>>> of DVB
>>>>>>> > Subtitles.
>>>>>>> >> >>
>>>>>>> >> >> I went through and validated
> against the spec, my
>>>>> own engine and
>>>>>>> > fixed the
>>>>>>> >> > bits that were failing or wrong.
>>>>>>> >> >
>>>>>>> >> > please explain how to reproduce the
> bugs that your patch
>>>>> fixes
>>>>>>> >> >
>>>>>>> >> > Also please explain more elaborately
> what and why each
>>>>> change is
>>>>>>> >> made.
>>>>>>> >> >
>>>>>>> >>
>>>>>>> >> I use LIVE and RECORDED streams from various
> sources which i
>>>>> have
>>>>>>> >> collected
>>>>>>> > over the past years, these are put through ffmpeg
> and then the
>>>>> output
>>>>>>> > stream is
>>>>>>> > played out in VLC and a couple of my own
> receivers which are used
>>>>> in
>>>>>>> > professional cable networks.
>>>>>>> >
>>>>>>> > understood, but does this patch fix any problem
> with these ffmpeg
>>>>>>> > generated streams?
>>>>>>> > Iam asking because id like to reproduce the
> problem and the fix.
>>>>>>> >
>>>>>>> >
>>>>>>> it fixes all the problems with the encoded streams,
> also it allows
>>>>> FFMPEG
>>>>>>> to process a file correctly without bailing out with
> an error.
>>>>>>>
>>>>>>> I can upload a couple of streams if you want for you
> to see the
>>>>> problems.
>>>>>>
>>>>>> thanks, yes this would help me.
>>>>>>
>>>>>> if they are small you can upload to our issue tracker
>>>>>> http://ffmpeg.org/trac/ffmpeg
>>>>>>
>>>>>>
>>>>>>>
>>>>>>> >>
>>>>>>> >> >
>>>>>>> >> >>
>>>>>>> >> >> I have tested this on recorded
> streams and live
>>>>> streams and found
>>>>>>> > no
>>>>>>> >> > problems except for one which i will
> try and explain
>>>>> about.
>>>>>>> >> >>
>>>>>>> >> >> Here in the UK some of our
> channels use a live
>>>>> speech convertor
>>>>>>> > where you
>>>>>>> >> > see the words appear as they are
> spoken, now this works
>>>>> fine but the
>>>>>>> >>
>>>>>>> > way the
>>>>>>> >> > decode/encode works a lot more space is
> taken by this
>>>>> encoder for
>>>>>>> >> the
>>>>>>> > subsequent
>>>>>>> >> > encoded stream. In one test the
> incoming packets made up
>>>>> around 3k
>>>>>>> >> of
>>>>>>> > data, but
>>>>>>> >> > the amount of data in the output stream
> was around 15k.
>>>>>>> >> >>
>>>>>>> >> >> joolz
>>>>>>> >> >
>>>>>>> >> >> ffmpeg.c | 11 -
>>>>>>> >> >> libavcodec/dvbsub.c | 103
> ++++++++---------
>>>>>>> >> >> libavcodec/dvbsubdec.c | 296
>>>>>>> >> >
> +++++++++++++++++++++++++++++--------------------
>>>>>>> >> >> libavcodec/utils.c | 3
>>>>>>> >> >> 4 files changed, 233
> insertions(+), 180
>>>>> deletions(-)
>>>>>>> >> >>
> d22a3a04cf764a0b25138652ad82fe484311405f
>>>>> dvbsubs.diff
>>>>>>> >> >> diff --git a/ffmpeg.c b/ffmpeg.c
>>>>>>> >> >> index 2c69608..e88bcfc 100644
>>>>>>> >> >> --- a/ffmpeg.c
>>>>>>> >> >> +++ b/ffmpeg.c
>>>>>>> >> >> @@ -1092,15 +1092,8 @@ static void
>>>>>>> > do_subtitle_out(AVFormatContext *s,
>>>>>>> >> >> subtitle_out =
>>>>> av_malloc(subtitle_out_max_size);
>>>>>>> >> >> }
>>>>>>> >> >>
>>>>>>> >> >> - /* 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 ==
> CODEC_ID_DVB_SUBTITLE)
>>>>>>> >> >> - nb = 2;
>>>>>>> >> >> - else
>>>>>>> >> >> - nb = 1;
>>>>>>> >> >> -
>>>>>>> >> >> - for(i = 0; i < nb; i++) {
>>>>>>> >> >> + i = 0;
>>>>>>> >> >> + {
>>>>>>> >> >> sub->pts =
> av_rescale_q(pts,
>>>>>>> > ist->st->time_base,
>>>>>>> >> > AV_TIME_BASE_Q);
>>>>>>> >> >> // start_display_time is
> required to be 0
>>>>>>> >> >> sub->pts
> +=
>>>>>>> >> >
> av_rescale_q(sub->start_display_time, (AVRational){1,
>>>>> 1000},
>>>>>>> > AV_TIME_BASE_Q);
>>>>>>> >> >
>>>>>>> >> > why is this changed?
>>>>>>> >> >
>>>>>>> >>
>>>>>>> >> Because only one stream is needed to process
> DVB Subtitles, i
>>>>> could
>>>>>>> >> not
>>>>>>> > work out why you needed this. The NOTE is wrong
> regarding needed
>>>>> one to
>>>>>>> > draw and
>>>>>>> > one to remove
>>>>>>> >>
>>>>>>> >> What you will get from the DVB Stream is a
> packet with NO
>>>>> regions,
>>>>>>> >> this is
>>>>>>> > the one that removes the subtitles from the
> picture, as per the
>>>>> spec,
>>>>>>> > there is a
>>>>>>> > timeout value, which in the ffmpeg engine is
> always set to 30
>>>>> seconds,
>>>>>>> > this
>>>>>>> > would cause the receiving unit to switch off the
> display after 30
>>>>>>> > seconds of no
>>>>>>> > valid dvb stream.
>>>>>>> >
>>>>>>> > Lets assume something generates subtitles, lets
> say it generates
>>>>> one
>>>>>>> > displaying "hello world" at start time
> 0:10 and end time
>>>>> 0:50
>>>>>>> > If i understand you correctly this needs a packet
> muxed with time
>>>>> 0:10
>>>>>>> > and a empty clear packet muxed at time 0:50
>>>>>>> >
>>>>>>> > And this loop generates these 2 packets from 1
> subtitle with start
>>>>> and
>>>>>>> > end times.
>>>>>>> >
>>>>>>> > You remove it and i dont see how this could still
> work afterwards
>>>>>>> > maybe iam missing something ?
>>>>>>>
>>>>>>> The subtitles dont have an end time per se, what you
> have is a
>>>>>>> display
>>>>>>> time and a timeout value, this is incase of a stream
> problem or
>>>>> corruption
>>>>>
>>>>>>> for a missing packet.
>>>>>>>
>>>>>>> now what happens is in you example you would get a
> display packet
>>>>>>> with
>>>>>>> regions/objects/cluts etc and a timeout of 60, then
> around 50 seconds
>>>>> into
>>>>>>> the stream you would get a packet which has no
> objects/regions/cluts.
>>>>> This
>>>>>>> packet would turn the subs off, now if this packet was
> corrupted then
>>>>> the
>>>>>>> timeout would take care of the removal.
>>>>>>
>>>>>> I understand that, but this is only so when the source is
> dvb subs too
>>>>>> it can be something else.a different subtitle format for
> example.
>>>>>
>>>>> This nb variable doesn't have anything with the numbers of
> streams.
>>>>>
>>>>> It ensures that in case of DVB output it would write 2 packets.
>>>>> The first one with the subtitles and the second one with no
>>>>> regions/segments/cluts.
>>>>> It does call avcodec_encode_subtitles() twice, so it does get
>>>>> different packets (I hope).
>>>>> It is kind of running the whole function twice in a row.
>>>>>
>>>>> It seems that the dvb encoder have variable
> "hide_state" that always
>>>>> inverts itself when called, so every second encoded packet
> would be
>>>>> "hide" packet. And It seems that removing this
> variable is part of
>>>>> your patch.
>>>>>
>>>>> So I guess the question is. If the source (input) subtitles are
> of the
>>>>> type that have both valid start and end time, how do you output
> 2
>>>>> packets (one to show and one to hide) ?
>>>>> Well, I guess this is mostly problem for the time when we have
>>>>> ass/ssa->dvb rendering.
>>>>
>>>> i think your all missing the point, there is no such thing as an
> off in
>>>> dvb
>>>> subtitles. what you have is a set of display/regions/cluts and
> objects
>>>> that
>>>> make up a subtitle frame. If there are not object/cluts/regions
> then in
>>>> theory you have an off frame. it does not always work that you have
> a on
>>>> followed by an off. If you look at the bbc_small.ts i uploaded you
> can
>>>> see
>>>> that it draws 2 regions, 1 is a full width region with a single
> object,
>>>> the
>>>> 2nd line is made up of multiple objects as the new text is added.
>>>
>>> Yes. This is what we call subtitle hiding - displaying off frame.
>>>
>>> There are text subtitle formats like sami, that have only start time
>>> and no duration. You need to display an empty line if you want to hide
>>> the text from the previous line. While DVB subtitles allow more
>>> complicated rendering they work on same principle.
>>>
>>> I think this is also the analogy Michael had in mind.
>>>
>>>> So in this case you get 6 packets before we get a new region whch
> will
>>>> cause
>>>> a full redraw, no where in this stream is an "off"
> packet, so the
>>>> inverting
>>>> of the hide_state is NOT CORRECT and is not the correct way of
> doing dvb
>>>> subtitles.
>>>
>>> I don't think there is anybody who would disagree with you.
>>> The old method is hack, it seems to relay on the fact that the whole
>>> subtitle could be coded in a single packet. It should be replaced by
>>> proper solution. That's however a little tricky. (e.g. the encoder
> api
>>> can't output more than one packet at the moment).
>>>
>>>> Also you will have noticed that i made changes to how the resetting
> and
>>>> clearing of the cluts/objects etc are performed only as to only
> reset
>>>> when
>>>> the main display mode changes.
>>>>
>>>> ass/ssa -> dvb subtitles
>>>>
>>>> Well i did post a message 2 weeks ago asking about is, no replys,
> as to
>>>> if
>>>
>>> Do you refer to the "Hard subtitles" thread? IMHO it is a
> different issue.
>>>
>>>> there was some way to do what you mentioned ass/ssa to dvb
> subtitles. In
>>>> this case all that needs to be done is that when you get the off
> time
>>>> from
>>>> the input you send to the encoder a 0 region subtitle block, the
> dvb
>>>> subtitles encoder will then output an "off" packet.
>>>
>>> Yes, I think this is exactly what the problem is circling around.
>>> It could be done, but there is no code that does it at the moment.
>>>
>>> Now, the whole things is a little bit more complicated if you consider
>>> that the subtitle code should handle other types of subtitles than
>>> DVB.
>>> To illustrate the problems more clearly I'll be using the following
>>> convention.
>>> #1 type. In this mode the packet have only one timestamp, in
>>> AVSubtitle this would be start_display_time. In order to show subtitle
>>> line you need one AVSubtitle, and to hide it you need another. In
>>> short it's the DVB and Sami case.
>>> #2 type. In this mode there are two timestamps available. In
>>> AVSubtitle they would be start/end_display_time. This is the
>>> srt/ass/DVD(vobsub)/xsub etc...
>>>
>>> I would assume the case where we have input:output as #1#1 and #2#2
>>> to be trivial cases. (In your changes you seem to assume ffmpeg should
>>> work in #1#1 while at the moment it does work as #2#2).
>>>
>>> The most troublesome case is #1#2. We can't output encoded packet
>>> before we know the end time. This means we need to wait for a second
>>> input subtitle packet (or timeout), before we can output the first
>>> encoded. This introduces a lag, quite bit lag as subtitles have
>>> (relatively) huge durations. So the first output subtitle packet in
>>> normal case would appear about 2 seconds after the corresponding audio
>>> and video packets have been fed to the muxer. Have in mind that video
>>> packets also have their lag, but it is only 1 or 2 frames.
>>>
>>
>> Why do we need to wait for another packet? you have the timeout value, this
>
> In the sami case, It may not have any timeout value.
>
>> added to your start_time gives the end_time. Does this cause a problem is
>> another packet comes in with a start_time less than the last packets
>> end_time, for example
>
>> in #1 start_time 1.0s timeout 30.0s
>> out #2 start_time 1.0s end_time 31.0s
>> in #1 start_time 5.0s timeout 30.0s
>> out #2 start_time 5.0s end_time 35.0s
>
> First, page_time_out is not meant to be used as duration. It is in
> whole seconds and allows +5s delay before triggering.
>
> Second, at least with text subtitles there exists so called
> "overlapped subtitles". e.g.
> 0:0 -> 0:2 "I want to tell you...."
> 0:1 -> 0:3 "No, no, no."
> And for the time these subtitles time overlap (0:1-0:2) both would be visible.
>
> While for DVB design this is not issue (I think it supports native
> composition), it could be issue in the general case.
>
>> Now the other way you would subtract the two to get the timeout
>> in #2 start_time 1.0s end_time 2.3s
>> out #1 start_time 1.0s timeout 2.6s (double just for an extra bit of
>> space)in #2 start_time 5.0s end_time 7.3s
>> out #2 start_time 5.0s timeout 5.2s
>
> in #2 out #1 is described bellow as #2#1 case.
>
> What I have in mind as description for #1#2 working is:
>
> in #1 start_time=1.0
> in #1 start_time=5.0
> in #1 start_time=9.0
> out #2 start_time=1.0 end_time=5.0
> out #2 start_time=5.0 end_time=9.0
> ...
>
>
>
>>> The less troublesome case is #2#1. We have both times and the subtitle
>>
>>> encoder should output 2 packets. One for the start time with the text
>>> and one with the empty text/image at the end time.
>>>
>>> --
>>> Now, about your patch. If I understand it correctly assumes the #1#1
>>> for proper implementation.
>>>
>>> However if you insist that the decoders should always output #1 type
>>> subtitles, the #2 type encoder will be forced into the #1#2 case, even
>>> when the source subtitles are #2 type. I'd like to avoid that.
>>
>> Nope i dont assume this, this is how it works. You send a subtitle frame to
>> the encoder and you will get what you send, a packet with subtitles or an
>> empty packet.
>>
>>
>>>
>>> On the other hand, if we insist on all decoders always output #2 type
>>> subtitles, we are faced by the same lag as in #1#2 case, but this time
>>> in the decoder. It would happen even when we have #1#1 chain. Even
>>> worse, such lag would be quite troublesome for video players that use
>>> only the decoder. I'd say avoid at all cost.
>>>
>>> So there are 2 proper solutions. simple and complicated.
>>> The complicated one is implementing support for handling the #1#2 case
>>> lag. I don't think I can ask that from you.
>>
>> i dont see that problem occuring, the time lag wont appear, the #1 packet
>> comes in and we use the timeout, see above
>>
>>>
>>> The simple solution is to make at least the other 3 modes operational.
>>> Here are some steps that are needed.
>>>
>>> 1. Find a way to signal #1 or #2 type.
>>> Simple solution would be setting end_display_time=0. Right now the dvb
>>> decoder fills it with timeout value.
>>> Another way is to add new flag field to AVSubtitle. I recommend
>>> uint16_t value right after the "uint16_t format" as it may not
> even
>>> break ABI due to struct members alignment.
>>>
>>> 2. Make the dvb decoder output #1 type subtitles.
>>>
>>> 3. Make the dvb encoder output one additional packet with #2 type
> subtitle.
>>>
>>> 3.1. The dvb encoder should check if the AVSubtitle is #2 type, and if
>>> it is so, at the end output a packet with pts of end_display_time and
>>> empty subtitle. In essence instead of having "hide_state"
> iterate at
>>> every call, the variable should keep the #2 type flag and output empty
>>> packet if it is fed with empty subtitle.
>>>
>>> 3.2. The function do_subtitle_out() in ffmpeg.c should be fixed so it
>>> would call subtitle encoder once with the AVSubtitle packet and then
>>> loop at encoder (with empty sub) & writing packets until encoder
>>> stops producing data. A number of problems here. the
>>> avcodec_encode_subtitle() doesn't return packets, so the encoder
> can't
>>> adjust pts on its own. No "got_picture" variable either.
>>> So you may pick the easy road (just make it work) or go the highway
>>> (create avcodec_encode_subtitle2() that return packets. so the loops
>>> turns into
>>> for(;;){
>>> if (avcodec_subtitle_encode2(enc, &pkt, sub) <= 0 ) break;
>>> write_frame(s,&pkt, ...);
>>> }
>>>
>>> ---
>>>
>>> The proposed changes should probably be discussed with the other
>>> developers before agreeing on final implementation.
>>>
>>>
>>> Sorry for the long delay. I've never ever messed with the subtitles
>>> code before and I don't even know how to use it. So I needed some
> time
>>> to look up the internal working. And then I needed some more time to
>>> figure out the possible correct solutions.
>>>
>>> I would understand if you loose patience, but we are just trying to
>>> don't break one thing while fixing another.
>>>
>>> Your work is really appreciated and it is great that there is somebody
>>> interested in fixing code that haven't been touched for over 6
> years.
>>> Just don't give up (yet:)
>>>
>>> Best Regards.
>>> Ivan Kalvachev
>> Ok ill wait on your talks but in the meantime im moving on to finishing my
>> modifications, if they dont make it in then hey.
>>
>> But i need to start looking into adding "hard subs" so once im
> happy i have
>> everything done im moving onto the that.
>>
>> Once and you have decided on your direction of changes let me know and i
>> will try and help with the dvbsubs engine, either in code or knowledge of
>> how the system works.
>>
>> Its a pity that removing around 12 lines of code will at least allow
> dvbsubs
>> to be encoded and not cause a crash.
>>
>> Is there/has there been any work on getting something that would allow
>> transcoding of subtitles from differing formats yet, as this would be a
> good
>> way for me to try and see the problems you mention.
>
> By looking at the code it seems that dvdsub* and xsub* are both #2
> type. I would advice you to test dvd->dvb and dvb->dvd conversion, as
> we would expect it to be quite common.
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
so can you give me a command line for taking in a dvd file, VOB i take it, and using a subtitle stream with output to mpeg2 and dvb subtitles
ffmpeg -i xxx.vob <insert your stuff for taking dvd subtitle> -f mpegts <insert your stuff for encoding to dvb subtitles> -b 1000k -y a.ts
joolz
More information about the ffmpeg-devel
mailing list