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

[Aurelien Jacobs]

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...

Aurel

[1] http://thread.gmane.org/gmane.comp.video.ffmpeg.devel/95315