[FFmpeg-devel] [PATCH] avcodec/vp9_parser: export profile and pixel format

[James Almer]

On 10/24/2018 3:12 PM, Chris Cunningham wrote:
> On Wed, Oct 3, 2018 at 10:49 AM James Almer <jamrial at gmail.com> wrote:
> 
>> Signed-off-by: James Almer <jamrial at gmail.com>
>> ---
>>  libavcodec/vp9_parser.c | 82 ++++++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 80 insertions(+), 2 deletions(-)
>>
>> diff --git a/libavcodec/vp9_parser.c b/libavcodec/vp9_parser.c
>> index 9531f34a32..d4110f20bf 100644
>> --- a/libavcodec/vp9_parser.c
>> +++ b/libavcodec/vp9_parser.c
>> @@ -23,36 +23,114 @@
>>
>>  #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 colorspace, bits = avctx->profile <= 1 ? 0 : 1 + get_bits1(gb);
>> // 0:8, 1:10, 2:12
>> +
>> +    colorspace = colorspaces[get_bits(gb, 3)];
>> +    if (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;
>> +        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 } }
>> +        };
>> +        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;
>>
>> -    if ((res = init_get_bits8(&gb, data, size)) < 0)
>> +    if (!size || (res = init_get_bits8(&gb, data, size)) < 0)
>>          return size; // parsers can't return errors
>>      get_bits(&gb, 2); // frame marker
>>      profile  = get_bits1(&gb);
>>      profile |= get_bits1(&gb) << 1;
>>      if (profile == 3) profile += get_bits1(&gb);
>> +    if (profile > 3)
>> +        return size;
>>
>> +    avctx->profile = profile;
>>      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;
>> +            if (profile >= 1) {
>> +                if ((read_colorspace_details(ctx, avctx, &gb)) < 0)
>> +                    return size;
>> +            } else {
>> +                ctx->format = AV_PIX_FMT_YUV420P;
>> +            }
>> +        }
>> +
>>          ctx->pict_type = AV_PICTURE_TYPE_P;
>>          ctx->key_frame = 0;
>>      } else {
>> +        if (get_bits_long(&gb, 24) != VP9_SYNCCODE) // synccode
>> +            return size;
>> +        if (read_colorspace_details(ctx, avctx, &gb) < 0)
>> +            return size;
>> +
>>          ctx->pict_type = AV_PICTURE_TYPE_I;
>>          ctx->key_frame = 1;
>>      }
>> --
>> 2.19.0
>>
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel at ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> 
> Friendly ping for review.

Dropping this as it's not correct. It's not taking into account
show_existing_frame frames (And reading bits way past the header bits),
or superframes, where it may be parsing an invisible frame.

Pushed only the part setting profile.