[FFmpeg-devel] [PATCH] snow: remove strange av_assert2
andreas.cadhalpun at googlemail.com
Tue Jul 7 23:32:59 CEST 2015
On 06.07.2015 02:40, Michael Niedermayer wrote:
> On Sun, Jul 05, 2015 at 09:11:44PM +0200, Andreas Cadhalpun wrote:
>> Can you explain how elements can be too larger to fit?
> emulated_edge_mc() writes a block of width x height,
More precisely that's block_w x block_h.
> if stride < width this will not work as intended
Then an assert for stride > block_w might make sense in emulated_edge_mc,
but not in add_yblock.
> iam quite unsure if this is the intended thing for the assert to
> check, i hthik there was more code that possibly was fixed
I think the assert is a historic leftover:
Before commit cc884a35 src_stride > 7*MB_SIZE was necessary, because
the blocks were interleaved in the tmp buffer and the last block
was added with an offset of 6*MB_SIZE.
It was changed for src_stride <= 7*MB_SIZE to write the blocks
sequentially, hence the larger tmp_stride. (A comment about this
in the code would have been nice.)
After that I think the assert was only necessary to make sure that
the buffer remained large enough.
Since commit bd2b6b33 s->scratchbuf is used as tmp buffer.
As part of commit 86e107a7 the minimal scratchbuf size was increased
to 256*7*MB_SIZE, which is enough for any src_stride <= 7*MB_SIZE.
So I think removing this assert is the correct way forward.
However, there are still some things in this code which are unclear for me:
* Where does the 5 in 'src_stride > 2*MB_SIZE + 5' come from?
Shouldn't that have been HTAPS_MAX-1, because that is added to block_h,
when calling emulated_edge_mc?
* Why the factor 2 in 'src_stride > 2*MB_SIZE + 5'?
Before commit cc884a35 the buffer size was 'src_stride*(b_h+5)' and
b_h is at maximum MB_SIZE, not 2*MB_SIZE.
* Why is tmp_step based on MB_SIZE and not (MB_SIZE + HTAPS_MAX-1)?
This 'HTAPS_MAX-1' is added to b_w/b_h when calling emulated_edge_mc.
Maybe you can enlighten me.
More information about the ffmpeg-devel