[FFmpeg-devel] [PATCH] avcodec/openh264: return (DE|EN)CODER_NOT_FOUND if version check fails

[FFmpeg-devel] [PATCH] avcodec/openh264: return (DE|EN)CODER_NOT_FOUND if version check fails

[Andreas Schneider]

Signed-off-by: Andreas Schneider <asn@cryptomilk.org>
---
 libavcodec/libopenh264dec.c | 2 +-
 libavcodec/libopenh264enc.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavcodec/libopenh264dec.c b/libavcodec/libopenh264dec.c
index 7f5e85402a..97d3630df6 100644
--- a/libavcodec/libopenh264dec.c
+++ b/libavcodec/libopenh264dec.c
@@ -56,7 +56,7 @@ static av_cold int svc_decode_init(AVCodecContext *avctx)
     WelsTraceCallback callback_function;
 
     if ((err = ff_libopenh264_check_version(avctx)) < 0)
-        return err;
+        return AVERROR_DECODER_NOT_FOUND;
 
     if (WelsCreateDecoder(&s->decoder)) {
         av_log(avctx, AV_LOG_ERROR, "Unable to create decoder\n");
diff --git a/libavcodec/libopenh264enc.c b/libavcodec/libopenh264enc.c
index 7c0501a2eb..7649e7b025 100644
--- a/libavcodec/libopenh264enc.c
+++ b/libavcodec/libopenh264enc.c
@@ -137,7 +137,7 @@ static av_cold int svc_encode_init(AVCodecContext *avctx)
     AVCPBProperties *props;
 
     if ((err = ff_libopenh264_check_version(avctx)) < 0)
-        return err;
+        return AVERROR_ENCODER_NOT_FOUND;
 
     if (WelsCreateSVCEncoder(&s->encoder)) {
         av_log(avctx, AV_LOG_ERROR, "Unable to create encoder\n");
-- 
2.35.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avcodec/openh264: return (DE|EN)CODER_NOT_FOUND if version check fails

[Neal Gompa]

On Fri, Feb 18, 2022 at 7:53 AM Andreas Schneider <asn@cryptomilk.org> wrote:
>
> Signed-off-by: Andreas Schneider <asn@cryptomilk.org>
> ---
>  libavcodec/libopenh264dec.c | 2 +-
>  libavcodec/libopenh264enc.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/libavcodec/libopenh264dec.c b/libavcodec/libopenh264dec.c
> index 7f5e85402a..97d3630df6 100644
> --- a/libavcodec/libopenh264dec.c
> +++ b/libavcodec/libopenh264dec.c
> @@ -56,7 +56,7 @@ static av_cold int svc_decode_init(AVCodecContext *avctx)
>      WelsTraceCallback callback_function;
>
>      if ((err = ff_libopenh264_check_version(avctx)) < 0)
> -        return err;
> +        return AVERROR_DECODER_NOT_FOUND;
>
>      if (WelsCreateDecoder(&s->decoder)) {
>          av_log(avctx, AV_LOG_ERROR, "Unable to create decoder\n");
> diff --git a/libavcodec/libopenh264enc.c b/libavcodec/libopenh264enc.c
> index 7c0501a2eb..7649e7b025 100644
> --- a/libavcodec/libopenh264enc.c
> +++ b/libavcodec/libopenh264enc.c
> @@ -137,7 +137,7 @@ static av_cold int svc_encode_init(AVCodecContext *avctx)
>      AVCPBProperties *props;
>
>      if ((err = ff_libopenh264_check_version(avctx)) < 0)
> -        return err;
> +        return AVERROR_ENCODER_NOT_FOUND;
>
>      if (WelsCreateSVCEncoder(&s->encoder)) {
>          av_log(avctx, AV_LOG_ERROR, "Unable to create encoder\n");
> --
> 2.35.1
>

Works well for me and looks good.

Reviewed-by: Neal Gompa <ngompa13@gmail.com>



-- 
真実はいつも一つ！/ Always, there's only one truth!
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avcodec/openh264: return (DE|EN)CODER_NOT_FOUND if version check fails

[Martin Storsjö]

On Fri, 18 Feb 2022, Andreas Schneider wrote:

> Signed-off-by: Andreas Schneider <asn@cryptomilk.org>
> ---
> libavcodec/libopenh264dec.c | 2 +-
> libavcodec/libopenh264enc.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/libavcodec/libopenh264dec.c b/libavcodec/libopenh264dec.c
> index 7f5e85402a..97d3630df6 100644
> --- a/libavcodec/libopenh264dec.c
> +++ b/libavcodec/libopenh264dec.c
> @@ -56,7 +56,7 @@ static av_cold int svc_decode_init(AVCodecContext *avctx)
>     WelsTraceCallback callback_function;
>
>     if ((err = ff_libopenh264_check_version(avctx)) < 0)
> -        return err;
> +        return AVERROR_DECODER_NOT_FOUND;
>
>     if (WelsCreateDecoder(&s->decoder)) {
>         av_log(avctx, AV_LOG_ERROR, "Unable to create decoder\n");
> diff --git a/libavcodec/libopenh264enc.c b/libavcodec/libopenh264enc.c
> index 7c0501a2eb..7649e7b025 100644
> --- a/libavcodec/libopenh264enc.c
> +++ b/libavcodec/libopenh264enc.c
> @@ -137,7 +137,7 @@ static av_cold int svc_encode_init(AVCodecContext *avctx)
>     AVCPBProperties *props;
>
>     if ((err = ff_libopenh264_check_version(avctx)) < 0)
> -        return err;
> +        return AVERROR_ENCODER_NOT_FOUND;
>
>     if (WelsCreateSVCEncoder(&s->encoder)) {
>         av_log(avctx, AV_LOG_ERROR, "Unable to create encoder\n");
> -- 
> 2.35.1

This looks reasonable to me, so I could push this in a little while if 
there's no more comments on it.

But the patch lacks an explanation of _why_ this is done, in addition to 
_what_ it does. I presume that's because the current error code makes some 
decoder/encoder selection logic error out entirely, instead of continuing 
trying some other codec - is that right? That would really be valuable to 
include in the commit message.

// Martin

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avcodec/openh264: return (DE|EN)CODER_NOT_FOUND if version check fails

[Martin Storsjö]

On Wed, 20 Apr 2022, Martin Storsjö wrote:

> On Fri, 18 Feb 2022, Andreas Schneider wrote:
>
>> Signed-off-by: Andreas Schneider <asn@cryptomilk.org>
>> ---
>> libavcodec/libopenh264dec.c | 2 +-
>> libavcodec/libopenh264enc.c | 2 +-
>> 2 files changed, 2 insertions(+), 2 deletions(-)
>> 
>> diff --git a/libavcodec/libopenh264dec.c b/libavcodec/libopenh264dec.c
>> index 7f5e85402a..97d3630df6 100644
>> --- a/libavcodec/libopenh264dec.c
>> +++ b/libavcodec/libopenh264dec.c
>> @@ -56,7 +56,7 @@ static av_cold int svc_decode_init(AVCodecContext *avctx)
>>     WelsTraceCallback callback_function;
>>
>>     if ((err = ff_libopenh264_check_version(avctx)) < 0)
>> -        return err;
>> +        return AVERROR_DECODER_NOT_FOUND;
>>
>>     if (WelsCreateDecoder(&s->decoder)) {
>>         av_log(avctx, AV_LOG_ERROR, "Unable to create decoder\n");
>> diff --git a/libavcodec/libopenh264enc.c b/libavcodec/libopenh264enc.c
>> index 7c0501a2eb..7649e7b025 100644
>> --- a/libavcodec/libopenh264enc.c
>> +++ b/libavcodec/libopenh264enc.c
>> @@ -137,7 +137,7 @@ static av_cold int svc_encode_init(AVCodecContext 
>> *avctx)
>>     AVCPBProperties *props;
>>
>>     if ((err = ff_libopenh264_check_version(avctx)) < 0)
>> -        return err;
>> +        return AVERROR_ENCODER_NOT_FOUND;
>>
>>     if (WelsCreateSVCEncoder(&s->encoder)) {
>>         av_log(avctx, AV_LOG_ERROR, "Unable to create encoder\n");
>> -- 
>> 2.35.1
>
> This looks reasonable to me, so I could push this in a little while if 
> there's no more comments on it.
>
> But the patch lacks an explanation of _why_ this is done, in addition to 
> _what_ it does. I presume that's because the current error code makes some 
> decoder/encoder selection logic error out entirely, instead of continuing 
> trying some other codec - is that right? That would really be valuable to 
> include in the commit message.

Actually, I don't see anything in the code where returning 
AVERROR_DECODER_NOT_FOUND would behave differently than returning 
AVERROR(EINVAL) (as I would say it does right now).

So what's the motive for this patch, what does it change in practice?

I guess it changes what error message is printed (which certainly makes 
sense to change), but it would be very relevant to know whether this is a 
cosmetic change or if it changes something functionally that I haven't 
found.

// Martin
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".