[FFmpeg-devel] [PATCH]Basic XSUB encoder (take 5)

[Reimar Döffinger]

On Fri, Jun 19, 2009 at 10:31:49AM +0200, Reimar D?ffinger wrote:
> On Thu, Jun 18, 2009 at 03:56:49PM +0200, Michael Niedermayer wrote:
> > On Sat, May 23, 2009 at 11:03:31PM +0200, Reimar D?ffinger wrote:
> > > On Sat, May 23, 2009 at 06:54:55PM +0200, Diego Biurrun wrote:
> > > > On Sat, May 23, 2009 at 05:22:44PM +0200, Reimar D?ffinger wrote:
> > > > > 
> > > > > Haven't done anything on the muxer code part, but I fixed the trivial
> > > > > stuff pointed out here and one or two more simplifications and updated
> > > > > to SVN.
> > > > 
> > > > > --- libavcodec/xsubenc.c	(revision 0)
> > > > > +++ libavcodec/xsubenc.c	(revision 0)
> > > > > @@ -0,0 +1,224 @@
> > > > > +    if (xsub_encode_rle(&pb,
> > > > > +                h->rects[0]->pict.data[0],
> > > > > +                h->rects[0]->pict.linesize[0]*2,
> > > > > +                h->rects[0]->w, (h->rects[0]->h + 1) >> 1))
> > > > > +
> > > > > +    if (xsub_encode_rle(&pb,
> > > > > +            h->rects[0]->pict.data[0] + h->rects[0]->pict.linesize[0],
> > > > > +            h->rects[0]->pict.linesize[0]*2,
> > > > > +            h->rects[0]->w, h->rects[0]->h >> 1))
> > > > 
> > > > This looks weirdly indented.
> > > > 
> > > > > --- libavformat/avienc.c	(revision 18911)
> > > > > +++ libavformat/avienc.c	(working copy)
> > > > > @@ -212,8 +215,10 @@
> > > > > -        if(stream->codec_type == CODEC_TYPE_VIDEO)
> > > > > +        if(stream->codec_type == CODEC_TYPE_VIDEO
> > > > > +                || stream->codec_type == CODEC_TYPE_SUBTITLE)
> > > > 
> > > > If you keep the || on the first line, you can align this nicely.
> > > 
> > > Both changed, avi muxer and pts handling parts unchanged (those are
> > > hopefully the only parts that need to be changed still).
> > 
> > 
> > now the avi changes ...
> > 
> > [...]
> > > Index: libavformat/avienc.c
> > > ===================================================================
> > > --- libavformat/avienc.c	(revision 18911)
> > > +++ libavformat/avienc.c	(working copy)
> > > @@ -81,6 +81,9 @@
> > >      if (type == CODEC_TYPE_VIDEO) {
> > >          tag[2] = 'd';
> > >          tag[3] = 'c';
> > > +    } else if (type == CODEC_TYPE_SUBTITLE) {
> > > +        tag[2] = 's';
> > > +        tag[3] = 'b';
> > >      } else {
> > >          tag[2] = 'w';
> > >          tag[3] = 'b';
> > > @@ -212,8 +215,10 @@
> > >          case CODEC_TYPE_AUDIO: put_tag(pb, "auds"); break;
> > >  //        case CODEC_TYPE_TEXT : put_tag(pb, "txts"); break;
> > >          case CODEC_TYPE_DATA : put_tag(pb, "dats"); break;
> > > +        case CODEC_TYPE_SUBTITLE: put_tag(pb, "vids"); break;
> > 
> > can you quote some spec that requires these?
> > 
> > 
> > [...]
> > > @@ -253,6 +258,7 @@
> > >          strf = start_tag(pb, "strf");
> > >          switch(stream->codec_type) {
> > >          case CODEC_TYPE_VIDEO:
> > > +        case CODEC_TYPE_SUBTITLE:
> > >              put_bmp_header(pb, stream, codec_bmp_tags, 0);
> > >              break;
> > >          case CODEC_TYPE_AUDIO:
> > 
> > same question
> > 
> > if its just xsub then there should be a check for that not just the
> > codec_type IMHO
> 
> I am quite sure that all this is XSUB-specific, and the checks should
> be for that.
> Well, except maybe using "sb" for the subtitle track tag, does anyone
> know something about that? "wb" at least seems strange for a subtitle
> track...

The official specification btw. says:
db 	Uncompressed video frame
dc 	Compressed video frame
pc 	Palette change
wb 	Audio data

So using "sb" for subtitles at least it is not worse than what the
current code does, I added a comment that it is not an official code
though.