Re: [FFmpeg-devel] [PATCH] libvpxenc: add color-space option for vp9

2015-06-17 Thread James Zern
On Tue, Jun 16, 2015 at 10:58 PM, James Almer jamr...@gmail.com wrote:
 On 16/06/15 2:43 AM, James Zern wrote:
 the vp9 bitstream supports 8 values:
 unknown (default), bt601, bt709, smpte170, smpte240, bt2020, reserved
 and sRGB.
 ---
  doc/encoders.texi  | 12 
  libavcodec/libvpxenc.c | 22 ++
  2 files changed, 34 insertions(+)

 diff --git a/doc/encoders.texi b/doc/encoders.texi
 index 8b0ecb7..09c90c2 100644
 --- a/doc/encoders.texi
 +++ b/doc/encoders.texi
 @@ -1550,6 +1550,18 @@ Enable frame parallel decodability features.
  @item aq-mode
  Set adaptive quantization mode (0: off (default), 1: variance 2: 
 complexity, 3:
  cyclic refresh).
 +@item color-space
 +Set input color space.
 +@table @samp
 +@item unknown
 +@item bt601
 +@item bt709
 +@item smpte170
 +@item smpte240
 +@item bt2020
 +@item reserved
 +@item sRGB
 +@end table
  @end table

  @end table
 diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c
 index 28a0e14..10f16e7 100644
 --- a/libavcodec/libvpxenc.c
 +++ b/libavcodec/libvpxenc.c
 @@ -100,8 +100,12 @@ typedef struct VP8EncoderContext {
  int tile_rows;
  int frame_parallel;
  int aq_mode;
 +int colorspace;
  } VP8Context;

 +// VP9E_SET_COLOR_SPACE was added just prior to v1.4.0.
 +#define HAVE_VP9_COLORSPACE_CONTROL (VPX_ENCODER_ABI_VERSION  8)

 Why this custom define? the end result is not shorter and there are other 
 uses of VPX version
 defines on this same file, like VPX_IMAGE_ABI_VERSION.


Given the current setup it doesn't do much for clarity; dropped.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] libvpxenc: add color-space option for vp9

2015-06-16 Thread James Almer
On 16/06/15 2:43 AM, James Zern wrote:
 the vp9 bitstream supports 8 values:
 unknown (default), bt601, bt709, smpte170, smpte240, bt2020, reserved
 and sRGB.
 ---
  doc/encoders.texi  | 12 
  libavcodec/libvpxenc.c | 22 ++
  2 files changed, 34 insertions(+)
 
 diff --git a/doc/encoders.texi b/doc/encoders.texi
 index 8b0ecb7..09c90c2 100644
 --- a/doc/encoders.texi
 +++ b/doc/encoders.texi
 @@ -1550,6 +1550,18 @@ Enable frame parallel decodability features.
  @item aq-mode
  Set adaptive quantization mode (0: off (default), 1: variance 2: complexity, 
 3:
  cyclic refresh).
 +@item color-space
 +Set input color space.
 +@table @samp
 +@item unknown
 +@item bt601
 +@item bt709
 +@item smpte170
 +@item smpte240
 +@item bt2020
 +@item reserved
 +@item sRGB
 +@end table
  @end table
  
  @end table
 diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c
 index 28a0e14..10f16e7 100644
 --- a/libavcodec/libvpxenc.c
 +++ b/libavcodec/libvpxenc.c
 @@ -100,8 +100,12 @@ typedef struct VP8EncoderContext {
  int tile_rows;
  int frame_parallel;
  int aq_mode;
 +int colorspace;
  } VP8Context;
  
 +// VP9E_SET_COLOR_SPACE was added just prior to v1.4.0.
 +#define HAVE_VP9_COLORSPACE_CONTROL (VPX_ENCODER_ABI_VERSION  8)

Why this custom define? the end result is not shorter and there are other uses 
of VPX version
defines on this same file, like VPX_IMAGE_ABI_VERSION.

 +
  /** String mappings for enum vp8e_enc_control_id */
  static const char *const ctlidstr[] = {
  [VP8E_UPD_ENTROPY]   = VP8E_UPD_ENTROPY,
 @@ -128,6 +132,9 @@ static const char *const ctlidstr[] = {
  [VP9E_SET_TILE_ROWS]   = VP9E_SET_TILE_ROWS,
  [VP9E_SET_FRAME_PARALLEL_DECODING] = VP9E_SET_FRAME_PARALLEL_DECODING,
  [VP9E_SET_AQ_MODE] = VP9E_SET_AQ_MODE,
 +#if HAVE_VP9_COLORSPACE_CONTROL
 +[VP9E_SET_COLOR_SPACE] = VP9E_SET_COLOR_SPACE,
 +#endif
  #endif
  };
  
 @@ -593,6 +600,10 @@ static av_cold int vpx_init(AVCodecContext *avctx,
  codecctl_int(avctx, VP9E_SET_FRAME_PARALLEL_DECODING, 
 ctx-frame_parallel);
  if (ctx-aq_mode = 0)
  codecctl_int(avctx, VP9E_SET_AQ_MODE, ctx-aq_mode);
 +#if HAVE_VP9_COLORSPACE_CONTROL
 +if (ctx-colorspace = 0)
 +codecctl_int(avctx, VP9E_SET_COLOR_SPACE, ctx-colorspace);
 +#endif
  }
  #endif
  
 @@ -968,6 +979,17 @@ static const AVOption vp9_options[] = {
  { variance,Variance based Aq,   0, AV_OPT_TYPE_CONST, {.i64 
 = 1}, 0, 0, VE, aq_mode },
  { complexity,  Complexity based Aq, 0, AV_OPT_TYPE_CONST, {.i64 
 = 2}, 0, 0, VE, aq_mode },
  { cyclic,  Cyclic Refresh Aq,   0, AV_OPT_TYPE_CONST, {.i64 
 = 3}, 0, 0, VE, aq_mode },
 +#if HAVE_VP9_COLORSPACE_CONTROL
 +{ color-space, Input color space,   
 OFFSET(colorspace),  AV_OPT_TYPE_INT, {.i64 = -1}, -1, 7, VE, 
 colorspace},
 +{ unknown, NULL, 0, AV_OPT_TYPE_CONST, {.i64 = 
 VPX_CS_UNKNOWN},   0, 0, VE, colorspace },
 +{ bt601,   NULL, 0, AV_OPT_TYPE_CONST, {.i64 = VPX_CS_BT_601}, 
0, 0, VE, colorspace },
 +{ bt709,   NULL, 0, AV_OPT_TYPE_CONST, {.i64 = VPX_CS_BT_709}, 
0, 0, VE, colorspace },
 +{ smpte170,NULL, 0, AV_OPT_TYPE_CONST, {.i64 = 
 VPX_CS_SMPTE_170}, 0, 0, VE, colorspace },
 +{ smpte240,NULL, 0, AV_OPT_TYPE_CONST, {.i64 = 
 VPX_CS_SMPTE_240}, 0, 0, VE, colorspace },
 +{ bt2020,  NULL, 0, AV_OPT_TYPE_CONST, {.i64 = 
 VPX_CS_BT_2020},   0, 0, VE, colorspace },
 +{ reserved,NULL, 0, AV_OPT_TYPE_CONST, {.i64 = 
 VPX_CS_RESERVED},  0, 0, VE, colorspace },
 +{ sRGB,NULL, 0, AV_OPT_TYPE_CONST, {.i64 = VPX_CS_SRGB},   
0, 0, VE, colorspace },
 +#endif
  LEGACY_OPTIONS
  { NULL }
  };
 

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


Re: [FFmpeg-devel] [PATCH] libvpxenc: add color-space option for vp9

2015-06-16 Thread Michael Niedermayer
On Mon, Jun 15, 2015 at 10:43:17PM -0700, James Zern wrote:
 the vp9 bitstream supports 8 values:
 unknown (default), bt601, bt709, smpte170, smpte240, bt2020, reserved
 and sRGB.
 ---
  doc/encoders.texi  | 12 
  libavcodec/libvpxenc.c | 22 ++
  2 files changed, 34 insertions(+)
 
 diff --git a/doc/encoders.texi b/doc/encoders.texi
 index 8b0ecb7..09c90c2 100644
 --- a/doc/encoders.texi
 +++ b/doc/encoders.texi
 @@ -1550,6 +1550,18 @@ Enable frame parallel decodability features.
  @item aq-mode
  Set adaptive quantization mode (0: off (default), 1: variance 2: complexity, 
 3:
  cyclic refresh).
 +@item color-space
 +Set input color space.
 +@table @samp
 +@item unknown
 +@item bt601
 +@item bt709
 +@item smpte170
 +@item smpte240
 +@item bt2020
 +@item reserved
 +@item sRGB
 +@end table
  @end table
  
  @end table
 diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c
 index 28a0e14..10f16e7 100644
 --- a/libavcodec/libvpxenc.c
 +++ b/libavcodec/libvpxenc.c
 @@ -100,8 +100,12 @@ typedef struct VP8EncoderContext {
  int tile_rows;
  int frame_parallel;
  int aq_mode;
 +int colorspace;
  } VP8Context;
  
 +// VP9E_SET_COLOR_SPACE was added just prior to v1.4.0.
 +#define HAVE_VP9_COLORSPACE_CONTROL (VPX_ENCODER_ABI_VERSION  8)
 +
  /** String mappings for enum vp8e_enc_control_id */
  static const char *const ctlidstr[] = {
  [VP8E_UPD_ENTROPY]   = VP8E_UPD_ENTROPY,
 @@ -128,6 +132,9 @@ static const char *const ctlidstr[] = {
  [VP9E_SET_TILE_ROWS]   = VP9E_SET_TILE_ROWS,
  [VP9E_SET_FRAME_PARALLEL_DECODING] = VP9E_SET_FRAME_PARALLEL_DECODING,
  [VP9E_SET_AQ_MODE] = VP9E_SET_AQ_MODE,
 +#if HAVE_VP9_COLORSPACE_CONTROL
 +[VP9E_SET_COLOR_SPACE] = VP9E_SET_COLOR_SPACE,
 +#endif
  #endif
  };
  
 @@ -593,6 +600,10 @@ static av_cold int vpx_init(AVCodecContext *avctx,
  codecctl_int(avctx, VP9E_SET_FRAME_PARALLEL_DECODING, 
 ctx-frame_parallel);
  if (ctx-aq_mode = 0)
  codecctl_int(avctx, VP9E_SET_AQ_MODE, ctx-aq_mode);
 +#if HAVE_VP9_COLORSPACE_CONTROL
 +if (ctx-colorspace = 0)
 +codecctl_int(avctx, VP9E_SET_COLOR_SPACE, ctx-colorspace);
 +#endif
  }
  #endif
  
 @@ -968,6 +979,17 @@ static const AVOption vp9_options[] = {
  { variance,Variance based Aq,   0, AV_OPT_TYPE_CONST, {.i64 
 = 1}, 0, 0, VE, aq_mode },
  { complexity,  Complexity based Aq, 0, AV_OPT_TYPE_CONST, {.i64 
 = 2}, 0, 0, VE, aq_mode },
  { cyclic,  Cyclic Refresh Aq,   0, AV_OPT_TYPE_CONST, {.i64 
 = 3}, 0, 0, VE, aq_mode },
 +#if HAVE_VP9_COLORSPACE_CONTROL
 +{ color-space, Input color space,   
 OFFSET(colorspace),  AV_OPT_TYPE_INT, {.i64 = -1}, -1, 7, VE, 
 colorspace},
 +{ unknown, NULL, 0, AV_OPT_TYPE_CONST, {.i64 = 
 VPX_CS_UNKNOWN},   0, 0, VE, colorspace },
 +{ bt601,   NULL, 0, AV_OPT_TYPE_CONST, {.i64 = VPX_CS_BT_601}, 
0, 0, VE, colorspace },
 +{ bt709,   NULL, 0, AV_OPT_TYPE_CONST, {.i64 = VPX_CS_BT_709}, 
0, 0, VE, colorspace },
 +{ smpte170,NULL, 0, AV_OPT_TYPE_CONST, {.i64 = 
 VPX_CS_SMPTE_170}, 0, 0, VE, colorspace },
 +{ smpte240,NULL, 0, AV_OPT_TYPE_CONST, {.i64 = 
 VPX_CS_SMPTE_240}, 0, 0, VE, colorspace },
 +{ bt2020,  NULL, 0, AV_OPT_TYPE_CONST, {.i64 = 
 VPX_CS_BT_2020},   0, 0, VE, colorspace },
 +{ reserved,NULL, 0, AV_OPT_TYPE_CONST, {.i64 = 
 VPX_CS_RESERVED},  0, 0, VE, colorspace },
 +{ sRGB,NULL, 0, AV_OPT_TYPE_CONST, {.i64 = VPX_CS_SRGB},   
0, 0, VE, colorspace },
 +#endif

why does this not use AVCodecContext.colorspace or other colorspace
related fields ?


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

Its not that you shouldnt use gotos but rather that you should write
readable code and code with gotos often but not always is less readable


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