[FFmpeg-devel] [PATCH] snow: remove strange av_assert2

[Andreas Cadhalpun]

On 08.07.2015 01:22, Michael Niedermayer wrote:
> On Tue, Jul 07, 2015 at 11:32:59PM +0200, Andreas Cadhalpun wrote:
>> 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.)
> 
> yes, should i add one or you want to ?

I added one to the patch. Updated version attached.
Does that sound good?

>> 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.
> 
> I dont remember trying to make the assert be exact just sufficient
> to detect problems, it was not intended to stay IIRC, so it would have
> been a waste of time to calculate exactly what the minimum size
> was
> also i think that we should only spend time on this investigation if
> we intend to work on the code. Its hardly worth for just removing
> the apparently unneeded assert.
> if you want to improve snow (the algorithm or implementation)
> then investigating every smal bit does make sense

OK.

>>  * 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.
> 
> to keep things aligned in memory, it may help or be required for SIMD
> use

I suppose that's correct then.

Best regards,
Andreas
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-snow-remove-an-obsolete-av_assert2.patch
Type: text/x-diff
Size: 2010 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20150709/d2c71ccb/attachment.patch>