[FFmpeg-devel] [PATCH] H264: use alias-safe macros

[Måns Rullgård]

Michael Niedermayer <michaelni at gmx.at> writes:

> On Thu, Feb 18, 2010 at 03:09:08PM +0000, Mans Rullgard wrote:
>> This eliminates all aliasing violation warnings in h264 code.
>> No measurable speed difference with gcc-4.4.3 on i7.
>> ---
>>  libavcodec/h264.c            |    6 +-
>>  libavcodec/h264.h            |  111 +++++++++++++++++++++---------------------
>>  libavcodec/h264_direct.c     |   14 +++---
>>  libavcodec/h264_loopfilter.c |   33 +++++++------
>>  libavcodec/h264_mvpred.h     |    6 +-
>>  5 files changed, 87 insertions(+), 83 deletions(-)
>> 
>> diff --git a/libavcodec/h264.c b/libavcodec/h264.c
>> index f0abd6b..080b872 100644
>> --- a/libavcodec/h264.c
>> +++ b/libavcodec/h264.c
> [...]
>> @@ -1160,8 +1160,8 @@ static av_always_inline void hl_decode_mb_internal(H264Context *h, int simple){
>>                                      const int topright_avail= (h->topright_samples_available<<i)&0x8000;
>>                                      assert(mb_y || linesize <= block_offset[i]);
>>                                      if(!topright_avail){
>> -                                        tr= ptr[3 - linesize]*0x01010101;
>>                                          topright= (uint8_t*) &tr;
>> +                                        AV_WN32A(topright, ptr[3 - linesize]*0x01010101);
>>                                      }else
>>                                          topright= ptr + 4 - linesize;
>>                                  }else
>
> This hunk is unneccesary (or wrong)
>
> the effective type is int, writing to this as int is not an aliasing
> violation, casting to uint8_t is risky but not wrong either
> the aliasing violation happens just if the uint8_t pointer is being converted
> to a non char and non int type and dereferenced and this still is an
> aliasing violation after your code so i dont think this hunk should be
> commited

I had some doubts about that one myself...

> the other hunks look ok

Applied.

-- 
M?ns Rullg?rd
mans at mansr.com