On 2013-03-22 15:30:19 +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 | 86
> +++++++++++++++++++++++++++++++++-----------
> libavcodec/h264.h | 6 ++++
> libavcodec/h264_ps.c | 43 +++++++++++++---------
> libavcodec/options_table.h | 1 +
> 6 files changed, 108 insertions(+), 36 deletions(-)
>
> diff --git a/doc/APIchanges b/doc/APIchanges
> index 91bbeec..72f8190 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-xx-xx - lavu 52.9.0 - pixdesc.h
> Add av_pix_fmt_count_planes() function for counting planes in a pixel
> format.
>
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index 84a6859..c36d976 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
> #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 04ceab5..fe5b4d7 100644
> --- a/libavcodec/h264.c
> +++ b/libavcodec/h264.c
> @@ -1423,9 +1423,8 @@ av_cold int ff_h264_decode_init(AVCodecContext *avctx)
> int i;
>
> h->avctx = avctx;
> -
> - h->width = h->avctx->width;
> - h->height = h->avctx->height;
> + h->orig_width = avctx->width;
> + h->orig_height = avctx->height;
I fail to see the need for this
> h->bit_depth_luma = 8;
> h->chroma_format_idc = 1;
> @@ -2972,14 +2971,51 @@ static enum AVPixelFormat
> get_pixel_format(H264Context *h)
> }
> }
>
> +/* export coded and cropped frame dimensions to AVCodecContext */
> +static int init_dimensions(H264Context *h)
> +{
> + int width = h->width - (h->sps.crop_right + h->sps.crop_left);
> + int height = h->height - (h->sps.crop_top + h->sps.crop_bottom);
> +
> + /* handle container cropping */
> + if (!h->sps.crop &&
> + FFALIGN(h->orig_width, 16) == h->width &&
> + FFALIGN(h->orig_height, 16) == h->height) {
> + width = h->orig_width;
> + height = h->orig_height;
> + }
> +
> + h->avctx->coded_width = h->width;
> + h->avctx->coded_height = h->height;
> + h->avctx->width = width;
> + h->avctx->height = height;
> +
> + if (h->avctx->width <= 0 || h->avctx->height <= 0) {
I would check width and height before setting it, makes not really a
difference except that you would have to write less 'h->avctx->'
> + 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;
add h->sps.crop = 0;
> + h->avctx->width = h->width;
> + h->avctx->height = h->height;
> + }
> +
> + return 0;
> +}
> +
> static int h264_slice_header_init(H264Context *h, int reinit)
> {
> int nb_slices = (HAVE_THREADS &&
> h->avctx->active_thread_type & FF_THREAD_SLICE) ?
> h->avctx->thread_count : 1;
> - int i;
> + int i, ret;
> +
> + ret = init_dimensions(h);
> + if (ret < 0)
> + return ret;
>
> - 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,
> @@ -3190,17 +3226,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;
> - }
> + h->width = 16 * h->mb_width;
> + h->height = 16 * h->mb_height;
this makes me wonder if we even need height and width in the context.
Also changes of the cropping won't update the avctx values and the
dimensions of the returned frame will differ from the avctx. the
init_dimensions() call needs to happen here
> if (h->sps.video_signal_type_present_flag) {
> h->avctx->color_range = h->sps.full_range ? AVCOL_RANGE_JPEG
> @@ -3215,8 +3242,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) {
> @@ -4603,6 +4630,23 @@ 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;
if (h->sps->crop) {
> + for (i = 0; i < 3; i++) {
> + int hshift = (i > 0) ? h->chroma_x_shift : 0;
> + int vshift = (i > 0) ? h->chroma_y_shift : 0;
> + int off = ((h->sps.crop_left >> hshift) << h->pixel_shift) +
> + (h->sps.crop_top >> vshift) * dst->linesize[i];
> + dst->data[i] += off;
> + }
}
no need to calculate and add zero
> + return 0;
> +}
> +
> static int decode_frame(AVCodecContext *avctx, void *data,
> int *got_frame, AVPacket *avpkt)
> {
> @@ -4640,7 +4684,8 @@ out:
> h->delayed_pic[i] = h->delayed_pic[i + 1];
>
> if (out) {
> - if ((ret = av_frame_ref(pict, &out->f)) < 0)
> + ret = output_frame(h, pict, &out->f);
> + if (ret < 0)
> return ret;
> *got_frame = 1;
> }
> @@ -4675,7 +4720,8 @@ out:
> /* Wait for second field. */
> *got_frame = 0;
> } else {
> - if ((ret = av_frame_ref(pict, &h->next_output_pic->f)) < 0)
> + ret = output_frame(h, pict, &h->next_output_pic->f);
> + if (ret < 0)
> return ret;
> *got_frame = 1;
> }
> diff --git a/libavcodec/h264.h b/libavcodec/h264.h
> index f5246fe..ce0c66e 100644
> --- a/libavcodec/h264.h
> +++ b/libavcodec/h264.h
> @@ -164,6 +164,8 @@ typedef struct SPS {
> int mb_aff; ///< mb_adaptive_frame_field_flag
> int direct_8x8_inference_flag;
> int crop; ///< frame_cropping_flag
> +
> + /* those 4 are already in luma samples */
> unsigned int crop_left; ///< frame_cropping_rect_left_offset
> unsigned int crop_right; ///< frame_cropping_rect_right_offset
> unsigned int crop_top; ///< frame_cropping_rect_top_offset
> @@ -272,7 +274,11 @@ typedef struct H264Context {
>
> int qp_thresh; ///< QP threshold to skip loopfilter
>
> + /* coded dimensions -- 16 * mb w/h */
> int width, height;
> + /* dimensions set by the caller before opening the codec
> + * we use them for cropping */
> + int orig_width, orig_height;
> int linesize, uvlinesize;
> int chroma_x_shift, chroma_y_shift;
>
> diff --git a/libavcodec/h264_ps.c b/libavcodec/h264_ps.c
> index 6c895a4..e94f423 100644
> --- a/libavcodec/h264_ps.c
> +++ b/libavcodec/h264_ps.c
> @@ -413,31 +413,42 @@ int ff_h264_decode_seq_parameter_set(H264Context *h){
> #endif
> sps->crop= get_bits1(&h->gb);
> if(sps->crop){
> - int crop_vertical_limit = sps->chroma_format_idc & 2 ? 16 : 8;
> - int crop_horizontal_limit = sps->chroma_format_idc == 3 ? 16 : 8;
> - sps->crop_left = get_ue_golomb(&h->gb);
> - sps->crop_right = get_ue_golomb(&h->gb);
> - sps->crop_top = get_ue_golomb(&h->gb);
> - sps->crop_bottom= get_ue_golomb(&h->gb);
> + int crop_left = get_ue_golomb(&h->gb);
> + int crop_right = get_ue_golomb(&h->gb);
> + int crop_top = get_ue_golomb(&h->gb);
> + int crop_bottom = get_ue_golomb(&h->gb);
> +
> if (h->avctx->flags2 & CODEC_FLAG2_IGNORE_CROP) {
> av_log(h->avctx, AV_LOG_DEBUG,
> "discarding sps cropping, "
> "original values are l:%u r:%u t:%u b:%u\n",
> - sps->crop_left,
> - sps->crop_right,
> - sps->crop_top,
> - sps->crop_bottom);
> + crop_left,
> + crop_right,
> + crop_top,
> + crop_bottom);
while you're at it, could you please make the log call not take seven
lines?
> sps->crop_left =
> sps->crop_right =
> sps->crop_top =
> sps->crop_bottom = 0;
unecessary now but setting sps->crop to 0 would makes sense
> - }
> - if(sps->crop_left || sps->crop_top){
> - av_log(h->avctx, AV_LOG_ERROR, "insane cropping not completely
> supported, this could look slightly wrong ...\n");
> - }
> - if(sps->crop_right >= crop_horizontal_limit || sps->crop_bottom >=
> crop_vertical_limit){
> - av_log(h->avctx, AV_LOG_ERROR, "brainfart cropping not
> supported, this could look slightly wrong ...\n");
> + } else {
> + int vsub = (sps->chroma_format_idc == 1) ? 1 : 0;
> + int hsub = (sps->chroma_format_idc == 1 ||
> sps->chroma_format_idc == 2) ? 1 : 0;
> + int step_x = 1 << hsub;
> + int step_y = (2 - sps->frame_mbs_only_flag) << vsub;
> +
> + if (crop_left & (0x1F >> (sps->bit_depth_luma > 8)) &&
> + !(h->avctx->flags & CODEC_FLAG_UNALIGNED)) {
> + crop_left &= ~(0x1F >> (sps->bit_depth_luma > 8));
> + av_log(h->avctx, AV_LOG_WARNING, "Reducing left cropping to
> %d "
> + "chroma samples to preserve alignment.\n",
> + sps->crop_left);
just crop_left, sps->crop_left is still zero
> + }
> +
> + sps->crop_left = crop_left * step_x;
> + sps->crop_right = crop_right * step_x;
> + sps->crop_top = crop_top * step_y;
> + sps->crop_bottom = crop_bottom * step_y;
> }
> }else{
> sps->crop_left =
> diff --git a/libavcodec/options_table.h b/libavcodec/options_table.h
> index e1b124b..5595c0e 100644
> --- a/libavcodec/options_table.h
> +++ b/libavcodec/options_table.h
> @@ -46,6 +46,7 @@ static const AVOption options[]={
> "to minimum/maximum bitrate. Lowering tolerance too much has an
> adverse effect on quality.",
> OFFSET(bit_rate_tolerance), AV_OPT_TYPE_INT, {.i64 =
> AV_CODEC_DEFAULT_BITRATE*20 }, 1, INT_MAX, V|E},
> {"flags", NULL, OFFSET(flags), AV_OPT_TYPE_FLAGS, {.i64 = DEFAULT }, 0,
> UINT_MAX, V|A|E|D, "flags"},
> +{"unaligned", "allow decoders to produce unaligned output", 0,
> AV_OPT_TYPE_CONST, { .i64 = CODEC_FLAG_UNALIGNED }, INT_MIN, INT_MAX, V | D,
> "flags" },
> {"mv4", "use four motion vectors per macroblock (MPEG-4)", 0,
> AV_OPT_TYPE_CONST, {.i64 = CODEC_FLAG_4MV }, INT_MIN, INT_MAX, V|E, "flags"},
> {"qpel", "use 1/4-pel motion compensation", 0, AV_OPT_TYPE_CONST, {.i64 =
> CODEC_FLAG_QPEL }, INT_MIN, INT_MAX, V|E, "flags"},
> {"loop", "use loop filter", 0, AV_OPT_TYPE_CONST, {.i64 =
> CODEC_FLAG_LOOP_FILTER }, INT_MIN, INT_MAX, V|E, "flags"},
Janne
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
