[FFmpeg-devel] [PATCH] lavu/opt: add AV_OPT_SAMPLE_FMT option

[Stefano Sabatini]

On date Friday 2012-11-02 15:06:55 +0100, Nicolas George encoded:
> Le duodi 12 brumaire, an CCXXI, Stefano Sabatini a écrit :
> > TODO: bump minor, add APIchanges entry
> > ---
> >  libavutil/opt.c |   28 ++++++++++++++++++++++++++++
> >  libavutil/opt.h |    1 +
> >  2 files changed, 29 insertions(+), 0 deletions(-)
> > 
> > diff --git a/libavutil/opt.c b/libavutil/opt.c
> > index ed475ec..c221cbc 100644
> > --- a/libavutil/opt.c
> > +++ b/libavutil/opt.c
> > @@ -35,6 +35,7 @@
> >  #include "parseutils.h"
> >  #include "pixdesc.h"
> >  #include "mathematics.h"
> > +#include "samplefmt.h"
> >  
> >  #if FF_API_FIND_OPT
> >  //FIXME order them and do a bin search
> > @@ -279,6 +280,22 @@ int av_opt_set(void *obj, const char *name, const char *val, int search_flags)
> >          }
> >          *(enum AVPixelFormat *)dst = ret;
> >          return 0;
> > +    case AV_OPT_TYPE_SAMPLE_FMT:
> > +        if (!val || !strcmp(val, "none"))
> > +            ret = AV_SAMPLE_FMT_NONE;
> > +        else {
> 
> Nit++: braces on the if, for symmetry.

Fixed.

> 
> > +            ret = av_get_sample_fmt(val);
> > +            if (ret == AV_SAMPLE_FMT_NONE) {
> > +                char *tail;
> > +                ret = strtol(val, &tail, 0);
> > +                if (*tail || (unsigned)ret >= AV_SAMPLE_FMT_NB) {
> > +                    av_log(obj, AV_LOG_ERROR, "Unable to parse option value \"%s\" as sample format\n", val);
> > +                    return AVERROR(EINVAL);
> > +                }
> 
> Maybe print a warning to say that numeric sample formats are deprecated.

No I believe they can be useful (e.g. with plain printf), also
consistent with AV_OPT_TYPE_PIXEL_FMT, can be changed later.

> > +            }
> > +        }
> > +        *(enum AVSampleFormat *)dst = ret;
> > +        return 0;
> >      }
> >  
> >      av_log(obj, AV_LOG_ERROR, "Invalid option type.\n");
> > @@ -467,6 +484,9 @@ int av_opt_get(void *obj, const char *name, int search_flags, uint8_t **out_val)
> >      case AV_OPT_TYPE_PIXEL_FMT:
> >          ret = snprintf(buf, sizeof(buf), "%s", (char *)av_x_if_null(av_get_pix_fmt_name(*(enum AVPixelFormat *)dst), "none"));
> >          break;
> > +    case AV_OPT_TYPE_SAMPLE_FMT:
> > +        ret = snprintf(buf, sizeof(buf), "%s", (char *)av_x_if_null(av_get_sample_fmt_name(*(enum AVSampleFormat *)dst), "none"));
> > +        break;
> >      default:
> >          return AVERROR(EINVAL);
> >      }
> > @@ -642,6 +662,9 @@ static void opt_list(void *obj, void *av_log_obj, const char *unit,
> >              case AV_OPT_TYPE_PIXEL_FMT:
> >                  av_log(av_log_obj, AV_LOG_INFO, "%-7s ", "<pix_fmt>");
> >                  break;
> > +            case AV_OPT_TYPE_SAMPLE_FMT:
> > +                av_log(av_log_obj, AV_LOG_INFO, "%-7s ", "<sample_fmt>");
> > +                break;
> >              case AV_OPT_TYPE_CONST:
> >              default:
> >                  av_log(av_log_obj, AV_LOG_INFO, "%-7s ", "");
> > @@ -992,6 +1015,7 @@ typedef struct TestContext
> >      AVRational rational;
> >      int w, h;
> >      enum AVPixelFormat pix_fmt;
> > +    enum AVSampleFormat sample_fmt;
> >  } TestContext;
> >  
> >  #define OFFSET(x) offsetof(TestContext, x)
> > @@ -1011,6 +1035,7 @@ static const AVOption test_options[]= {
> >  {"mu",       "set mu flag ",   0,                AV_OPT_TYPE_CONST,    {.i64 = TEST_FLAG_MU},   INT_MIN,  INT_MAX, 0, "flags" },
> >  {"size",     "set size",       OFFSET(w),        AV_OPT_TYPE_IMAGE_SIZE,{0},             0,        0                   },
> >  {"pix_fmt",  "set pixfmt",     OFFSET(pix_fmt),  AV_OPT_TYPE_PIXEL_FMT,{0},              0,        0                   },
> > +{"sample_fmt",  "set samplefmt", OFFSET(sample_fmt), AV_OPT_TYPE_SAMPLE_FMT,{0},         0,        0                   },
> >  {NULL},
> >  };
> >  
> > @@ -1058,6 +1083,9 @@ int main(void)
> >              "pix_fmt=yuv420p",
> >              "pix_fmt=2",
> >              "pix_fmt=bogus",
> > +            "sample_fmt=s16",
> > +            "sample_fmt=2",
> > +            "sample_fmt=bogus",
> >          };
> >  
> >          test_ctx.class = &test_class;
> > diff --git a/libavutil/opt.h b/libavutil/opt.h
> > index 81fefd9..5d2031a 100644
> > --- a/libavutil/opt.h
> > +++ b/libavutil/opt.h
> > @@ -227,6 +227,7 @@ enum AVOptionType{
> >      AV_OPT_TYPE_CONST = 128,
> >      AV_OPT_TYPE_IMAGE_SIZE = MKBETAG('S','I','Z','E'), ///< offset must point to two consecutive integers
> >      AV_OPT_TYPE_PIXEL_FMT  = MKBETAG('P','F','M','T'),
> > +    AV_OPT_TYPE_SAMPLE_FMT = MKBETAG('S','F','M','T'),
> >  #if FF_API_OLD_AVOPTIONS
> >      FF_OPT_TYPE_FLAGS = 0,
> >      FF_OPT_TYPE_INT,
> 
> Apart from those minor points, good idea, and I do not see any flaw. Same
> goes for the second patch, the one using the new feature.

Applied, thanks.
-- 
FFmpeg = Fiendish and Freak Mortal Practical EnGraver