| Message ID | 1516251814-6220-2-git-send-email-zhong.li@intel.com |
|---|---|
| State | New |
| Headers | show |
2018-01-18 6:03 GMT+01:00 Zhong Li <zhong.li@intel.com>: > Currently a hacky way is used for some specific codecs such as > H264/VP6F/DXV > (and "lowres" case is broken now). How can I reproduce this? Carl Eugen
> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf > Of Carl Eugen Hoyos > Sent: Thursday, January 18, 2018 5:26 PM > To: FFmpeg development discussions and patches > <ffmpeg-devel@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [PATCH V2 2/2] Don't overwrite previously setup > dimensions for all codecs > > 2018-01-18 6:03 GMT+01:00 Zhong Li <zhong.li@intel.com>: > > Currently a hacky way is used for some specific codecs such as > > H264/VP6F/DXV > > > (and "lowres" case is broken now). > > How can I reproduce this? Just infer from the code. The logic is different between H264/VP6F/DXV and non-H264/VP6F/DXV if lowres is enable. (Maybe there is somewhere else to handle H264/VP6F/DXV lowres cases and make it work as expectation?) Please correct me if I am wrong and I am happy to update the commit message. BTW, there is a typo in the patch (lowles->lowres). Fixed locally but forgot to submit : (.
2018-01-19 4:28 GMT+01:00 Li, Zhong <zhong.li@intel.com>: >> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf >> Of Carl Eugen Hoyos >> Sent: Thursday, January 18, 2018 5:26 PM >> To: FFmpeg development discussions and patches >> <ffmpeg-devel@ffmpeg.org> >> Subject: Re: [FFmpeg-devel] [PATCH V2 2/2] Don't overwrite previously setup >> dimensions for all codecs >> >> 2018-01-18 6:03 GMT+01:00 Zhong Li <zhong.li@intel.com>: >> > Currently a hacky way is used for some specific codecs such as >> > H264/VP6F/DXV >> >> > (and "lowres" case is broken now). >> >> How can I reproduce this? > > Just infer from the code. Sadly, I am not so good at this, I prefer to test actual command lines. Carl Eugen
On Thu, Jan 18, 2018 at 01:03:34PM +0800, Zhong Li wrote: > Currently a hacky way is used for some specific codecs such as > H264/VP6F/DXV (and "lowres" case is broken now). > Replace with a more generic way(an evolution based on a libav commit > 9de9b828 but hasn't been merged since it breaks lowres). > > V1->V2: add "lowres" handle code > > Signed-off-by: Zhong Li <zhong.li@intel.com> > --- > libavcodec/utils.c | 8 ++------ > 1 file changed, 2 insertions(+), 6 deletions(-) > > diff --git a/libavcodec/utils.c b/libavcodec/utils.c > index 427f612..fdd1b46 100644 > --- a/libavcodec/utils.c > +++ b/libavcodec/utils.c > @@ -684,16 +684,12 @@ int attribute_align_arg avcodec_open2(AVCodecContext *avctx, const AVCodec *code > goto free_and_end; > } > > - // only call ff_set_dimensions() for non H.264/VP6F/DXV codecs so as not to overwrite previously setup dimensions > - if (!(avctx->coded_width && avctx->coded_height && avctx->width && avctx->height && > - (avctx->codec_id == AV_CODEC_ID_H264 || avctx->codec_id == AV_CODEC_ID_VP6F || avctx->codec_id == AV_CODEC_ID_DXV))) { > - if (avctx->coded_width && avctx->coded_height) > + if (avctx->coded_width && avctx->coded_height && (!avctx->width && !avctx->height || avctx->lowles)) > ret = ff_set_dimensions(avctx, avctx->coded_width, avctx->coded_height); > - else if (avctx->width && avctx->height) > + else if (avctx->width && avctx->height && (!avctx->coded_width && !avctx->coded_height || avctx->lowles)) > ret = ff_set_dimensions(avctx, avctx->width, avctx->height); > if (ret < 0) > goto free_and_end; > - } This has typos in variable names, it will not work nor build please make sure that submitted patches have been tested before [...]
> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf > Of Michael Niedermayer > Sent: Saturday, January 27, 2018 8:05 AM > To: FFmpeg development discussions and patches > <ffmpeg-devel@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [PATCH V2 2/2] Don't overwrite previously setup > dimensions for all codecs > > On Thu, Jan 18, 2018 at 01:03:34PM +0800, Zhong Li wrote: > > Currently a hacky way is used for some specific codecs such as > > H264/VP6F/DXV (and "lowres" case is broken now). > > Replace with a more generic way(an evolution based on a libav commit > > 9de9b828 but hasn't been merged since it breaks lowres). > > > > V1->V2: add "lowres" handle code > > > > Signed-off-by: Zhong Li <zhong.li@intel.com> > > --- > > libavcodec/utils.c | 8 ++------ > > 1 file changed, 2 insertions(+), 6 deletions(-) > > > > diff --git a/libavcodec/utils.c b/libavcodec/utils.c index > > 427f612..fdd1b46 100644 > > --- a/libavcodec/utils.c > > +++ b/libavcodec/utils.c > > @@ -684,16 +684,12 @@ int attribute_align_arg > avcodec_open2(AVCodecContext *avctx, const AVCodec *code > > goto free_and_end; > > } > > > > - // only call ff_set_dimensions() for non H.264/VP6F/DXV codecs so as > not to overwrite previously setup dimensions > > - if (!(avctx->coded_width && avctx->coded_height && avctx->width > && avctx->height && > > - (avctx->codec_id == AV_CODEC_ID_H264 || avctx->codec_id > == AV_CODEC_ID_VP6F || avctx->codec_id == AV_CODEC_ID_DXV))) { > > - if (avctx->coded_width && avctx->coded_height) > > + if (avctx->coded_width && avctx->coded_height && (!avctx->width > > + && !avctx->height || avctx->lowles)) > > ret = ff_set_dimensions(avctx, avctx->coded_width, > avctx->coded_height); > > - else if (avctx->width && avctx->height) > > + else if (avctx->width && avctx->height && (!avctx->coded_width && > > + !avctx->coded_height || avctx->lowles)) > > ret = ff_set_dimensions(avctx, avctx->width, avctx->height); > > if (ret < 0) > > goto free_and_end; > > - } > > This has typos in variable names, it will not work nor build > > please make sure that submitted patches have been tested before Really sorry for that, I fixed it locally but forgot to submit. I've sent an updated patch now.
diff --git a/libavcodec/utils.c b/libavcodec/utils.c index 427f612..fdd1b46 100644 --- a/libavcodec/utils.c +++ b/libavcodec/utils.c @@ -684,16 +684,12 @@ int attribute_align_arg avcodec_open2(AVCodecContext *avctx, const AVCodec *code goto free_and_end; } - // only call ff_set_dimensions() for non H.264/VP6F/DXV codecs so as not to overwrite previously setup dimensions - if (!(avctx->coded_width && avctx->coded_height && avctx->width && avctx->height && - (avctx->codec_id == AV_CODEC_ID_H264 || avctx->codec_id == AV_CODEC_ID_VP6F || avctx->codec_id == AV_CODEC_ID_DXV))) { - if (avctx->coded_width && avctx->coded_height) + if (avctx->coded_width && avctx->coded_height && (!avctx->width && !avctx->height || avctx->lowles)) ret = ff_set_dimensions(avctx, avctx->coded_width, avctx->coded_height); - else if (avctx->width && avctx->height) + else if (avctx->width && avctx->height && (!avctx->coded_width && !avctx->coded_height || avctx->lowles)) ret = ff_set_dimensions(avctx, avctx->width, avctx->height); if (ret < 0) goto free_and_end; - } if ((avctx->coded_width || avctx->coded_height || avctx->width || avctx->height) && ( av_image_check_size2(avctx->coded_width, avctx->coded_height, avctx->max_pixels, AV_PIX_FMT_NONE, 0, avctx) < 0
Currently a hacky way is used for some specific codecs such as H264/VP6F/DXV (and "lowres" case is broken now). Replace with a more generic way(an evolution based on a libav commit 9de9b828 but hasn't been merged since it breaks lowres). V1->V2: add "lowres" handle code Signed-off-by: Zhong Li <zhong.li@intel.com> --- libavcodec/utils.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)