[FFmpeg-devel] [PATCH] H.264/AVCHD interlaced fixes

Michael Niedermayer michaelni
Thu Feb 19 14:45:05 CET 2009


On Thu, Feb 19, 2009 at 12:51:52PM +0100, Ivan Schreter wrote:
> Michael Niedermayer wrote:
>> On Tue, Feb 17, 2009 at 10:44:43AM +0100, Ivan Schreter wrote:
>>   
>>> Michael Niedermayer wrote:
>>>     [...]
>>>> also the whole ff_h264_decode_rbsp_trailing seems unneeded unless i
>>>> miss something
>>>>         
>>> I'm also unsure, whether it is needed. Byte-exact length should be 
>>> actually sufficient for the stuff parsed in parser (SEI, SPS, PPS and 
>>> slice header). I tried removing it and could parse my samples without 
>>> problem, as it seems. OTOH, in the future, some other NALs might need 
>>> bit-exact length to decode without warning and then we'll have to search 
>>> why the warning comes. So maybe we should let it in. What do you think?
>>>     
>>
>> remove it or factorize the code so it is not duplicated
>>
>>   
> I've factorized decode_nal to actually return bit length. Patch attached.
>
> I'll change the rest of parser patches appropriately, after this is in (and 
> the convergence duration patch in other thread).
>
> Regards,
>
> Ivan
>

[...]

> @@ -1385,11 +1386,11 @@
>  # if HAVE_FAST_64BIT
>  #   define RS 7
>      for(i=0; i+1<length; i+=9){
> -        if(!((~*(uint64_t*)(src+i) & (*(uint64_t*)(src+i) - 0x0100010001000101ULL)) & 0x8000800080008080ULL))
> +        if(!((~*(const uint64_t*)(src+i) & (*(const uint64_t*)(src+i) - 0x0100010001000101ULL)) & 0x8000800080008080ULL))
>  # else
>  #   define RS 3
>      for(i=0; i+1<length; i+=5){
> -        if(!((~*(uint32_t*)(src+i) & (*(uint32_t*)(src+i) - 0x01000101U)) & 0x80008080U))
> +        if(!((~*(const uint32_t*)(src+i) & (*(const uint32_t*)(src+i) - 0x01000101U)) & 0x80008080U))
>  # endif
>              continue;
>          if(i>0 && !src[i]) i--;

this does not belong in this patch


> @@ -1411,9 +1412,9 @@
>      }
>  
>      if(i>=length-1){ //no escaped 0
> -        *dst_length= length;
>          *consumed= length+1; //+1 for the header
> -        return src;
> +        ret= src;
> +        goto find_bitlength;
>      }
>  
>      bufidx = h->nal_unit_type == NAL_DPC ? 1 : 0; // use second escape buffer for inter data
> @@ -1450,27 +1451,34 @@
>  
>      memset(dst+di, 0, FF_INPUT_BUFFER_PADDING_SIZE);
>  
> -    *dst_length= di;
> +    length= di;
>      *consumed= si + 1;//+1 for the header

> -//FIXME store exact number of bits in the getbitcontext (it is needed for decoding)
> -    return dst;
> -}
> +    ret= dst;
>  
> -/**
> - * identifies the exact end of the bitstream
> - * @return the length of the trailing, or 0 if damaged
> - */
> -static int decode_rbsp_trailing(H264Context *h, const uint8_t *src){
> -    int v= *src;
> -    int r;

iam not happy that these 2 functions are merged

with factorization i did not mean to merge functions in a messy large blob
but to rather create a NEW function that factorizes the common code from
the 2 instances


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If a bugfix only changes things apparently unrelated to the bug with no
further explanation, that is a good sign that the bugfix is wrong.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090219/3c45992e/attachment.pgp>



More information about the ffmpeg-devel mailing list