[FFmpeg-devel] [PATCH 2/4] add ASS parser

Michael Niedermayer michaelni
Sun Jul 11 01:50:58 CEST 2010


On Sat, Jul 10, 2010 at 03:04:23PM +0200, Aurelien Jacobs wrote:
> On Fri, Jul 09, 2010 at 04:03:05PM +0200, Michael Niedermayer wrote:
> > On Thu, Jul 08, 2010 at 10:06:14AM +0200, Aurelien Jacobs wrote:
> > > On Thu, Jul 08, 2010 at 12:46:17AM +0200, Michael Niedermayer wrote:
> > > > On Thu, Jul 08, 2010 at 12:22:33AM +0200, Aurelien Jacobs wrote:
> > > > > On Wed, Jul 07, 2010 at 09:13:38PM +0200, Michael Niedermayer wrote:
> > > > > > On Tue, Jul 06, 2010 at 10:54:54PM +0200, Aurelien Jacobs wrote:
> > > > > > [...]
> > > > > > > -static int read_packet(AVFormatContext *s, AVPacket *pkt)
> > > > > > > +static int ass_parse(AVCodecParserContext *s, AVCodecContext *avctx,
> > > > > > > +                     const uint8_t **poutbuf, int *poutbuf_size,
> > > > > > > +                     const uint8_t *buf, int buf_size)
> > > > > > >  {
> > > > > > >      ASSContext *ass = s->priv_data;
> > > > > > > +    if (!ass->event_buffer)
> > > > > > > +        ass_read_header(s, avctx, buf, buf_size);
> > > > > > >  
> > > > > > > +    if (ass->event_index >= ass->event_count) {
> > > > > > > +        *poutbuf = NULL;
> > > > > > > +        *poutbuf_size = 0;
> > > > > > > +        return buf_size;
> > > > > > > +    }
> > > > > > >  
> > > > > > > +    *poutbuf = ass->event[ass->event_index++];
> > > > > > > +    *poutbuf_size = strlen(*poutbuf);
> > > > > > > +    s->pts = s->dts = get_pts(*poutbuf);
> > > > > > > +    if (ass->event_index >= ass->event_count)
> > > > > > > +        return buf_size;
> > > > > > > +    return *poutbuf_size;
> > > > > > >  }
> > > > > > 
> > > > > > i dont see how this would work with seeking?
> > > > > 
> > > > > I'm not sure what you have in mind. Actually, I don't really know how a
> > > > > parser can handle seeking... And especially the ASS parser.
> > > > > 
> > > > > Just for the record, the ASS parser currently require to be feeded with
> > > > > the *whole* ASS file/stream before it can start outputing the first
> > > > > packet. This is due to this stupidity in the ASS spec allowing events to
> > > > > be stored out of order.
> > > > > 
> > > > > The only alternative I could see to this, would be to do the reordering
> > > > > at a higher level, in av_read_frame() for example. This would also allow
> > > > > to properly interleave the ASS packets with the other streams, but this
> > > > > seems slightly hackish...
> > > > 
> > > > what is the problem with reordering in the demuxer, as done currently?
> > > 
> > > The AVI demuxer don't do reordering currently... It only contains one
> > > blob that contains some subtitles, that it passes as a single AVPacket
> > > to the parser. The AVI demuxer don't even know if the subtitle packet
> > > contains ASS or SRT...
> > 
> > the avi demuxer can just call probe() of ass/srt, it can just call the
> > ass demuxer. One day demuxer chaining could be used of course ...
> > 
> > the design you suggest isnt scaleable as for every seek all subtitles
> > will always be passed through the whole chain.
> > that is seeking would be O(n), this may be ok for desktops or normal
> > 2 hour movies but reduce cpu capability and increase file length and
> > you hit a point where this breaks down, and i dont think a design
> > like this is good even if it wont affect many real world cases.
> 
> Hum... Well...
> So basically you're suggesting that I go back to something similar to
> what I proposed almost a year ago [1], and that I forget about your
> AVParser usage suggestion ? Is that right ?
> If so, I will try to rework this patchset in this direction...

oops, iam sorry for having pushed into this direction, i honestly
thought it would be alot simpler and require few if any changes to
the (in svn) code. I was unaware  back then that subtitle packets could
be randomly ordered in a single avi chunk.
What i was thinking of back then was probably something along the lines
of mpeg-es in mpeg-ps or audio in avi, that one simply passes to a generic
spliting parser that simply splits it.

Though to my defense, my excuse, i said:
------
> > i wouldnt mind havng ass split in the demuxer if it could be done naturally
>
> What do you mean ? Should the ass spliting be implemented as a parser instead
> of a demuxer ? And assdec.c would just set need_parsing and do nothing else ?

iam not sure ... maybe, maybe not


> Is it OK for a parser to receive a packet that needs to be split accross
> several hours ?

if it works and the alternative if too complex i dont see a problem
------

but considering the whole, yes i think your original patch is the better
design
i would suggest though that ass_demuxer is used and not ff_* functions
exportet _where_doing_so_is_simpler_.

again, iam sorry for this, it is entirely my mistake

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

When the tyrant has disposed of foreign enemies by conquest or treaty, and
there is nothing more to fear from them, then he is always stirring up
some war or other, in order that the people may require a leader. -- Plato
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100711/d63cb0e3/attachment.pgp>



More information about the ffmpeg-devel mailing list