On 2013-03-12 12:48:36 +0100, Anton Khirnov wrote:
> Based on a patch by Vittorio Giovara <[email protected]>
> 
> Fixes Bug 378.
> ---
>  doc/APIchanges             |    3 ++
>  libavcodec/avcodec.h       |    5 ++++
>  libavcodec/h264.c          |   70 
> +++++++++++++++++++++++++++++++-------------
>  libavcodec/h264_ps.c       |   14 ++++-----
>  libavcodec/options_table.h |    1 +
>  5 files changed, 64 insertions(+), 29 deletions(-)
> 
> diff --git a/doc/APIchanges b/doc/APIchanges
> index 0d317fc..ff5884a 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -13,6 +13,9 @@ libavutil:     2012-10-22
>  
>  API changes, most recent first:
>  
> +2013-03-xx - xxxxxxx - lavc 55.0.0 - avcodec.h
> +  Add CODEC_FLAG_UNALIGNED to allow decoders to produce unaligned output.
> +
>  2013-03-xx - Reference counted buffers - lavu 52.8.0, lavc 55.0.0, lavf 
> 55.0.0,
>  lavd 54.0.0, lavfi 3.5.0
>    xxxxxxx, xxxxxxx - add a new API for reference counted buffers and buffer
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index e48b114..ced7fed 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -609,6 +609,11 @@ typedef struct RcOverride{
>     Note: Not everything is supported yet.
>  */
>  
> +/**
> + * Allow decoders to produce frames with data planes that are not aligned
> + * to CPU requirements (e.g. due to cropping).
> + */
> +#define CODEC_FLAG_UNALIGNED 0x0001

not sure if this is the best way to handle it but certainly better than
copying frames.

>  #define CODEC_FLAG_QSCALE 0x0002  ///< Use fixed qscale.
>  #define CODEC_FLAG_4MV    0x0004  ///< 4 MV per MB allowed / advanced 
> prediction for H.263.
>  #define CODEC_FLAG_QPEL   0x0010  ///< Use qpel MC.
> diff --git a/libavcodec/h264.c b/libavcodec/h264.c
> index 3fa6475..1d84a9d 100644
> --- a/libavcodec/h264.c
> +++ b/libavcodec/h264.c
> @@ -1423,9 +1423,6 @@ av_cold int ff_h264_decode_init(AVCodecContext *avctx)
>  
>      h->avctx = avctx;
>  
> -    h->width    = h->avctx->width;
> -    h->height   = h->avctx->height;
> -
>      h->bit_depth_luma    = 8;
>      h->chroma_format_idc = 1;
>  
> @@ -2973,11 +2970,30 @@ static int h264_slice_header_init(H264Context *h, int 
> reinit)
>                      h->avctx->thread_count : 1;
>      int i;
>  
> -    avcodec_set_dimensions(h->avctx, h->width, h->height);
> -    h->avctx->sample_aspect_ratio = h->sps.sar;
> -    av_assert0(h->avctx->sample_aspect_ratio.den);
>      av_pix_fmt_get_chroma_sub_sample(h->avctx->pix_fmt,
>                                       &h->chroma_x_shift, &h->chroma_y_shift);
> +    h->avctx->coded_width  = h->width;
> +    h->avctx->coded_height = h->height;
> +    h->avctx->width        = h->width - (2 >> CHROMA444(h)) * 
> (h->sps.crop_right +
> +                                                               
> h->sps.crop_left);

That's not correct, chroma_format_idc == 0 (monochrome) also uses 1 as
unit.

> +    h->avctx->height       = h->height - ((h->sps.frame_mbs_only_flag ? 1 : 
> 2) <<

(2 - h->sps.frame_mbs_only_flag)

> +                                          h->chroma_y_shift) *

same problem, since we use yuv420p as pixel format for chroma_format_idc
== 0

> +                                         (h->sps.crop_top + 
> h->sps.crop_bottom);

I guess it would make more sense to do the calculations during sps
parsing and specify h->sps.crop_* in luma pixels.

> +
> +    if (h->avctx->width <= 0 || h->avctx->height <= 0) {
> +        av_log(h->avctx, AV_LOG_ERROR, "Invalid cropped dimensions: 
> %dx%d.\n",
> +               h->avctx->width, h->avctx->height);
> +        if (h->avctx->err_recognition & AV_EF_EXPLODE)
> +            return AVERROR_INVALIDDATA;
> +
> +        av_log(h->avctx, AV_LOG_WARNING, "Ignoring cropping information.\n");
> +        h->sps.crop_bottom = h->sps.crop_top = h->sps.crop_right = 
> h->sps.crop_left = 0;
> +        h->avctx->width  = h->width;
> +        h->avctx->height = h->height;
> +    }
> +
> +    h->avctx->sample_aspect_ratio = h->sps.sar;
> +    av_assert0(h->avctx->sample_aspect_ratio.den);
>  
>      if (h->sps.timing_info_present_flag) {
>          int64_t den = h->sps.time_scale;
> @@ -3184,17 +3200,8 @@ static int decode_slice_header(H264Context *h, 
> H264Context *h0)
>  
>      h->chroma_y_shift = h->sps.chroma_format_idc <= 1; // 400 uses yuv420p
>  
> -    h->width = 16 * h->mb_width - (2 >> CHROMA444(h)) * 
> FFMIN(h->sps.crop_right, (8 << CHROMA444(h)) - 1);
> -    if (h->sps.frame_mbs_only_flag)
> -        h->height = 16 * h->mb_height - (1 << h->chroma_y_shift) * 
> FFMIN(h->sps.crop_bottom, (16 >> h->chroma_y_shift) - 1);
> -    else
> -        h->height = 16 * h->mb_height - (2 << h->chroma_y_shift) * 
> FFMIN(h->sps.crop_bottom, (16 >> h->chroma_y_shift) - 1);
> -
> -    if (FFALIGN(h->avctx->width,  16) == h->width &&
> -        FFALIGN(h->avctx->height, 16) == h->height) {
> -        h->width  = h->avctx->width;
> -        h->height = h->avctx->height;
> -    }

removing this breaks the workaround added in
30f515091c323da59c0f1b533703dedca2f4b95d said canon samples have no
cropping info in the sps

> +    h->width  = 16 * h->mb_width;
> +    h->height = 16 * h->mb_height;
>  
>      if (h->sps.video_signal_type_present_flag) {
>          h->avctx->color_range = h->sps.full_range ? AVCOL_RANGE_JPEG
> @@ -3209,8 +3216,8 @@ static int decode_slice_header(H264Context *h, 
> H264Context *h0)
>      }
>  
>      if (h->context_initialized &&
> -        (h->width  != h->avctx->width   ||
> -         h->height != h->avctx->height  ||
> +        (h->width  != h->avctx->coded_width   ||
> +         h->height != h->avctx->coded_height  ||
>           needs_reinit)) {
>  
>          if (h != h0) {
> @@ -4597,6 +4604,25 @@ static int get_consumed_bytes(int pos, int buf_size)
>      return pos;
>  }
>  
> +static int output_frame(H264Context *h, AVFrame *dst, AVFrame *src)
> +{
> +    int i;
> +    int ret = av_frame_ref(dst, src);
> +    if (ret < 0)
> +        return ret;
> +
> +    for (i = 0; i < 3; i++) {
> +        int step_x = (2 >> CHROMA444(h));
> +        int step_y = (h->sps.frame_mbs_only_flag ? 1 : 2) << 
> h->chroma_y_shift;

duplicating the incorrect calculations, please move them to sps parsing

Janne
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to