[FFmpeg-devel,V2,2/2] Don't overwrite previously setup dimensions for all codecs

Submitted by Zhong Li on Jan. 18, 2018, 5:03 a.m.

Details

Message ID 1516251814-6220-2-git-send-email-zhong.li@intel.com
State New
Headers show

Commit Message

Zhong Li Jan. 18, 2018, 5:03 a.m.
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(-)

Comments

Carl Eugen Hoyos Jan. 18, 2018, 9:26 a.m.
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
Zhong Li Jan. 19, 2018, 3:28 a.m.
> 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 : (.
Carl Eugen Hoyos Jan. 19, 2018, 11:17 a.m.
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
Michael Niedermayer Jan. 27, 2018, 12:04 a.m.
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

[...]
Zhong Li Jan. 27, 2018, 3:57 a.m.
> 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.

Patch hide | download patch | download mbox

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