On Thu, Feb 05, 2015 at 08:17:25AM +0100, Christophe Gisquet wrote: > Hi, > > 2015-02-05 7:29 GMT+01:00 Christophe Gisquet <christophe.gisq...@gmail.com>: > > We were previously reference-counting the sao-buffer. Should we do > > that for sao_pixel_buffer_[hv], then? > > Something like the attached patch. > > Note: I'm probably overallocating compared to previously, but that > doesn't look to be a big deal. > > On the other hand, those are buffers sized 2*block_height*image_width > and 2*block_width*image_height. > The storing is maybe overzealous, as I don't see why just more than 1 > or 2 lines/columns need to be stored. > > -- > Christophe
> hevc.c | 50 +++++++++++++++++++++++++++++++++-----------------
> hevc.h | 4 ++--
> hevc_filter.c | 18 ++++++++++--------
> 3 files changed, 45 insertions(+), 27 deletions(-)
> 139899fbe6e35afb746820042742bbae86da69f8
> 0001-hevc-sao-referenced-line-column-buffers.patch
> From 8ac4c9e5db63de26523e4677f397019c5eb6bd89 Mon Sep 17 00:00:00 2001
> From: Christophe Gisquet <christophe.gisq...@gmail.com>
> Date: Thu, 5 Feb 2015 08:11:28 +0100
> Subject: [PATCH] hevc/sao: referenced line/column buffers
>
> Otherwise, a change in parameters may cause the buffers to be lost,
> causing memory leak.
> ---
> libavcodec/hevc.c | 50
> ++++++++++++++++++++++++++++++++----------------
> libavcodec/hevc.h | 4 ++--
> libavcodec/hevc_filter.c | 18 +++++++++--------
> 3 files changed, 45 insertions(+), 27 deletions(-)
>
> diff --git a/libavcodec/hevc.c b/libavcodec/hevc.c
> index 0624cb0..9a6df3c 100644
> --- a/libavcodec/hevc.c
> +++ b/libavcodec/hevc.c
> @@ -280,6 +280,23 @@ static int decode_lt_rps(HEVCContext *s, LongTermRPS
> *rps, GetBitContext *gb)
> return 0;
> }
>
> +static int get_buffer_sao(HEVCContext *s, const HEVCSPS *sps)
> +{
> + int ret;
> + AVFrame *frame = s->tmp_frame[0];
> + frame->width = FFALIGN(4 * sps->width, FF_INPUT_BUFFER_PADDING_SIZE);
FF_INPUT_BUFFER_PADDING_SIZE is not guranteed to be a power of 2 but
FFALIGN needs a power of 2
> + frame->height = sps->ctb_height;
> + if ((ret = ff_get_buffer(s->avctx, frame, AV_GET_BUFFER_FLAG_REF)) < 0)
> + return ret;
> +
> + frame = s->tmp_frame[1];
> + frame->width = FFALIGN(4 * sps->height, FF_INPUT_BUFFER_PADDING_SIZE);
> + frame->height = sps->ctb_width;
> + if ((ret = ff_get_buffer(s->avctx, frame, AV_GET_BUFFER_FLAG_REF)) < 0)
> + return ret;
> + return 0;
> +}
> +
> static int set_sps(HEVCContext *s, const HEVCSPS *sps)
> {
> #define HWACCEL_MAX (CONFIG_HEVC_DXVA2_HWACCEL)
> @@ -335,19 +352,14 @@ static int set_sps(HEVCContext *s, const HEVCSPS *sps)
> ff_videodsp_init (&s->vdsp, sps->bit_depth);
>
> if (sps->sao_enabled && !s->avctx->hwaccel) {
> - int c_count = (sps->chroma_format_idc != 0) ? 3 : 1;
> - int c_idx;
> -
> - for(c_idx = 0; c_idx < c_count; c_idx++) {
> - int w = sps->width >> sps->hshift[c_idx];
> - int h = sps->height >> sps->vshift[c_idx];
> - s->sao_pixel_buffer_h[c_idx] =
> - av_malloc((w * 2 * sps->ctb_height) <<
> - sps->pixel_shift);
> - s->sao_pixel_buffer_v[c_idx] =
> - av_malloc((h * 2 * sps->ctb_width) <<
> - sps->pixel_shift);
> - }
> + av_frame_unref(s->tmp_frame[0]);
> + av_frame_unref(s->tmp_frame[1]);
> + s->sao_frame[0] = NULL;
> + s->sao_frame[1] = NULL;
> + if ( (ret = get_buffer_sao(s, sps)) < 0 )
> + goto fail;
> + s->sao_frame[0] = s->tmp_frame[0];
> + s->sao_frame[1] = s->tmp_frame[1];
I dont understand this code
tmp_frame is allocated then the pointer copied into sao_frame
later then tmp_frame is unreferenced which makes sao_frame a stale
pointer if its still pointing to the same memory
it seems the code works though with make fate even under valgrind,
so maybe iam missing something but wouldnt a simply av_freep() with
the existing code achive the same ?
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Complexity theory is the science of finding the exact solution to an
approximation. Benchmarking OTOH is finding an approximation of the exact
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
