[FFmpeg-devel] [PATCH 4/4] avidec: demux ASS and SRT tracks

[Aurelien Jacobs]

On Wed, Jul 07, 2010 at 09:27:30PM +0200, Michael Niedermayer wrote:
> On Tue, Jul 06, 2010 at 10:55:04PM +0200, Aurelien Jacobs wrote:
> > 
> > ---
> >  libavformat/avidec.c |   42 ++++++++++++++++++++++++++++++++++++++++--
> >  libavformat/utils.c  |    2 ++
> >  2 files changed, 42 insertions(+), 2 deletions(-)
> 
> >  avidec.c |   42 ++++++++++++++++++++++++++++++++++++++++--
> >  utils.c  |    2 ++
> >  2 files changed, 42 insertions(+), 2 deletions(-)
> > e528c97d940b4d16e1a96f56dd4686a3b8f3b83e  avidec-demux-ass-and-srt-track.patch
> > diff --git a/libavformat/avidec.c b/libavformat/avidec.c
> > index cdf8307..ac1e3b6 100644
> > --- a/libavformat/avidec.c
> > +++ b/libavformat/avidec.c
> > @@ -598,6 +598,14 @@ static int avi_read_header(AVFormatContext *s, AVFormatParameters *ap)
> >                          ast->dshow_block_align = 0;
> >                      }
> >                      break;
> > +                case AVMEDIA_TYPE_SUBTITLE:
> > +                    st->codec->codec_type = AVMEDIA_TYPE_SUBTITLE;
> > +                    st->codec->codec_id   = CODEC_ID_PROBE;
> 
> > +                    st->codec->codec_tag  = 0;
> 
> why ?

Good question... I don't remember why I did this, but this looks like a
copy/paste of the default case a bit below. It's useless anyway, so removed.

> > @@ -687,6 +695,26 @@ static int avi_read_header(AVFormatContext *s, AVFormatParameters *ap)
> >      return 0;
> >  }
> >  
> > +static void read_gab2_sub(AVStream *st, AVPacket *pkt) {
> > +    if(!strcmp(pkt->data, "GAB2") && AV_RL16(pkt->data+5) == 2) {
> > +        uint8_t tmp, desc[256], *d = desc;
> > +        const uint8_t *ptr = pkt->data+7;
> > +        int i, size, name_size = bytestream_get_le32(&ptr);
> > +
> > +        for(i=0; i<name_size; i+=2)
> > +            PUT_UTF8(bytestream_get_le16(&ptr), tmp,
> > +                     if(d < desc+sizeof(desc)-1)  *d++ = tmp;);
> 
> no end of array checks for the input

Fixed.

> > +        *d = 0;
> > +        av_metadata_set2(&st->metadata, "title", desc, 0);
> > +
> > +        ptr += 2;
> > +        size = bytestream_get_le32(&ptr);
> > +        size = FFMIN(size, pkt->size+pkt->data-ptr);
> > +        memmove(pkt->data, ptr, size);
> > +        pkt->size = size;
> 
> that could be negative

Fixed.

> > @@ -801,6 +829,10 @@ resync:
> >                  av_log(s, AV_LOG_ERROR, "Failed to append palette\n");
> >          }
> >  
> > +        if (st->codec->codec_type == AVMEDIA_TYPE_SUBTITLE
> > +            && !st->codec->codec_tag)
> > +            read_gab2_sub(st, pkt);
> 
> does this really belong in the avi demuxer layer?

I have no idea what you have in mind...
AFAICT the gab2 blob is specific to avi. It's not used in any other
container. And it belongs to the demuxer layer as it contains a title
metadata (which belongs to AVStream).

> > @@ -1138,6 +1170,12 @@ static int avi_read_seek(AVFormatContext *s, int stream_index, int64_t timestamp
> >          ast2->packet_size=
> >          ast2->remaining= 0;
> >  
> > +        if (st2->codec->codec_type == AVMEDIA_TYPE_SUBTITLE
> > +            && !st2->codec->codec_tag) {
> > +            ast2->frame_offset = 0;
> > +            continue;
> > +        }
> 
> this doesnt feel correct, all sees go to point 0 ?

Well, actually gab2 subtitles are composed of one and only one
packet. After seeking, subtitles contained in this packet may be needed
(or not, but the avi demuxer has no way to know). So the only thing the
avi demuxer can do after a seek is to output this single packet.
Did I missed anything ?

Aurel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: avidec-demux-ass-and-srt-track.diff
Type: text/x-diff
Size: 4611 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100708/5f588ade/attachment.diff>