[FFmpeg-devel] [PATCH] Add support for vp9 in iso-bmff

[Ronald S. Bultje]

Hi,

On Sat, Jun 11, 2016 at 11:48 AM, James Almer <jamrial at gmail.com> wrote:

> > +enum VpxColorSpace
> > +{
> > +    VPX_COLOR_SPACE_UNSPECIFIED = 0,
> > +    VPX_COLOR_SPACE_BT601 = 1,
> > +    VPX_COLOR_SPACE_BT709 = 2,
> > +    VPX_COLOR_SPACE_SMPTE_170 = 3,
> > +    VPX_COLOR_SPACE_SMPTE_240 = 4,
> > +    VPX_COLOR_SPACE_BT2020_NCL = 5,
> > +    VPX_COLOR_SPACE_BT2020_CL = 6,
> > +    VPX_COLOR_SPACE_RGB = 7,
> > +};
> > +
> > +static int get_vpx_color_space(enum AVColorSpace color_space)
> > +{
> > +    switch (color_space) {
> > +        case AVCOL_SPC_RGB:
> > +            return VPX_COLOR_SPACE_RGB;
> > +        case AVCOL_SPC_BT709:
> > +            return VPX_COLOR_SPACE_BT709;
> > +        case AVCOL_SPC_SMPTE170M:
> > +            return VPX_COLOR_SPACE_SMPTE_170;
> > +        case AVCOL_SPC_SMPTE240M:
> > +            return VPX_COLOR_SPACE_SMPTE_240;
> > +        case AVCOL_SPC_BT2020_NCL:
> > +            return VPX_COLOR_SPACE_BT2020_NCL;
> > +        case AVCOL_SPC_BT2020_CL:
> > +            return VPX_COLOR_SPACE_BT2020_CL;
> > +        default:
> > +            return VPX_COLOR_SPACE_UNSPECIFIED;
> > +    }
> > +}
>
> Isn't the above duplicating code or structs from libavcodec? (either the
> native vp9 decoder or the libvpx wrapper). If so, maybe it could be shared.
>
> Same probably also goes for most of the stuff below.
>

I don't think so.


> > +static int get_bit_depth(enum AVPixelFormat pixel_format)
> > +{
> > +  switch (pixel_format) {
> > +      case AV_PIX_FMT_YUV420P:
> > +      case AV_PIX_FMT_YUV422P:
> > +      case AV_PIX_FMT_YUV444P:
> > +          return 8;
> > +      case AV_PIX_FMT_YUV420P10LE:
> > +      case AV_PIX_FMT_YUV420P10BE:
> > +      case AV_PIX_FMT_YUV422P10LE:
> > +      case AV_PIX_FMT_YUV422P10BE:
> > +      case AV_PIX_FMT_YUV444P10LE:
> > +      case AV_PIX_FMT_YUV444P10BE:
> > +          return 10;
> > +      case AV_PIX_FMT_YUV420P12LE:
> > +      case AV_PIX_FMT_YUV420P12BE:
> > +      case AV_PIX_FMT_YUV422P12LE:
> > +      case AV_PIX_FMT_YUV422P12BE:
> > +      case AV_PIX_FMT_YUV444P12LE:
> > +      case AV_PIX_FMT_YUV444P12BE:
> > +          return 12;
> > +      default:
> > +          // Defaults to 8.
> > +          av_log(NULL, AV_LOG_WARNING,
> > +                 "Unspecified or unrecognized bit depth. Assuming 8.");
> > +          return 8;
> > +  }
> > +}
>
> I'm fairly sure this is duplicating libavutil functionality. A function
> returning bit depth based on a AVPixelFormat argument sounds generic
> enough that plenty modules would use.


av_pix_fmt_desc_get(pixel_format)->comp[0].depth

Ronald