[FFmpeg-devel] [PATCH] ffmpeg: switch to codecpar

[wm4]

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.