Message ID | 20220824084318.333-5-anton@khirnov.net |
---|---|
State | New |
Headers | show |
Series | [FFmpeg-devel,01/18] tests/fate/mov: add a test for dv audio demuxed through dv demuxer | expand |
Context | Check | Description |
---|---|---|
yinshiyou/make_loongarch64 | success | Make finished |
yinshiyou/make_fate_loongarch64 | fail | Make fate failed |
andriy/make_x86 | success | Make finished |
andriy/make_fate_x86 | fail | Make fate failed |
Anton Khirnov: > The function contains only two assignments, which are simpler to > duplicate into decoder and encoder. Additionally, dvdec does not use > DVVideoContext.avctx at all. > --- > libavcodec/dv.c | 10 ---------- > libavcodec/dv.h | 2 -- > libavcodec/dvdec.c | 4 +++- > libavcodec/dvenc.c | 5 ++++- > 4 files changed, 7 insertions(+), 14 deletions(-) > > diff --git a/libavcodec/dv.c b/libavcodec/dv.c > index e2550c4cc1..9e05aa8927 100644 > --- a/libavcodec/dv.c > +++ b/libavcodec/dv.c > @@ -184,13 +184,3 @@ int ff_dv_init_dynamic_tables(DVVideoContext *ctx, const AVDVProfile *d) > > return 0; > } > - > -av_cold int ff_dvvideo_init(AVCodecContext *avctx) > -{ > - DVVideoContext *s = avctx->priv_data; > - > - s->avctx = avctx; > - avctx->chroma_sample_location = AVCHROMA_LOC_TOPLEFT; > - > - return 0; > -} > diff --git a/libavcodec/dv.h b/libavcodec/dv.h > index 331b8e846a..2b082d0140 100644 > --- a/libavcodec/dv.h > +++ b/libavcodec/dv.h > @@ -97,8 +97,6 @@ enum dv_pack_type { > > int ff_dv_init_dynamic_tables(DVVideoContext *s, const AVDVProfile *d); > > -int ff_dvvideo_init(AVCodecContext *avctx); > - > static inline int dv_work_pool_size(const AVDVProfile *d) > { > int size = d->n_difchan * d->difseg_size * 27; > diff --git a/libavcodec/dvdec.c b/libavcodec/dvdec.c > index bad8419925..daee2347e6 100644 > --- a/libavcodec/dvdec.c > +++ b/libavcodec/dvdec.c > @@ -240,6 +240,8 @@ static av_cold int dvvideo_decode_init(AVCodecContext *avctx) > DVVideoContext *s = avctx->priv_data; > int i; > > + avctx->chroma_sample_location = AVCHROMA_LOC_TOPLEFT; > + > ff_idctdsp_init(&s->idsp, avctx); > > for (i = 0; i < 64; i++) > @@ -258,7 +260,7 @@ static av_cold int dvvideo_decode_init(AVCodecContext *avctx) > > ff_thread_once(&init_static_once, dv_init_static); > > - return ff_dvvideo_init(avctx); > + return 0; > } > > /* decode AC coefficients */ > diff --git a/libavcodec/dvenc.c b/libavcodec/dvenc.c > index 5ba4de3213..8027feb9b3 100644 > --- a/libavcodec/dvenc.c > +++ b/libavcodec/dvenc.c > @@ -55,6 +55,9 @@ static av_cold int dvvideo_encode_init(AVCodecContext *avctx) > PixblockDSPContext pdsp; > int ret; > > + s->avctx = avctx; > + avctx->chroma_sample_location = AVCHROMA_LOC_TOPLEFT; > + > s->sys = av_dv_codec_profile2(avctx->width, avctx->height, avctx->pix_fmt, avctx->time_base); > if (!s->sys) { > av_log(avctx, AV_LOG_ERROR, "Found no DV profile for %ix%i %s video. " > @@ -91,7 +94,7 @@ static av_cold int dvvideo_encode_init(AVCodecContext *avctx) > } > #endif > > - return ff_dvvideo_init(avctx); > + return 0; > } > > /* bit budget for AC only in 5 MBs */ There is actually another issue here: Encoders are not supposed to set chroma_sample_location at all according to the documentation. I sent a patch to implement this (https://ffmpeg.org/pipermail/ffmpeg-devel/2022-July/298518.html -- it also inlines and removes ff_dvvideo_init()), but I am unsure whether it is not the documentation that needs to be updated. - Andreas
Quoting Andreas Rheinhardt (2022-08-24 14:27:43) > There is actually another issue here: Encoders are not supposed to set > chroma_sample_location at all according to the documentation. I sent a > patch to implement this > (https://ffmpeg.org/pipermail/ffmpeg-devel/2022-July/298518.html -- it > also inlines and removes ff_dvvideo_init()), but I am unsure whether it > is not the documentation that needs to be updated. I would say update the documentation. It does make sense for encoders to set this field.
Quoting Anton Khirnov (2022-08-25 11:48:27) > Quoting Andreas Rheinhardt (2022-08-24 14:27:43) > > There is actually another issue here: Encoders are not supposed to set > > chroma_sample_location at all according to the documentation. I sent a > > patch to implement this > > (https://ffmpeg.org/pipermail/ffmpeg-devel/2022-July/298518.html -- it > > also inlines and removes ff_dvvideo_init()), but I am unsure whether it > > is not the documentation that needs to be updated. > > I would say update the documentation. It does make sense for encoders to > set this field. Or on second thought, maybe no. Apparently no other encoders set it. Though we might want to check whether the value matches and warn/error out otherwise.
diff --git a/libavcodec/dv.c b/libavcodec/dv.c index e2550c4cc1..9e05aa8927 100644 --- a/libavcodec/dv.c +++ b/libavcodec/dv.c @@ -184,13 +184,3 @@ int ff_dv_init_dynamic_tables(DVVideoContext *ctx, const AVDVProfile *d) return 0; } - -av_cold int ff_dvvideo_init(AVCodecContext *avctx) -{ - DVVideoContext *s = avctx->priv_data; - - s->avctx = avctx; - avctx->chroma_sample_location = AVCHROMA_LOC_TOPLEFT; - - return 0; -} diff --git a/libavcodec/dv.h b/libavcodec/dv.h index 331b8e846a..2b082d0140 100644 --- a/libavcodec/dv.h +++ b/libavcodec/dv.h @@ -97,8 +97,6 @@ enum dv_pack_type { int ff_dv_init_dynamic_tables(DVVideoContext *s, const AVDVProfile *d); -int ff_dvvideo_init(AVCodecContext *avctx); - static inline int dv_work_pool_size(const AVDVProfile *d) { int size = d->n_difchan * d->difseg_size * 27; diff --git a/libavcodec/dvdec.c b/libavcodec/dvdec.c index bad8419925..daee2347e6 100644 --- a/libavcodec/dvdec.c +++ b/libavcodec/dvdec.c @@ -240,6 +240,8 @@ static av_cold int dvvideo_decode_init(AVCodecContext *avctx) DVVideoContext *s = avctx->priv_data; int i; + avctx->chroma_sample_location = AVCHROMA_LOC_TOPLEFT; + ff_idctdsp_init(&s->idsp, avctx); for (i = 0; i < 64; i++) @@ -258,7 +260,7 @@ static av_cold int dvvideo_decode_init(AVCodecContext *avctx) ff_thread_once(&init_static_once, dv_init_static); - return ff_dvvideo_init(avctx); + return 0; } /* decode AC coefficients */ diff --git a/libavcodec/dvenc.c b/libavcodec/dvenc.c index 5ba4de3213..8027feb9b3 100644 --- a/libavcodec/dvenc.c +++ b/libavcodec/dvenc.c @@ -55,6 +55,9 @@ static av_cold int dvvideo_encode_init(AVCodecContext *avctx) PixblockDSPContext pdsp; int ret; + s->avctx = avctx; + avctx->chroma_sample_location = AVCHROMA_LOC_TOPLEFT; + s->sys = av_dv_codec_profile2(avctx->width, avctx->height, avctx->pix_fmt, avctx->time_base); if (!s->sys) { av_log(avctx, AV_LOG_ERROR, "Found no DV profile for %ix%i %s video. " @@ -91,7 +94,7 @@ static av_cold int dvvideo_encode_init(AVCodecContext *avctx) } #endif - return ff_dvvideo_init(avctx); + return 0; } /* bit budget for AC only in 5 MBs */