On Tue, 20 Sep 2016 14:44:47 +0200 Clément Bœsch <u...@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. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
