| Message ID | cbb540de-9e51-807a-d349-ad2f7c60b767@carpalis.nl |
|---|---|
| State | Superseded |
| Headers | show |
2018-11-14 10:57 GMT+01:00, Jerome Borsboom <jerome.borsboom@carpalis.nl>: > + av_reduce(&avctx->sample_aspect_ratio.num, > + &avctx->sample_aspect_ratio.den, > + v->disp_horiz_size * v->aspect_ratio.num * h, > + v->disp_vert_size * v->aspect_ratio.den * w, > + 1 << 30); > + ff_set_sar(avctx, avctx->sample_aspect_ratio); I would have expected these two statements to be redundant - am I wrong? Carl Eugen
>> + av_reduce(&avctx->sample_aspect_ratio.num, >> + &avctx->sample_aspect_ratio.den, >> + v->disp_horiz_size * v->aspect_ratio.num * h, >> + v->disp_vert_size * v->aspect_ratio.den * w, >> + 1 << 30); > >> + ff_set_sar(avctx, avctx->sample_aspect_ratio); > > I would have expected these two statements to be redundant - > am I wrong? > > Carl Eugen I am not fully sure that I understand your comment. av_reduce takes out the greatest common divisor and puts a bound on the individual fraction numbers. ff_set_sar checks that the SAR makes sense in relation to the image size. Although, the code is reassigning avctx->sample_aspect_ratio to itself in ff_set_sar, the different checks make both statements necessary. In addition, the large values that results from 'v->disp_horiz_size * v->aspect_ratio.num * h' and 'v->disp_vert_size * v->aspect_ratio.den * w', can in most cases be efficiently reduced. These two statements are moved to the entrypoint parser, because the coded size is an entrypoint element. At the position where the av_reduce statement was previously located, the v->s.avctx->height and v->s.avctx->width could contain values from the previous entrypoint header or be zero when parsing the first sequence header. Regards, Jerome
2018-11-14 17:16 GMT+01:00, Jerome Borsboom <jerome.borsboom@carpalis.nl>: >>> + av_reduce(&avctx->sample_aspect_ratio.num, >>> + &avctx->sample_aspect_ratio.den, >>> + v->disp_horiz_size * v->aspect_ratio.num * h, >>> + v->disp_vert_size * v->aspect_ratio.den * w, >>> + 1 << 30); >> >>> + ff_set_sar(avctx, avctx->sample_aspect_ratio); >> >> I would have expected these two statements to be redundant - >> am I wrong? > av_reduce takes out the greatest common divisor and puts a bound on the > individual fraction numbers. ff_set_sar checks that the SAR makes sense > in relation to the image size. Although, the code is reassigning > avctx->sample_aspect_ratio to itself in ff_set_sar, the different checks > make both statements necessary. Thank you! Carl Eugen
diff --git a/libavcodec/vc1.c b/libavcodec/vc1.c index 3581d87b57..efc6edc4b0 100644 --- a/libavcodec/vc1.c +++ b/libavcodec/vc1.c @@ -442,30 +442,24 @@ static int decode_sequence_header_adv(VC1Context *v, GetBitContext *gb) } v->s.max_b_frames = v->s.avctx->max_b_frames = 7; if (get_bits1(gb)) { //Display Info - decoding is not affected by it - int w, h, ar = 0; + int ar = 0; av_log(v->s.avctx, AV_LOG_DEBUG, "Display extended info:\n"); - w = get_bits(gb, 14) + 1; - h = get_bits(gb, 14) + 1; - av_log(v->s.avctx, AV_LOG_DEBUG, "Display dimensions: %ix%i\n", w, h); + v->disp_horiz_size = get_bits(gb, 14) + 1; + v->disp_vert_size = get_bits(gb, 14) + 1; + av_log(v->s.avctx, AV_LOG_DEBUG, "Display dimensions: %ix%i\n", + v->disp_horiz_size, v->disp_vert_size); if (get_bits1(gb)) ar = get_bits(gb, 4); if (ar && ar < 14) { - v->s.avctx->sample_aspect_ratio = ff_vc1_pixel_aspect[ar]; + v->aspect_ratio = ff_vc1_pixel_aspect[ar]; } else if (ar == 15) { - w = get_bits(gb, 8) + 1; - h = get_bits(gb, 8) + 1; - v->s.avctx->sample_aspect_ratio = (AVRational){w, h}; + v->aspect_ratio = (AVRational){get_bits(gb, 8) + 1, get_bits(gb, 8) + 1}; } else { - av_reduce(&v->s.avctx->sample_aspect_ratio.num, - &v->s.avctx->sample_aspect_ratio.den, - v->s.avctx->height * w, - v->s.avctx->width * h, - 1 << 30); + v->aspect_ratio = (AVRational){1, 1}; } - ff_set_sar(v->s.avctx, v->s.avctx->sample_aspect_ratio); - av_log(v->s.avctx, AV_LOG_DEBUG, "Aspect: %i:%i\n", - v->s.avctx->sample_aspect_ratio.num, - v->s.avctx->sample_aspect_ratio.den); + av_log(v->s.avctx, AV_LOG_DEBUG, "Aspect ratio: %i:%i\n", + v->aspect_ratio.num, + v->aspect_ratio.den); if (get_bits1(gb)) { //framerate stuff if (get_bits1(gb)) { @@ -490,6 +484,10 @@ static int decode_sequence_header_adv(VC1Context *v, GetBitContext *gb) v->transfer_char = get_bits(gb, 8); v->matrix_coef = get_bits(gb, 8); } + } else { + v->disp_horiz_size = v->max_coded_width; + v->disp_vert_size = v->max_coded_height; + v->aspect_ratio = (AVRational){1, 1}; } v->hrd_param_flag = get_bits1(gb); @@ -544,6 +542,12 @@ int ff_vc1_decode_entry_point(AVCodecContext *avctx, VC1Context *v, GetBitContex av_log(avctx, AV_LOG_ERROR, "Failed to set dimensions %d %d\n", w, h); return ret; } + av_reduce(&avctx->sample_aspect_ratio.num, + &avctx->sample_aspect_ratio.den, + v->disp_horiz_size * v->aspect_ratio.num * h, + v->disp_vert_size * v->aspect_ratio.den * w, + 1 << 30); + ff_set_sar(avctx, avctx->sample_aspect_ratio); if (v->extended_mv) v->extended_dmv = get_bits1(gb); diff --git a/libavcodec/vc1.h b/libavcodec/vc1.h index 69f6ca9e4d..7674b0f9a1 100644 --- a/libavcodec/vc1.h +++ b/libavcodec/vc1.h @@ -209,6 +209,8 @@ typedef struct VC1Context{ int hrd_param_flag; ///< Presence of Hypothetical Reference ///< Decoder parameters int psf; ///< Progressive Segmented Frame + int disp_horiz_size, disp_vert_size; + AVRational aspect_ratio; //@} /** Sequence header data for all Profiles
According to VC-1 spec: * Display size defaults to max coded size when not explicitly set in sequence header * Aspect ratio in the sequence header refers to the Display size elements. Therefore, the aspect ratio for the coded samples (SAR) needs to take into account the scaling from coded size to display size, and the aspect ratio of the display size elements. Signed-off-by: Jerome Borsboom <jerome.borsboom@carpalis.nl> --- VC-1 spec assumes that the output of the decoder, i.e. the pixel matrix with dimensions coded_width x coded_height, is scaled to display size, i.e. a pixel matrix of display_horiz_size x display_vert_size. This may introduce part of the sample aspect ratio when the sizes of the two pixel matrices are not equal. A further part of the sample aspect ratio is optionally specified in the sequence header as the aspect ratio of the display size pixels. This patch takes both aspect ratios into account and aims to be correct even when the coded image includes overscan regions. libavcodec/vc1.c | 38 +++++++++++++++++++++----------------- libavcodec/vc1.h | 2 ++ 2 files changed, 23 insertions(+), 17 deletions(-)