| Message ID | 1515571815-24627-1-git-send-email-zhong.li@intel.com |
|---|---|
| State | Superseded |
| Headers | show |
On Wed, Jan 10, 2018 at 9:10 AM, Zhong Li <zhong.li@intel.com> wrote: > Currently a hacky way is used for some specific codecs such as > H264/VP6F/DXV. > Replace with a more generic way(an evolution based on a history commit > 9de9b828ef005dec37052548c195a6b4f18fc701 but reverted somehow) to handle these cases. > What does this actually fix or change? That should preferably be documented in the commit message. - Hendrik
2018-01-10 9:10 GMT+01:00 Zhong Li <zhong.li@intel.com>: > Currently a hacky way is used for some specific codecs such as > H264/VP6F/DXV. > Replace with a more generic way(an evolution based on a history commit > 9de9b828ef005dec37052548c195a6b4f18fc701 but reverted somehow) This commit was never part of FFmpeg. > to handle these cases. Carl Eugen
> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf > Of Carl Eugen Hoyos > Sent: Wednesday, January 10, 2018 10:18 PM > To: FFmpeg development discussions and patches > <ffmpeg-devel@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [PATCH] Don't overwrite previously setup > dimensions for all codecs > > 2018-01-10 9:10 GMT+01:00 Zhong Li <zhong.li@intel.com>: > > Currently a hacky way is used for some specific codecs such as > > H264/VP6F/DXV. > > > Replace with a more generic way(an evolution based on a history commit > > 9de9b828ef005dec37052548c195a6b4f18fc701 but reverted somehow) > > This commit was never part of FFmpeg. IIUC, This commit was merged from Libav, checking the commit log of cf7d3846fc865df47bd64f7d4d10bbedf9e3a17b: Merge commit '9de9b828ef005dec37052548c195a6b4f18fc701' * commit '9de9b828ef005dec37052548c195a6b4f18fc701': lavc: don't overwrite display dimensions with coded dimensions. lavc: extend / update the [coded_]{width,height} doxy Conflicts: libavcodec/avcodec.h libavcodec/utils.c The change to the w/h handling is not merged as it breaks lowres Merged-by: Michael Niedermayer <michaelni@gmx.at>
On Thu, 11 Jan 2018 02:01:38 +0000 "Li, Zhong" <zhong.li@intel.com> wrote: > IIUC, This commit was merged from Libav, checking the commit log of cf7d3846fc865df47bd64f7d4d10bbedf9e3a17b: Yeah, but... > Merge commit '9de9b828ef005dec37052548c195a6b4f18fc701' > > * commit '9de9b828ef005dec37052548c195a6b4f18fc701': > lavc: don't overwrite display dimensions with coded dimensions. > lavc: extend / update the [coded_]{width,height} doxy > > Conflicts: > libavcodec/avcodec.h > libavcodec/utils.c > > The change to the w/h handling is not merged as it breaks lowres > > Merged-by: Michael Niedermayer <michaelni@gmx.at> This says this specific part was skipped. Apparently it broke lowres, an obscure feature that probably nobody uses and that Libav removed.
> > IIUC, This commit was merged from Libav, checking the commit log of > cf7d3846fc865df47bd64f7d4d10bbedf9e3a17b: > > Yeah, but... > > > Merge commit '9de9b828ef005dec37052548c195a6b4f18fc701' > > > > * commit '9de9b828ef005dec37052548c195a6b4f18fc701': > > lavc: don't overwrite display dimensions with coded dimensions. > > lavc: extend / update the [coded_]{width,height} doxy > > > > Conflicts: > > libavcodec/avcodec.h > > libavcodec/utils.c > > > > The change to the w/h handling is not merged as it breaks lowres > > > > Merged-by: Michael Niedermayer <michaelni@gmx.at> > > This says this specific part was skipped. Apparently it broke lowres, an > obscure feature that probably nobody uses and that Libav removed. Yup... 9de9b82 was not merged as a whole. I guess I am not the only one person who is confused why w/h are overwritten by coded_w/coded_h here (https://ffmpeg.org/pipermail/ffmpeg-devel/2013-March/140142.html ) For "lowres", is it still broken for H264/VP6F/DXV by current code?
On Thu, 11 Jan 2018 05:34:07 +0000 "Li, Zhong" <zhong.li@intel.com> wrote: > > > IIUC, This commit was merged from Libav, checking the commit log of > > cf7d3846fc865df47bd64f7d4d10bbedf9e3a17b: > > > > Yeah, but... > > > > > Merge commit '9de9b828ef005dec37052548c195a6b4f18fc701' > > > > > > * commit '9de9b828ef005dec37052548c195a6b4f18fc701': > > > lavc: don't overwrite display dimensions with coded dimensions. > > > lavc: extend / update the [coded_]{width,height} doxy > > > > > > Conflicts: > > > libavcodec/avcodec.h > > > libavcodec/utils.c > > > > > > The change to the w/h handling is not merged as it breaks lowres > > > > > > Merged-by: Michael Niedermayer <michaelni@gmx.at> > > > > This says this specific part was skipped. Apparently it broke lowres, an > > obscure feature that probably nobody uses and that Libav removed. > > Yup... 9de9b82 was not merged as a whole. > I guess I am not the only one person who is confused why w/h are overwritten by coded_w/coded_h here (https://ffmpeg.org/pipermail/ffmpeg-devel/2013-March/140142.html ) > For "lowres", is it still broken for H264/VP6F/DXV by current code? I don't know (from the mail above this might be more an API problem?), but I'd prefer your patch over the current situation as well.
2018-01-11 3:01 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: Wednesday, January 10, 2018 10:18 PM >> To: FFmpeg development discussions and patches >> <ffmpeg-devel@ffmpeg.org> >> Subject: Re: [FFmpeg-devel] [PATCH] Don't overwrite previously setup >> dimensions for all codecs >> >> 2018-01-10 9:10 GMT+01:00 Zhong Li <zhong.li@intel.com>: >> > Currently a hacky way is used for some specific codecs such as >> > H264/VP6F/DXV. >> >> > Replace with a more generic way(an evolution based on a history commit >> > 9de9b828ef005dec37052548c195a6b4f18fc701 but reverted somehow) >> >> This commit was never part of FFmpeg. > > IIUC, This commit was merged from Libav But not the part you referenced in your original mail here. Would you like to also comment on Hendrik's question? Carl Eugen
> >> 2018-01-10 9:10 GMT+01:00 Zhong Li <zhong.li@intel.com>: > >> > Currently a hacky way is used for some specific codecs such as > >> > H264/VP6F/DXV. > >> > >> > Replace with a more generic way(an evolution based on a history > >> > commit > >> > 9de9b828ef005dec37052548c195a6b4f18fc701 but reverted somehow) > >> > >> This commit was never part of FFmpeg. > > > > IIUC, This commit was merged from Libav > > But not the part you referenced in your original mail here. > > Would you like to also comment on Hendrik's question? I'm not sure you are interested to an issue on a forked repo (https://github.com/Intel-FFmpeg-Plugin/Intel_FFmpeg_plugins/issues/12 ). This issue is caused by w/h are overwritten by coded_w/coded_h (Just like https://trac.ffmpeg.org/ticket/1386 ). It can't be reproduced on latest FFmpeg since codec->coded_w/coded_h haven't been passed in avcodec_parameters_from_context(). It means ticket #1386 also can't be reproduced now even remove the original fixing patch (commit 33b0549) But latest FFmpeg introduces a new issue (https://trac.ffmpeg.org/ticket/6958 )
diff --git a/libavcodec/utils.c b/libavcodec/utils.c index 4d736d2..3f3f3db 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) 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) 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. Replace with a more generic way(an evolution based on a history commit 9de9b828ef005dec37052548c195a6b4f18fc701 but reverted somehow) to handle these cases. Signed-off-by: Zhong Li <zhong.li@intel.com> --- libavcodec/utils.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)