[FFmpeg-devel] [PATCH] lavu/error: use a sorted table for storing error codes and strings, add test

Nicolas George nicolas.george at normalesup.org
Sun Jun 10 14:04:22 CEST 2012


Le quartidi 14 prairial, an CCXX, Stefano Sabatini a écrit :
> Updated.
> 
> Now I'm seeing if it would be possible to implement an av_err2str()
> macro (maybe using the same trick of av_time2str()?).
> -- 
> FFmpeg = Fundamental and Forgiving Multipurpose Powered Elected Guru

> >From 1dfb08f0c5a8ad6dc5c263d542c1b0870adc83b8 Mon Sep 17 00:00:00 2001
> From: Stefano Sabatini <stefasab at gmail.com>
> Date: Mon, 6 Feb 2012 14:16:33 +0100
> Subject: [PATCH] lavu/error: use a table for storing error codes and strings,
>  add test
> 
> The table is mostly useful for enumerating the available AVERROR* in the
> test output.
> ---
>  libavutil/Makefile |    1 +
>  libavutil/error.c  |   85 ++++++++++++++++++++++++++++++++++++---------------
>  2 files changed, 61 insertions(+), 25 deletions(-)
> 
> diff --git a/libavutil/Makefile b/libavutil/Makefile
> index a449831..e20f1a7 100644
> --- a/libavutil/Makefile
> +++ b/libavutil/Makefile
> @@ -94,6 +94,7 @@ TESTPROGS = adler32                                                     \
>              cpu                                                         \
>              crc                                                         \
>              des                                                         \
> +            error                                                       \
>              eval                                                        \
>              file                                                        \
>              fifo                                                        \
> diff --git a/libavutil/error.c b/libavutil/error.c
> index 8f725d7..b1f8be5 100644
> --- a/libavutil/error.c
> +++ b/libavutil/error.c
> @@ -20,34 +20,44 @@
>  #include "avutil.h"
>  #include "avstring.h"
>  
> +struct error_entry {
> +    int num;
> +    const char *tag;
> +    const char *str;
> +};
> +
> +#define ERROR_TAG(tag) AVERROR_##tag, #tag
> +struct error_entry error_entries[] = {
> +    { ERROR_TAG(BSF_NOT_FOUND),      "Bitstream filter not found"                     },
> +    { ERROR_TAG(BUG),                "Internal bug, should not have happened"         },
> +    { ERROR_TAG(BUG2),               "Internal bug, should not have happened"         },
> +    { ERROR_TAG(DECODER_NOT_FOUND),  "Decoder not found"                              },
> +    { ERROR_TAG(DEMUXER_NOT_FOUND),  "Demuxer not found"                              },
> +    { ERROR_TAG(ENCODER_NOT_FOUND),  "Encoder not found"                              },
> +    { ERROR_TAG(EOF),                "End of file"                                    },
> +    { ERROR_TAG(EXIT),               "Immediate exit requested"                       },
> +    { ERROR_TAG(FILTER_NOT_FOUND),   "Filter not found"                               },
> +    { ERROR_TAG(INVALIDDATA),        "Invalid data found when processing input"       },
> +    { ERROR_TAG(MUXER_NOT_FOUND),    "Muxer not found"                                },
> +    { ERROR_TAG(OPTION_NOT_FOUND),   "Option not found"                               },
> +    { ERROR_TAG(PATCHWELCOME),       "Not yet implemented in FFmpeg, patches welcome" },
> +    { ERROR_TAG(PROTOCOL_NOT_FOUND), "Protocol not found"                             },
> +    { ERROR_TAG(STREAM_NOT_FOUND),   "Stream not found"                               },
> +};
> +
>  int av_strerror(int errnum, char *errbuf, size_t errbuf_size)
>  {
> -    int ret = 0;
> -    const char *errstr = NULL;
> -
> -    switch (errnum) {
> -    case AVERROR_BSF_NOT_FOUND:     errstr = "Bitstream filter not found"                   ; break;
> -    case AVERROR_BUG2:
> -    case AVERROR_BUG:               errstr = "Internal bug, should not have happened"       ; break;
> -    case AVERROR_DECODER_NOT_FOUND: errstr = "Decoder not found"                            ; break;
> -    case AVERROR_DEMUXER_NOT_FOUND: errstr = "Demuxer not found"                            ; break;
> -    case AVERROR_ENCODER_NOT_FOUND: errstr = "Encoder not found"                            ; break;
> -    case AVERROR_EOF:               errstr = "End of file"                                  ; break;
> -    case AVERROR_EXIT:              errstr = "Immediate exit requested"                     ; break;
> -    case AVERROR_FILTER_NOT_FOUND:  errstr = "Filter not found"                             ; break;
> -    case AVERROR_INVALIDDATA:       errstr = "Invalid data found when processing input"     ; break;
> -    case AVERROR_MUXER_NOT_FOUND:   errstr = "Muxer not found"                              ; break;
> -    case AVERROR_OPTION_NOT_FOUND:  errstr = "Option not found"                             ; break;
> -    case AVERROR_PATCHWELCOME:      errstr = "Not yet implemented in FFmpeg, patches welcome"; break;
> -    case AVERROR_PROTOCOL_NOT_FOUND:errstr = "Protocol not found"                           ; break;
> -    case AVERROR_STREAM_NOT_FOUND:  errstr = "Stream not found"                             ; break;
> -    case AVERROR_UNKNOWN:           errstr = "Unknown error occurred"                       ; break;
> -    case AVERROR(EINVAL):           errstr = "Invalid argument"                             ; break;

You are losing that case. Michael added it because tests/ref/fate/parseutils
relies on its exact wording. IMHO, it would be better to alter the
parseutils test to avoid that.

> -    case 0:                         errstr = "Success"                                      ; break;
> -    }
> +    int ret = 0, i;
> +    struct error_entry *entry = NULL;
>  
> -    if (errstr) {
> -        av_strlcpy(errbuf, errstr, errbuf_size);
> +    for (i = 0; i < FF_ARRAY_ELEMS(error_entries); i++) {
> +        if (errnum == error_entries[i].num) {
> +            entry = &error_entries[i];
> +            break;
> +        }
> +    }
> +    if (entry) {
> +        av_strlcpy(errbuf, entry->str, errbuf_size);

This part looks good to me.

>      } else {
>  #if HAVE_STRERROR_R
>          ret = strerror_r(AVUNERROR(errnum), errbuf, errbuf_size);
> @@ -60,3 +70,28 @@ int av_strerror(int errnum, char *errbuf, size_t errbuf_size)
>  
>      return ret;
>  }
> +
> +#ifdef TEST
> +
> +#undef printf
> +
> +int main(void)
> +{
> +    int i;
> +    char errbuf[256];
> +
> +    for (i = 0; i < FF_ARRAY_ELEMS(error_entries); i++) {
> +        struct error_entry *entry = &error_entries[i];
> +        av_strerror(entry->num, errbuf, sizeof(errbuf));
> +        printf("%d: %s [%s]\n", entry->num, errbuf, entry->tag);
> +    }
> +
> +    for (i = 0; i < 256; i++) {
> +        av_strerror(-i, errbuf, sizeof(errbuf));
> +        printf("%d: %s\n", -i, errbuf);
> +    }
> +
> +    return 0;
> +}
> +
> +#endif /* TEST */

I do not really see the point of the test in that case: we will not be able
to include it in regression testing anyway since it relies on
system-dependant errno values.

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120610/1c32d0c8/attachment.asc>


More information about the ffmpeg-devel mailing list