Re: [FFmpeg-devel] [PATCH 2/2] avcodec/utils: Warn if a encoder is missing the pix_fmts or sample_fmts list

[Hendrik Leppkes]

On Sun, Feb 28, 2016 at 2:14 AM, Michael Niedermayer
 wrote:
> On Sun, Feb 28, 2016 at 01:26:16AM +0100, Hendrik Leppkes wrote:
>> On Sun, Feb 28, 2016 at 12:17 AM, Michael Niedermayer
>>  wrote:
>> > This would require listing supported formats for rawvideo and 
>> > wrapped_avframe
>> >
>> > Signed-off-by: Michael Niedermayer 
>> > ---
>> >  libavcodec/utils.c |5 +
>> >  1 file changed, 5 insertions(+)
>> >
>> > diff --git a/libavcodec/utils.c b/libavcodec/utils.c
>> > index 2690d0d..267c973 100644
>> > --- a/libavcodec/utils.c
>> > +++ b/libavcodec/utils.c
>> > @@ -188,6 +188,11 @@ av_cold void avcodec_register(AVCodec *codec)
>> >
>> >  if (codec->init_static_data)
>> >  codec->init_static_data(codec);
>> > +
>> > +if (av_codec_is_encoder(codec) && codec->type == AVMEDIA_TYPE_VIDEO 
>> > && !codec->pix_fmts)
>> > +av_log(NULL, AV_LOG_WARNING, "Encoder %s is missing the pix_fmts 
>> > field\n", codec->name);
>> > +if (av_codec_is_encoder(codec) && codec->type == AVMEDIA_TYPE_AUDIO  
>> > && !codec->sample_fmts)
>> > +av_log(NULL, AV_LOG_WARNING, "Encoder %s is missing the 
>> > sample_fmts field\n", codec->name);
>> >  }
>>
>> It seems kinda silly to write code to check if other code is correct.
>
> i dont think i understand because what you say would mean that
> fate and all self tests are a bad idea if i dont misunderstand
>
>

A dedicated test suite is different to random checks in random
functions. FATE wouldn't even detect such cases, since its just log.

- Hendrik
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 2/2] avcodec/utils: Warn if a encoder is missing the pix_fmts or sample_fmts list

[wm4]

On Sun, 28 Feb 2016 02:14:20 +0100
Michael Niedermayer  wrote:

> On Sun, Feb 28, 2016 at 01:26:16AM +0100, Hendrik Leppkes wrote:
> > On Sun, Feb 28, 2016 at 12:17 AM, Michael Niedermayer
> >  wrote:  
> > > This would require listing supported formats for rawvideo and 
> > > wrapped_avframe
> > >
> > > Signed-off-by: Michael Niedermayer 
> > > ---
> > >  libavcodec/utils.c |5 +
> > >  1 file changed, 5 insertions(+)
> > >
> > > diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> > > index 2690d0d..267c973 100644
> > > --- a/libavcodec/utils.c
> > > +++ b/libavcodec/utils.c
> > > @@ -188,6 +188,11 @@ av_cold void avcodec_register(AVCodec *codec)
> > >
> > >  if (codec->init_static_data)
> > >  codec->init_static_data(codec);
> > > +
> > > +if (av_codec_is_encoder(codec) && codec->type == AVMEDIA_TYPE_VIDEO 
> > > && !codec->pix_fmts)
> > > +av_log(NULL, AV_LOG_WARNING, "Encoder %s is missing the pix_fmts 
> > > field\n", codec->name);
> > > +if (av_codec_is_encoder(codec) && codec->type == AVMEDIA_TYPE_AUDIO  
> > > && !codec->sample_fmts)
> > > +av_log(NULL, AV_LOG_WARNING, "Encoder %s is missing the 
> > > sample_fmts field\n", codec->name);
> > >  }  
> > 
> > It seems kinda silly to write code to check if other code is correct.  
> 
> i dont think i understand because what you say would mean that
> fate and all self tests are a bad idea if i dont misunderstand
> 
> 
> > Apparently all encoders today are correct, so lets just pay attention  
> 
> rawvideo and wrapped_avframe dont list pix_fmts, they would need to
> be fixed first if we apply this
> 

Really, for 2 encoders?

> 
> > in the future when adding new ones?  
> 
> a self test like this would detect that reliably without the need for
> (less reliable and limited) human resources.
> 

This can be done externally, no need to litter utils.c with more
effectively dead code.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 2/2] avcodec/utils: Warn if a encoder is missing the pix_fmts or sample_fmts list

[Michael Niedermayer]

On Sun, Feb 28, 2016 at 01:26:16AM +0100, Hendrik Leppkes wrote:
> On Sun, Feb 28, 2016 at 12:17 AM, Michael Niedermayer
>  wrote:
> > This would require listing supported formats for rawvideo and 
> > wrapped_avframe
> >
> > Signed-off-by: Michael Niedermayer 
> > ---
> >  libavcodec/utils.c |5 +
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> > index 2690d0d..267c973 100644
> > --- a/libavcodec/utils.c
> > +++ b/libavcodec/utils.c
> > @@ -188,6 +188,11 @@ av_cold void avcodec_register(AVCodec *codec)
> >
> >  if (codec->init_static_data)
> >  codec->init_static_data(codec);
> > +
> > +if (av_codec_is_encoder(codec) && codec->type == AVMEDIA_TYPE_VIDEO && 
> > !codec->pix_fmts)
> > +av_log(NULL, AV_LOG_WARNING, "Encoder %s is missing the pix_fmts 
> > field\n", codec->name);
> > +if (av_codec_is_encoder(codec) && codec->type == AVMEDIA_TYPE_AUDIO  
> > && !codec->sample_fmts)
> > +av_log(NULL, AV_LOG_WARNING, "Encoder %s is missing the 
> > sample_fmts field\n", codec->name);
> >  }
> 
> It seems kinda silly to write code to check if other code is correct.

i dont think i understand because what you say would mean that
fate and all self tests are a bad idea if i dont misunderstand


> Apparently all encoders today are correct, so lets just pay attention

rawvideo and wrapped_avframe dont list pix_fmts, they would need to
be fixed first if we apply this


> in the future when adding new ones?

a self test like this would detect that reliably without the need for
(less reliable and limited) human resources.


[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If you drop bombs on a foreign country and kill hundred thousands of
innocent people, expect your government to call the consequence
"unprovoked inhuman terrorist attacks" and use it to justify dropping
more bombs and killing more people. The technology changed, the idea is old.


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 2/2] avcodec/utils: Warn if a encoder is missing the pix_fmts or sample_fmts list

[Hendrik Leppkes]

On Sun, Feb 28, 2016 at 12:17 AM, Michael Niedermayer
 wrote:
> This would require listing supported formats for rawvideo and wrapped_avframe
>
> Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/utils.c |5 +
>  1 file changed, 5 insertions(+)
>
> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> index 2690d0d..267c973 100644
> --- a/libavcodec/utils.c
> +++ b/libavcodec/utils.c
> @@ -188,6 +188,11 @@ av_cold void avcodec_register(AVCodec *codec)
>
>  if (codec->init_static_data)
>  codec->init_static_data(codec);
> +
> +if (av_codec_is_encoder(codec) && codec->type == AVMEDIA_TYPE_VIDEO && 
> !codec->pix_fmts)
> +av_log(NULL, AV_LOG_WARNING, "Encoder %s is missing the pix_fmts 
> field\n", codec->name);
> +if (av_codec_is_encoder(codec) && codec->type == AVMEDIA_TYPE_AUDIO  && 
> !codec->sample_fmts)
> +av_log(NULL, AV_LOG_WARNING, "Encoder %s is missing the sample_fmts 
> field\n", codec->name);
>  }

It seems kinda silly to write code to check if other code is correct.
Apparently all encoders today are correct, so lets just pay attention
in the future when adding new ones?

- Hendrik
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel