[FFmpeg-devel] [PATCH] lavu/opt: change the way default pixel and sample format value is set

Michael Niedermayer michaelni at gmx.at
Tue Nov 27 16:18:46 CET 2012


On Tue, Nov 27, 2012 at 01:04:08PM +0100, Stefano Sabatini wrote:
> On date Sunday 2012-11-25 17:49:05 +0100, Michael Niedermayer encoded:
> > On Sun, Nov 25, 2012 at 04:34:48PM +0100, Stefano Sabatini wrote:
> [...]
> > > This is technically an API break, but hopefully there are no applications
> > > using this feature outside of FFmpeg.
> > 
> > what about a system that has mixed libs ? for example one updates
> > libavutil but leaves libavfilter and ffmpeg at old revissions.
> > This can happen with binary distributions that have the libs in
> > seperate packages.
> > 
> > maybe you could check the version field from the AVClass in the 2
> > cases that touch it
> > 
> > otherwise LGTM
> 
> Updated.
> -- 
> FFmpeg = Fostering & Fundamental Mind-dumbing Portable Elastic Gladiator

>  libavcodec/options_table.h |    2 +-
>  libavdevice/dshow.c        |    2 +-
>  libavutil/opt.c            |   23 ++++++++++++++++++++---
>  libavutil/version.h        |    2 +-
>  4 files changed, 23 insertions(+), 6 deletions(-)
> a781d8c852ed2e4bc7ac97dcf0bb9ff9317a8f07  0002-lavu-opt-change-the-way-default-pixel-and-sample-for.patch
> From 95f8250502bbb4db5ea19d7568503b81540628a9 Mon Sep 17 00:00:00 2001
> From: Stefano Sabatini <stefasab at gmail.com>
> Date: Sun, 25 Nov 2012 15:45:58 +0100
> Subject: [PATCH] lavu/opt: change the way default pixel and sample format
>  value is set
> 
> Use the i64 field rather than the string value. Using a string to set a
> default sample/pixel format is weird, also the new interface is more
> consistent with the rest of the API.
> 
> This is technically an API break, but hopefully there are no applications
> using this feature outside of FFmpeg. In order to save backward
> compatibility with mixed libraries in case libavutil is updated but not
> the other libraries, some ifdeffery hacks are added.
> ---
>  libavcodec/options_table.h |    2 +-
>  libavdevice/dshow.c        |    2 +-
>  libavutil/opt.c            |   23 ++++++++++++++++++++---
>  libavutil/version.h        |    2 +-
>  4 files changed, 23 insertions(+), 6 deletions(-)
> 
> diff --git a/libavcodec/options_table.h b/libavcodec/options_table.h
> index 2ddfad0..411092e 100644
> --- a/libavcodec/options_table.h
> +++ b/libavcodec/options_table.h
> @@ -401,7 +401,7 @@ static const AVOption options[]={
>  {"em", "Emergency",          0, AV_OPT_TYPE_CONST, {.i64 = AV_AUDIO_SERVICE_TYPE_EMERGENCY },         INT_MIN, INT_MAX, A|E, "audio_service_type"},
>  {"vo", "Voice Over",         0, AV_OPT_TYPE_CONST, {.i64 = AV_AUDIO_SERVICE_TYPE_VOICE_OVER },        INT_MIN, INT_MAX, A|E, "audio_service_type"},
>  {"ka", "Karaoke",            0, AV_OPT_TYPE_CONST, {.i64 = AV_AUDIO_SERVICE_TYPE_KARAOKE },           INT_MIN, INT_MAX, A|E, "audio_service_type"},
> -{"request_sample_fmt", "sample format audio decoders should prefer", OFFSET(request_sample_fmt), AV_OPT_TYPE_SAMPLE_FMT, {.str = NULL }, 0, 0, A|D, "request_sample_fmt"},
> +{"request_sample_fmt", "sample format audio decoders should prefer", OFFSET(request_sample_fmt), AV_OPT_TYPE_SAMPLE_FMT, {.i64=AV_SAMPLE_FMT_NONE}, 0, 0, A|D, "request_sample_fmt"},
>  {"pkt_timebase", NULL, OFFSET(pkt_timebase), AV_OPT_TYPE_RATIONAL, {.dbl = 0 }, 0, INT_MAX, 0},
>  {NULL},
>  };
> diff --git a/libavdevice/dshow.c b/libavdevice/dshow.c
> index 3bd90b0..26a4f15 100644
> --- a/libavdevice/dshow.c
> +++ b/libavdevice/dshow.c
> @@ -1018,7 +1018,7 @@ static int dshow_read_packet(AVFormatContext *s, AVPacket *pkt)
>  #define DEC AV_OPT_FLAG_DECODING_PARAM
>  static const AVOption options[] = {
>      { "video_size", "set video size given a string such as 640x480 or hd720.", OFFSET(requested_width), AV_OPT_TYPE_IMAGE_SIZE, {.str = NULL}, 0, 0, DEC },
> -    { "pixel_format", "set video pixel format", OFFSET(pixel_format), AV_OPT_TYPE_PIXEL_FMT, {.str = NULL}, 0, 0, DEC },
> +    { "pixel_format", "set video pixel format", OFFSET(pixel_format), AV_OPT_TYPE_PIXEL_FMT, {.i64 = AV_PIX_FMT_NONE}, 0, 0, DEC },
>      { "framerate", "set video frame rate", OFFSET(framerate), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, DEC },
>      { "sample_rate", "set audio sample rate", OFFSET(sample_rate), AV_OPT_TYPE_INT, {.i64 = 0}, 0, INT_MAX, DEC },
>      { "sample_size", "set audio sample size", OFFSET(sample_size), AV_OPT_TYPE_INT, {.i64 = 0}, 0, 16, DEC },
> diff --git a/libavutil/opt.c b/libavutil/opt.c
> index b9da676..1f294ae 100644
> --- a/libavutil/opt.c
> +++ b/libavutil/opt.c
> @@ -813,6 +813,7 @@ void av_opt_set_defaults(void *s)
>  void av_opt_set_defaults2(void *s, int mask, int flags)
>  {
>  #endif
> +    const AVClass *class = (AVClass *)s;
>      const AVOption *opt = NULL;
>      while ((opt = av_opt_next(s, opt)) != NULL) {
>  #if FF_API_OLD_AVOPTIONS
> @@ -843,9 +844,25 @@ void av_opt_set_defaults2(void *s, int mask, int flags)
>              break;
>              case AV_OPT_TYPE_STRING:
>              case AV_OPT_TYPE_IMAGE_SIZE:
> +                av_opt_set(s, opt->name, opt->default_val.str, 0);
> +                break;
>              case AV_OPT_TYPE_PIXEL_FMT:
> +#if LIBAVUTIL_VERSION_MAJOR < 53
> +                if (class->version < AV_VERSION_INT(52, 9, 103))
> +                    av_opt_set(s, opt->name, opt->default_val.str, 0);
> +                else
> +#else
> +                    av_opt_set_pixel_fmt(s, opt->name, opt->default_val.i64, 0);
> +#endif
> +                break;
>              case AV_OPT_TYPE_SAMPLE_FMT:
> -                av_opt_set(s, opt->name, opt->default_val.str, 0);
> +#if LIBAVUTIL_VERSION_MAJOR < 53
> +                if (class->version < AV_VERSION_INT(52, 9, 103))
> +                    av_opt_set(s, opt->name, opt->default_val.str, 0);
> +                else
> +#else
> +                    av_opt_set_sample_fmt(s, opt->name, opt->default_val.i64, 0);
> +#endif
>                  break;
>              case AV_OPT_TYPE_BINARY:
>                  /* Cannot set default for binary */
> @@ -1164,8 +1181,8 @@ static const AVOption test_options[]= {
>  {"lame",     "set lame flag ", 0,                AV_OPT_TYPE_CONST,    {.i64 = TEST_FLAG_LAME}, INT_MIN,  INT_MAX, 0, "flags" },
>  {"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                   },
> +{"pix_fmt",  "set pixfmt",     OFFSET(pix_fmt),  AV_OPT_TYPE_PIXEL_FMT, {.i64 = AV_PIX_FMT_NONE}},
> +{"sample_fmt", "set samplefmt", OFFSET(sample_fmt), AV_OPT_TYPE_SAMPLE_FMT, {.i64 = AV_SAMPLE_FMT_NONE}},
>  {NULL},
>  };
>  
> diff --git a/libavutil/version.h b/libavutil/version.h
> index af60f71..26a8178 100644
> --- a/libavutil/version.h
> +++ b/libavutil/version.h
> @@ -76,7 +76,7 @@
>  
>  #define LIBAVUTIL_VERSION_MAJOR  52
>  #define LIBAVUTIL_VERSION_MINOR   9
> -#define LIBAVUTIL_VERSION_MICRO 102
> +#define LIBAVUTIL_VERSION_MICRO 103

consider someone upgrades libavcodec but not libavutil
this definitly needs a minor bump in libavutil and i would also suggest
to minor or micro bump all other libs that change the format

all libs that update the format must depend on the new libavutil
version, so a new libavcodec with old libavutil cant happen
this needs libavutil to bump minor

otherwise LGTM

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Let us carefully observe those good qualities wherein our enemies excel us
and endeavor to excel them, by avoiding what is faulty, and imitating what
is excellent in them. -- Plutarch
-------------- 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/20121127/a9c99eed/attachment.asc>


More information about the ffmpeg-devel mailing list