[FFmpeg-devel] [PATCH] ffmpeg: switch to codecpar
wm4
nfxjfg at googlemail.com
Fri Sep 23 10:41:35 EEST 2016
On Tue, 20 Sep 2016 14:44:47 +0200
Clément Bœsch <u at pkh.me> wrote:
> 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.
+1
>
> 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?
You mean within libavcodec? Maybe, but only f the decoder absolutely
needs it, not unconditionally.
More information about the ffmpeg-devel
mailing list