On Tue, Sep 20, 2016 at 01:46:11PM +0200, Hendrik Leppkes wrote:
> > + /*
> > + * For subtitles, this is required by the decoding process in
> > order to
> > + * rescale the timestamps: in the current API the decoded subtitles
> > + * have their pts expressed in AV_TIME_BASE, and thus the lavc
> > + * internals need to know the stream time base in order to achieve
> > the
> > + * rescaling.
> > + *
> > + * That API is old and needs to be reworked to match behaviour
> > with A/V
> > + * (FIXME).
> > + *
> > + * For Audio, this is apparently required for the
> > + * fate-gaplessenc-itunes-to-ipod-aac test (FIXME).
> > + */
> > + if (ist->dec_ctx->codec_type == AVMEDIA_TYPE_SUBTITLE ||
> > + ist->dec_ctx->codec_type == AVMEDIA_TYPE_AUDIO)
> > + av_codec_set_pkt_timebase(ist->dec_ctx, ist->st->time_base);
> > +
> I didn't look at the surrounding code much, so maybe I'm out of
> context here, but I would think setting pkt_timebase on the decoder is
> generally not a bad thing, for any type of codec?
> Audio uses it to skip samples, subtitles for rescaling timestamps, at
> least the cuvid video decoder uses it to rescale timestamps for the
> external API as well.
> So maybe set it unconditionally? Setting pkt_timebase is not a bad
> thing, I would think. It was probably copied from the st->codec
> before, as well.
If that's fine with everyone I can drop the condition and comment. But
generally speaking, aside from subtitles it looks like it mostly works
when it's not set, so should we warn when the user doesn't set it in order
to avoid random undefined behaviour?
ffmpeg-devel mailing list