Hi,
On Mon, Jun 13, 2016 at 5:26 PM, Kongqun Yang <yangkong...@gmail.com> wrote:
>
> @@ -5369,6 +5384,17 @@ static int mov_write_header(AVFormatContext *s)
> pix_fmt == AV_PIX_FMT_MONOWHITE ||
> pix_fmt == AV_PIX_FMT_MONOBLACK;
> }
> + if (track->mode == MODE_MP4 &&
> + track->par->codec_id == AV_CODEC_ID_VP9) {
> + if (s->strict_std_compliance > FF_COMPLIANCE_EXPERIMENTAL) {
> + av_log(s, AV_LOG_ERROR,
> + "VP9 in MP4 support is experimental, add "
> + "'-strict %d' if you want to use it.\n",
> + FF_COMPLIANCE_EXPERIMENTAL);
> + ret = AVERROR_EXPERIMENTAL;
> + goto error;
> + }
> + }
>
Please use 4-space indentation (you're using 2).
> +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;
> + }
> +}
>
[..]
> +static int get_vpx_chroma_subsampling(enum AVPixelFormat pixel_format,
> + enum AVChromaLocation
> chroma_location)
> +{
> + switch (pixel_format) {
> + case AV_PIX_FMT_YUV420P:
> + case AV_PIX_FMT_YUV420P10LE:
> + case AV_PIX_FMT_YUV420P10BE:
> + case AV_PIX_FMT_YUV420P12LE:
> + case AV_PIX_FMT_YUV420P12BE:
> + if (chroma_location == AVCHROMA_LOC_LEFT)
> + return VPX_SUBSAMPLING_420_VERTICAL;
> + // Otherwise assume collocated.
> + return VPX_SUBSAMPLING_420_COLLOCATED_WITH_LUMA;
> + case AV_PIX_FMT_YUV422P:
> + case AV_PIX_FMT_YUV422P10LE:
> + case AV_PIX_FMT_YUV422P10BE:
> + case AV_PIX_FMT_YUV422P12LE:
> + case AV_PIX_FMT_YUV422P12BE:
> + return VPX_SUBSAMPLING_422;
> + case AV_PIX_FMT_YUV444P:
> + case AV_PIX_FMT_YUV444P10LE:
> + case AV_PIX_FMT_YUV444P10BE:
> + case AV_PIX_FMT_YUV444P12LE:
> + case AV_PIX_FMT_YUV444P12BE:
> + return VPX_SUBSAMPLING_444;
> + default:
> + av_log(NULL, AV_LOG_ERROR, "Unknown pixel format.");
> + return -1;
> + }
> +}
>
In ffmpeg, the case and switch should be indented at the same level:
switch (a) {
case b:
foo();
break;
default:
bar();
break;
}
What happens if the "default:" case is reached? Is the vpcc atom still
valid? If not, should the muxer error out? Can this even happen at all?
+static int get_vpx_transfer_function(
> + enum AVColorTransferCharacteristic transfer)
> +{
> + return (transfer == AVCOL_TRC_SMPTEST2084) ? 1 : 0;
> +}
> +
> +static int get_vpx_video_full_range_flag(enum AVColorRange color_range)
> +{
> + return (color_range == AVCOL_RANGE_JPEG) ? 1 : 0;
> +}
>
The (..) ? 1 : 0 construct is unnecessary, you can just return .. (or in
case of non-boolean values, we prefer !!(..)).
I also think it's a bad habit to log generic messages like "unknown pixel
format" without a logging context, the user will never understand what that
means.
Ronald
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
