[FFmpeg-cvslog] r19646 - trunk/libavcodec/xsubdec.c

[Reimar Döffinger]

On Sun, Aug 16, 2009 at 06:13:00PM +0200, Michael Niedermayer wrote:
> On Sat, Aug 15, 2009 at 11:44:07AM +0200, Reimar D?ffinger wrote:
> > On Sat, Aug 15, 2009 at 11:18:13AM +0200, Reimar D?ffinger wrote:
> > > On Sat, Aug 15, 2009 at 11:12:58AM +0200, reimar wrote:
> > > > Author: reimar
> > > > Date: Sat Aug 15 11:12:58 2009
> > > > New Revision: 19646
> > > > 
> > > > Log:
> > > > Fix start_display_time/end_display_time to be relative to packet pts in xsub decoder.
> > > > Makes no difference for the sample AVI files since they all have no packet pts values.
> > > 
> > > This is actually not working at all anyway, ffplay leaves the AVPacket pts in the stream
> > > time_base (which is unknown in the decoder) and ffmpeg does not set it at all.
> > 
> > Also great fun:
> > in ffmpeg.c:do_subtitle_out
> >         subtitle_out_size = avcodec_encode_subtitle(enc, subtitle_out,
> > 	                                                    subtitle_out_max_size,
> > 							    sub);
> > ...
> >                 pkt.pts += 90 * sub->start_display_time;
> > 
> > But in avcodec_encode_subtitle:
> >    if(sub->start_display_time) {
> >            av_log(avctx, AV_LOG_ERROR, "start_display_time must be 0.\n");
> >            return -1;
> >    }
> > 
> > If someone can explain to me WTF they thought when coming up with this
> > design I might be able to fix it.
> 
> the 90 * was probably fabrice being too lazy,busy and sloppy and thinkig
> just of mpegs 90khz

Well, the point is more that it is dead code anyway since sub->start_display_time
is == 0 at that point otherwise encoding would have failed...
Or more precisely:
either the pkt.pts += 90 * sub->start_display_time; or the
sub->start_display_time == 0 make no sense.

> the sub->start_display_time == 0 check follows the idea that muxers should
> mux packets with correct dts/pts so the muxer can place them in the file
> and write indexes and whatever else is needed in a way that they are
> returned on demuxing that file at the correct time even in case of seeking

Yes, I thought so, though I am not sure if it is all that correct with
DVD subtitles, the players might need them to be a few ms before the
point where they have to be displayed...
Anyway I guess it would be reasonably correct to "shift" the
start_display_time to the pkt.pts value.