On 1/19/2018 9:36 PM, Michael Niedermayer wrote: > On Fri, Jan 19, 2018 at 09:33:46PM -0300, James Almer wrote: >> On 1/19/2018 8:56 PM, Michael Niedermayer wrote: >>> On Fri, Jan 19, 2018 at 12:51:21AM -0300, James Almer wrote: >>>> Improves remuxing support when vp9 decoder is not compiled in. >>>> >>>> Signed-off-by: James Almer <jamr...@gmail.com> >>>> --- >>>> libavcodec/vp9_parser.c | 98 >>>> ++++++++++++++++++++++++++++++++++++++++++++++++- >>>> 1 file changed, 97 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/libavcodec/vp9_parser.c b/libavcodec/vp9_parser.c >>>> index 9531f34a32..b059477747 100644 >>>> --- a/libavcodec/vp9_parser.c >>>> +++ b/libavcodec/vp9_parser.c >>>> @@ -23,15 +23,69 @@ >>>> >>>> #include "libavutil/intreadwrite.h" >>>> #include "libavcodec/get_bits.h" >>>> +#include "libavcodec/internal.h" >>>> #include "parser.h" >>>> >>>> +#define VP9_SYNCCODE 0x498342 >>>> + >>>> +static int read_colorspace_details(AVCodecParserContext *ctx, >>>> AVCodecContext *avctx, >>>> + GetBitContext *gb) >>>> +{ >>>> + static const enum AVColorSpace colorspaces[8] = { >>>> + AVCOL_SPC_UNSPECIFIED, AVCOL_SPC_BT470BG, AVCOL_SPC_BT709, >>>> AVCOL_SPC_SMPTE170M, >>>> + AVCOL_SPC_SMPTE240M, AVCOL_SPC_BT2020_NCL, AVCOL_SPC_RESERVED, >>>> AVCOL_SPC_RGB, >>>> + }; >>>> + int bits = avctx->profile <= 1 ? 0 : 1 + get_bits1(gb); // 0:8, 1:10, >>>> 2:12 >>>> + >>>> + avctx->colorspace = colorspaces[get_bits(gb, 3)]; >>>> + if (avctx->colorspace == AVCOL_SPC_RGB) { // RGB = profile 1 >>>> + static const enum AVPixelFormat pix_fmt_rgb[3] = { >>>> + AV_PIX_FMT_GBRP, AV_PIX_FMT_GBRP10, AV_PIX_FMT_GBRP12 >>>> + }; >>>> + if (avctx->profile & 1) { >>>> + if (get_bits1(gb)) // reserved bit >>>> + return AVERROR_INVALIDDATA; >>>> + } else >>>> + return AVERROR_INVALIDDATA; >>>> + avctx->color_range = AVCOL_RANGE_JPEG; >>>> + ctx->format = pix_fmt_rgb[bits]; >>>> + } else { >>>> + static const enum AVPixelFormat pix_fmt_for_ss[3][2 /* v */][2 /* >>>> h */] = { >>>> + { { AV_PIX_FMT_YUV444P, AV_PIX_FMT_YUV422P }, >>>> + { AV_PIX_FMT_YUV440P, AV_PIX_FMT_YUV420P } }, >>>> + { { AV_PIX_FMT_YUV444P10, AV_PIX_FMT_YUV422P10 }, >>>> + { AV_PIX_FMT_YUV440P10, AV_PIX_FMT_YUV420P10 } }, >>>> + { { AV_PIX_FMT_YUV444P12, AV_PIX_FMT_YUV422P12 }, >>>> + { AV_PIX_FMT_YUV440P12, AV_PIX_FMT_YUV420P12 } } >>>> + }; >>>> + avctx->color_range = get_bits1(gb) ? AVCOL_RANGE_JPEG : >>>> AVCOL_RANGE_MPEG; >>>> + if (avctx->profile & 1) { >>>> + int ss_h, ss_v, format; >>>> + >>>> + ss_h = get_bits1(gb); >>>> + ss_v = get_bits1(gb); >>>> + format = pix_fmt_for_ss[bits][ss_v][ss_h]; >>>> + if (format == AV_PIX_FMT_YUV420P) >>>> + // YUV 4:2:0 not supported in profiles 1 and 3 >>>> + return AVERROR_INVALIDDATA; >>>> + else if (get_bits1(gb)) // color details reserved bit >>>> + return AVERROR_INVALIDDATA; >>>> + ctx->format = format; >>>> + } else { >>>> + ctx->format = pix_fmt_for_ss[bits][1][1]; >>>> + } >>>> + } >>>> + >>>> + return 0; >>>> +} >>>> + >>>> static int parse(AVCodecParserContext *ctx, >>>> AVCodecContext *avctx, >>>> const uint8_t **out_data, int *out_size, >>>> const uint8_t *data, int size) >>>> { >>>> GetBitContext gb; >>>> - int res, profile, keyframe; >>>> + int res, profile, keyframe, invisible, errorres; >>>> >>>> *out_data = data; >>>> *out_size = size; >>>> @@ -42,21 +96,63 @@ static int parse(AVCodecParserContext *ctx, >>>> profile = get_bits1(&gb); >>>> profile |= get_bits1(&gb) << 1; >>>> if (profile == 3) profile += get_bits1(&gb); >>>> + if (profile > 3) >>>> + return size; >>>> >>>> if (get_bits1(&gb)) { >>>> keyframe = 0; >>>> + skip_bits(&gb, 3); >>>> } else { >>>> keyframe = !get_bits1(&gb); >>>> } >>>> >>>> + invisible = !get_bits1(&gb); >>>> + errorres = get_bits1(&gb); >>>> + >>>> if (!keyframe) { >>>> + int intraonly = invisible ? get_bits1(&gb) : 0; >>>> + if (!errorres) >>>> + skip_bits(&gb, 2); >>>> + if (intraonly) { >>>> + if (get_bits_long(&gb, 24) != VP9_SYNCCODE) // synccode >>>> + return size; >>>> + avctx->profile = profile; >>>> + if (profile >= 1) { >>>> + if ((read_colorspace_details(ctx, avctx, &gb)) < 0) >>>> + return size; >>>> + } else { >>>> + ctx->format = AV_PIX_FMT_YUV420P; >>>> + avctx->colorspace = AVCOL_SPC_BT470BG; >>>> + avctx->color_range = AVCOL_RANGE_MPEG; >>>> + } >>>> + skip_bits(&gb, 8); // refreshrefmask >>>> + ctx->width = get_bits(&gb, 16) + 1; >>>> + ctx->height = get_bits(&gb, 16) + 1; >>>> + if (get_bits1(&gb)) // display size >>>> + skip_bits(&gb, 32); >>>> + } >>>> + >>>> ctx->pict_type = AV_PICTURE_TYPE_P; >>>> ctx->key_frame = 0; >>>> } else { >>>> + if (get_bits_long(&gb, 24) != VP9_SYNCCODE) // synccode >>>> + return size; >>>> + avctx->profile = profile; >>>> + if (read_colorspace_details(ctx, avctx, &gb) < 0) >>>> + return size; >>>> + ctx->width = get_bits(&gb, 16) + 1; >>>> + ctx->height = get_bits(&gb, 16) + 1; >>>> + if (get_bits1(&gb)) // display size >>>> + skip_bits(&gb, 32); >>>> + >>>> ctx->pict_type = AV_PICTURE_TYPE_I; >>>> ctx->key_frame = 1; >>>> } >>>> >>> >>>> + if (ctx->width && (!avctx->width || !avctx->height || >>>> + !avctx->coded_width || !avctx->coded_height)) >>>> + ff_set_dimensions(avctx, ctx->width, ctx->height); >>> >>> This is the only ff_set_dimensions() in the codebase with non fixed >>> parameters >>> that ignores the return value >>> is that intended ? >> >> Yes, since right after it the only line that remains in the entire >> function is the return. >> >> I can change it to "if (ff_set_dimensions() < 0) return size" if that's >> preferred, but it will be functionally the same since parsers can't >> return errors, so they always return the full frame size. > > maybe write something like (void)ff_set_dimensions() > so its clear that you intended to ignore the return
I find that kinda ugly, so amended locally to add a check for the result. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel