Janne Grunau <[email protected]> writes:
> Hi,
>
> it has still at least one hard to reproduce timing bug. I can reproduce
> it only with framecrc and 4 threads. I can't reproduce it when doing
> anything more time consuming than crc computation. Frame data and even
> framemd5 are identical.
That obviously needs to be fixed...
> Janne
> ---8<--
> Statistics for bourne.rmvb -an -f null
>
> 1 thread: 37.12s user 0.03s system 99% cpu 37.174 total
> 2 threads: 47.63s user 0.24s system 185% cpu 25.807 total
> 4 threads: 41.21s user 0.30s system 327% cpu 12.674 total
At least it scales nicely. What CPU was this on?
[...]
> case RV34_MB_B_DIRECT:
> //surprisingly, it uses motion scheme from next reference frame
> + /* wait for the current mb row to be finished */
> + if (HAVE_THREADS && (s->avctx->active_thread_type & FF_THREAD_FRAME))
> + ff_thread_await_progress(&s->next_picture_ptr->f, s->mb_y + 1, 0);
> +
Indentation is off.
> next_bt = s->next_picture_ptr->f.mb_type[s->mb_x + s->mb_y *
> s->mb_stride];
> if(IS_INTRA(next_bt) || IS_SKIP(next_bt)){
> ZERO8x2(s->current_picture_ptr->f.motion_val[0][s->mb_x * 2 +
> s->mb_y * 2 * s->b8_stride], s->b8_stride);
> @@ -1260,6 +1276,7 @@ static int rv34_decode_slice(RV34DecContext *r, int
> end, const uint8_t* buf, int
> }
> }
> s->mb_x = s->mb_y = 0;
> + ff_thread_finish_setup(s->avctx);
> } else {
> int slice_type = r->si.type ? r->si.type : AV_PICTURE_TYPE_I;
>
> @@ -1305,6 +1322,11 @@ static int rv34_decode_slice(RV34DecContext *r, int
> end, const uint8_t* buf, int
>
> if(r->loop_filter && s->mb_y >= 2)
> r->loop_filter(r, s->mb_y - 2);
> +
> + if (HAVE_THREADS && (s->avctx->active_thread_type &
> FF_THREAD_FRAME))
This is a recurring condition, not only in this decoder. Perhaps a
helper macro/function would make them all easier to manage.
> + ff_thread_report_progress(&s->current_picture_ptr->f,
> + s->mb_y-2, 0);
> +
> }
> if(s->mb_x == s->resync_mb_x)
> s->first_slice_line=0;
> @@ -1370,6 +1392,71 @@ av_cold int ff_rv34_decode_init(AVCodecContext *avctx)
> return 0;
> }
>
> +int ff_rv34_decode_init_thread_copy(AVCodecContext *avctx)
> +{
> + RV34DecContext *r = avctx->priv_data;
> +
> + r->s.avctx = avctx;
> +
> + if (avctx->internal->is_copy) {
> + FF_ALLOC_OR_GOTO(avctx, r->intra_types_hist, r->intra_types_stride *
> + 4 * 2 * sizeof(*r->intra_types_hist),
> err_mem1);
> + FF_ALLOC_OR_GOTO(avctx, r->mb_type, r->s.mb_stride *
> + r->s.mb_height * sizeof(*r->mb_type),
> err_mem2);
> + FF_ALLOC_OR_GOTO(avctx, r->cbp_luma, r->s.mb_stride *
> + r->s.mb_height * sizeof(*r->cbp_luma),
> err_mem3);
> + FF_ALLOC_OR_GOTO(avctx, r->cbp_chroma, r->s.mb_stride *
> + r->s.mb_height * sizeof(*r->cbp_chroma),
> err_mem4);
> + FF_ALLOC_OR_GOTO(avctx, r->deblock_coefs, r->s.mb_stride *
> + r->s.mb_height * sizeof(*r->deblock_coefs),
> err_mem5);
> +
> + r->intra_types = r->intra_types_hist + r->intra_types_stride *
> 4;
> + r->tmp_b_block_base = NULL;
> +
> + memset(r->mb_type, 0, r->s.mb_stride * r->s.mb_height *
> + sizeof(*r->mb_type));
> +
> + MPV_common_init(&r->s);
> + }
> + return 0;
> +err_mem5:
> + av_freep(&r->cbp_chroma);
> +err_mem4:
> + av_freep(&r->cbp_luma);
> +err_mem3:
> + av_freep(&r->mb_type);
> +err_mem2:
> + av_freep(&r->intra_types_hist);
> + r->intra_types = NULL;
> +err_mem1:
> + return AVERROR(ENOMEM);
No need for separate labels. Freeing a null pointer is perfectly safe.
--
Måns Rullgård
[email protected]
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
